jasonmolenda created this revision.
jasonmolenda added reviewers: fixathon, JDevlieghere.
jasonmolenda added a project: LLDB.
Herald added subscribers: omjavaid, kristof.beyls.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.
Slava was doing some fixups in ObjectFileMachO last August and one of the
changes had a small off-by-one error, https://reviews.llvm.org/D131554 . A
bunch of folks (including myself) looked at this specific bit of code and
didn't see the issue, and there's no testsuite coverage for it.
This bit of code in
`RegisterContextDarwin_arm_Mach::SetRegisterDataFrom_LC_THREAD` is reading the
data for an ARM_THREAD_STATE register context, which has a fixed size for all
practical purposes here. (the whole scheme of having ARM_THREAD_STATE +
ARM_THREAD_STATE_COUNT is to allow for new registers to be added in the future
and code to be able to detect which version it is based on size -- but I don't
expect us to add additional register to the armv7 register context as this
point).
Slava's change added a bounds check to ensure it was non-zero and within the
max size of the ARM_THREAD_STATE state size. This bounds check didn't account
for the cpsr register after the set of general purpose registers, so the
register context wasn't being loaded from corefiles. We can simplify this by
checking for equality for the total number of words for ARM_THREAD_STATE.
I also added a new test case that creates an empty armv7 and arm64 mach-o
corefile with two register contexts, loads them into lldb and confirms that it
could retrieve registers from both of the register contexts in both. I only
have this test set to build & run on Darwin systems because I use Mach-O
constants in the utility I wrote to create the nearly-empty corefiles. Too
easy to miss a tiny problem like this when there's no test coverage.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D149224
Files:
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/test/API/macosx/arm-corefile-regctx/Makefile
lldb/test/API/macosx/arm-corefile-regctx/TestArmMachoCorefileRegctx.py
lldb/test/API/macosx/arm-corefile-regctx/create-arm-corefiles.cpp
Index: lldb/test/API/macosx/arm-corefile-regctx/create-arm-corefiles.cpp
===================================================================
--- /dev/null
+++ lldb/test/API/macosx/arm-corefile-regctx/create-arm-corefiles.cpp
@@ -0,0 +1,199 @@
+#include <mach-o/loader.h>
+#include <mach/thread_status.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string>
+#include <vector>
+
+union uint32_buf {
+ uint8_t bytebuf[4];
+ uint32_t val;
+};
+
+union uint64_buf {
+ uint8_t bytebuf[8];
+ uint64_t val;
+};
+
+void add_uint64(std::vector<uint8_t> &buf, uint64_t val) {
+ uint64_buf conv;
+ conv.val = val;
+ for (int i = 0; i < 8; i++)
+ buf.push_back(conv.bytebuf[i]);
+}
+
+void add_uint32(std::vector<uint8_t> &buf, uint32_t val) {
+ uint32_buf conv;
+ conv.val = val;
+ for (int i = 0; i < 4; i++)
+ buf.push_back(conv.bytebuf[i]);
+}
+
+std::vector<uint8_t> armv7_lc_thread_load_command() {
+ std::vector<uint8_t> data;
+ add_uint32(data, LC_THREAD); // thread_command.cmd
+ add_uint32(data, 104); // thread_command.cmdsize
+ add_uint32(data, ARM_THREAD_STATE); // thread_command.flavor
+ add_uint32(data, ARM_THREAD_STATE_COUNT); // thread_command.count
+ add_uint32(data, 0x00010000); // r0
+ add_uint32(data, 0x00020000); // r1
+ add_uint32(data, 0x00030000); // r2
+ add_uint32(data, 0x00040000); // r3
+ add_uint32(data, 0x00050000); // r4
+ add_uint32(data, 0x00060000); // r5
+ add_uint32(data, 0x00070000); // r6
+ add_uint32(data, 0x00080000); // r7
+ add_uint32(data, 0x00090000); // r8
+ add_uint32(data, 0x000a0000); // r9
+ add_uint32(data, 0x000b0000); // r10
+ add_uint32(data, 0x000c0000); // r11
+ add_uint32(data, 0x000d0000); // r12
+ add_uint32(data, 0x000e0000); // sp
+ add_uint32(data, 0x000f0000); // lr
+ add_uint32(data, 0x00100000); // pc
+ add_uint32(data, 0x00110000); // cpsr
+
+ add_uint32(data, ARM_EXCEPTION_STATE); // thread_command.flavor
+ add_uint32(data, ARM_EXCEPTION_STATE_COUNT); // thread_command.count
+ add_uint32(data, 0x00003f5c); // far
+ add_uint32(data, 0xf2000000); // esr
+ add_uint32(data, 0x00000000); // exception
+
+ return data;
+}
+
+std::vector<uint8_t> arm64_lc_thread_load_command() {
+ std::vector<uint8_t> data;
+ add_uint32(data, LC_THREAD); // thread_command.cmd
+ add_uint32(data, 312); // thread_command.cmdsize
+ add_uint32(data, ARM_THREAD_STATE64); // thread_command.flavor
+ add_uint32(data, ARM_THREAD_STATE64_COUNT); // thread_command.count
+ add_uint64(data, 0x0000000000000001); // x0
+ add_uint64(data, 0x000000016fdff3c0); // x1
+ add_uint64(data, 0x000000016fdff3d0); // x2
+ add_uint64(data, 0x000000016fdff510); // x3
+ add_uint64(data, 0x0000000000000000); // x4
+ add_uint64(data, 0x0000000000000000); // x5
+ add_uint64(data, 0x0000000000000000); // x6
+ add_uint64(data, 0x0000000000000000); // x7
+ add_uint64(data, 0x000000010000d910); // x8
+ add_uint64(data, 0x0000000000000001); // x9
+ add_uint64(data, 0xe1e88de000000000); // x10
+ add_uint64(data, 0x0000000000000003); // x11
+ add_uint64(data, 0x0000000000000148); // x12
+ add_uint64(data, 0x0000000000004000); // x13
+ add_uint64(data, 0x0000000000000008); // x14
+ add_uint64(data, 0x0000000000000000); // x15
+ add_uint64(data, 0x0000000000000000); // x16
+ add_uint64(data, 0x0000000100003f5c); // x17
+ add_uint64(data, 0x0000000000000000); // x18
+ add_uint64(data, 0x0000000100003f5c); // x19
+ add_uint64(data, 0x000000010000c000); // x20
+ add_uint64(data, 0x000000010000d910); // x21
+ add_uint64(data, 0x000000016fdff250); // x22
+ add_uint64(data, 0x000000018ce12366); // x23
+ add_uint64(data, 0x000000016fdff1d0); // x24
+ add_uint64(data, 0x0000000000000001); // x25
+ add_uint64(data, 0x0000000000000000); // x26
+ add_uint64(data, 0x0000000000000000); // x27
+ add_uint64(data, 0x0000000000000000); // x28
+ add_uint64(data, 0x000000016fdff3a0); // fp
+ add_uint64(data, 0x000000018cd97f28); // lr
+ add_uint64(data, 0x000000016fdff140); // sp
+ add_uint64(data, 0x0000000100003f5c); // pc
+ add_uint32(data, 0x80001000); // cpsr
+
+ add_uint32(data, 0x00000000); // padding
+
+ add_uint32(data, ARM_EXCEPTION_STATE64); // thread_command.flavor
+ add_uint32(data, ARM_EXCEPTION_STATE64_COUNT); // thread_command.count
+ add_uint64(data, 0x0000000100003f5c); // far
+ add_uint32(data, 0xf2000000); // esr
+ add_uint32(data, 0x00000000); // exception
+
+ return data;
+}
+
+enum arch { unspecified, armv7, arm64 };
+
+int main(int argc, char **argv) {
+ if (argc != 3) {
+ fprintf(stderr,
+ "usage: create-arm-corefiles [armv7|arm64] <output-core-name>\n");
+ exit(1);
+ }
+
+ arch arch = unspecified;
+
+ if (strcmp(argv[1], "armv7") == 0)
+ arch = armv7;
+ else if (strcmp(argv[1], "arm64") == 0)
+ arch = arm64;
+ else {
+ fprintf(stderr, "unrecognized architecture %s\n", argv[1]);
+ exit(1);
+ }
+
+ // An array of load commands (in the form of byte arrays)
+ std::vector<std::vector<uint8_t>> load_commands;
+
+ // An array of corefile contents (page data, lc_note data, etc)
+ std::vector<uint8_t> payload;
+
+ // First add all the load commands / payload so we can figure out how large
+ // the load commands will actually be.
+ if (arch == armv7)
+ load_commands.push_back(armv7_lc_thread_load_command());
+ else if (arch == arm64)
+ load_commands.push_back(arm64_lc_thread_load_command());
+
+ int size_of_load_commands = 0;
+ for (const auto &lc : load_commands)
+ size_of_load_commands += lc.size();
+
+ int header_and_load_cmd_room =
+ sizeof(struct mach_header_64) + size_of_load_commands;
+
+ // Erase the load commands / payload now that we know how much space is
+ // needed, redo it.
+ load_commands.clear();
+ payload.clear();
+
+ if (arch == armv7)
+ load_commands.push_back(armv7_lc_thread_load_command());
+ else if (arch == arm64)
+ load_commands.push_back(arm64_lc_thread_load_command());
+
+ struct mach_header_64 mh;
+ mh.magic = MH_MAGIC_64;
+ if (arch == armv7) {
+ mh.cputype = CPU_TYPE_ARM;
+ mh.cpusubtype = CPU_SUBTYPE_ARM_V7M;
+ } else if (arch == arm64) {
+ mh.cputype = CPU_TYPE_ARM64;
+ mh.cpusubtype = CPU_SUBTYPE_ARM64_ALL;
+ }
+ mh.filetype = MH_CORE;
+ mh.ncmds = load_commands.size();
+ mh.sizeofcmds = size_of_load_commands;
+ mh.flags = 0;
+ mh.reserved = 0;
+
+ FILE *f = fopen(argv[2], "w");
+
+ if (f == nullptr) {
+ fprintf(stderr, "Unable to open file %s for writing\n", argv[2]);
+ exit(1);
+ }
+
+ fwrite(&mh, sizeof(struct mach_header_64), 1, f);
+
+ for (const auto &lc : load_commands)
+ fwrite(lc.data(), lc.size(), 1, f);
+
+ fseek(f, header_and_load_cmd_room, SEEK_SET);
+
+ fwrite(payload.data(), payload.size(), 1, f);
+
+ fclose(f);
+}
Index: lldb/test/API/macosx/arm-corefile-regctx/TestArmMachoCorefileRegctx.py
===================================================================
--- /dev/null
+++ lldb/test/API/macosx/arm-corefile-regctx/TestArmMachoCorefileRegctx.py
@@ -0,0 +1,69 @@
+"""Test that Mach-O armv7/arm64 corefile register contexts are read by lldb."""
+
+
+
+import os
+import re
+import subprocess
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestArmMachoCorefileRegctx(TestBase):
+
+ NO_DEBUG_INFO_TESTCASE = True
+ @skipUnlessDarwin
+
+ def setUp(self):
+ TestBase.setUp(self)
+ self.build()
+ self.create_corefile = self.getBuildArtifact("a.out")
+ self.corefile = self.getBuildArtifact("core")
+
+ def test_armv7_corefile(self):
+ ### Create corefile
+ retcode = call(self.create_corefile + " armv7 " + self.corefile, shell=True)
+
+ target = self.dbg.CreateTarget('')
+ err = lldb.SBError()
+ process = target.LoadCore(self.corefile)
+ self.assertEqual(process.IsValid(), True)
+ thread = process.GetSelectedThread()
+ frame = thread.GetSelectedFrame()
+
+ lr = frame.FindRegister("lr")
+ self.assertTrue(lr.IsValid())
+ self.assertEqual(lr.GetValueAsUnsigned(), 0x000f0000)
+
+ pc = frame.FindRegister("pc")
+ self.assertTrue(pc.IsValid())
+ self.assertEqual(pc.GetValueAsUnsigned(), 0x00100000)
+
+ exception = frame.FindRegister("exception")
+ self.assertTrue(exception.IsValid())
+ self.assertEqual(exception.GetValueAsUnsigned(), 0x00003f5c)
+
+ def test_arm64_corefile(self):
+ ### Create corefile
+ retcode = call(self.create_corefile + " arm64 " + self.corefile, shell=True)
+
+ target = self.dbg.CreateTarget('')
+ err = lldb.SBError()
+ process = target.LoadCore(self.corefile)
+ self.assertEqual(process.IsValid(), True)
+ thread = process.GetSelectedThread()
+ frame = thread.GetSelectedFrame()
+
+ lr = frame.FindRegister("lr")
+ self.assertTrue(lr.IsValid())
+ self.assertEqual(lr.GetValueAsUnsigned(), 0x000000018cd97f28)
+
+ pc = frame.FindRegister("pc")
+ self.assertTrue(pc.IsValid())
+ self.assertEqual(pc.GetValueAsUnsigned(), 0x0000000100003f5c)
+
+ exception = frame.FindRegister("far")
+ self.assertTrue(exception.IsValid())
+ self.assertEqual(exception.GetValueAsUnsigned(), 0x0000000100003f5c)
Index: lldb/test/API/macosx/arm-corefile-regctx/Makefile
===================================================================
--- /dev/null
+++ lldb/test/API/macosx/arm-corefile-regctx/Makefile
@@ -0,0 +1,6 @@
+MAKE_DSYM := NO
+
+CXX_SOURCES := create-arm-corefiles.cpp
+
+include Makefile.rules
+
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===================================================================
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -531,21 +531,18 @@
lldb::offset_t next_thread_state = offset + (count * 4);
switch (flavor) {
case GPRAltRegSet:
- case GPRRegSet:
- // On ARM, the CPSR register is also included in the count but it is
- // not included in gpr.r so loop until (count-1).
-
- // Prevent static analysis warnings by explicitly contstraining 'count'
- // to acceptable range. Handle possible underflow of count-1
- if (count > 0 && count <= sizeof(gpr.r) / sizeof(gpr.r[0])) {
+ case GPRRegSet: {
+ // r0-r15, plus CPSR
+ uint32_t gpr_buf_count = (sizeof(gpr.r) / sizeof(gpr.r[0])) + 1;
+ if (count == gpr_buf_count) {
for (uint32_t i = 0; i < (count - 1); ++i) {
gpr.r[i] = data.GetU32(&offset);
}
- }
- // Save cpsr explicitly.
- gpr.cpsr = data.GetU32(&offset);
+ gpr.cpsr = data.GetU32(&offset);
- SetError(GPRRegSet, Read, 0);
+ SetError(GPRRegSet, Read, 0);
+ }
+ }
offset = next_thread_state;
break;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
