[Lldb-commits] [lldb] 6b6bac2 - Fix off-by-one error in armv7 mach-o corefile register context

Jason Molenda via lldb-commits lldb-commits at lists.llvm.org
Wed Apr 26 13:06:14 PDT 2023


Author: Jason Molenda
Date: 2023-04-26T13:06:07-07:00
New Revision: 6b6bac2cd81df86354196304815b96c53bd93305

URL: https://github.com/llvm/llvm-project/commit/6b6bac2cd81df86354196304815b96c53bd93305
DIFF: https://github.com/llvm/llvm-project/commit/6b6bac2cd81df86354196304815b96c53bd93305.diff

LOG: Fix off-by-one error in armv7 mach-o corefile register context

The sanity check on the size of the register context we found in
the corefile was off by one, so lldb would not add the register
contents.  Add a test case to ensure it doesn't regress.

Differential Revision: https://reviews.llvm.org/D149224
rdar://108306070

Added: 
    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

Modified: 
    lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 621df79e52e56..33a70e21f9212 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -531,21 +531,18 @@ class RegisterContextDarwin_arm_Mach : public RegisterContextDarwin_arm {
       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;
 

diff  --git a/lldb/test/API/macosx/arm-corefile-regctx/Makefile b/lldb/test/API/macosx/arm-corefile-regctx/Makefile
new file mode 100644
index 0000000000000..e1d0354441cd4
--- /dev/null
+++ b/lldb/test/API/macosx/arm-corefile-regctx/Makefile
@@ -0,0 +1,6 @@
+MAKE_DSYM := NO
+
+CXX_SOURCES := create-arm-corefiles.cpp
+
+include Makefile.rules
+

diff  --git a/lldb/test/API/macosx/arm-corefile-regctx/TestArmMachoCorefileRegctx.py b/lldb/test/API/macosx/arm-corefile-regctx/TestArmMachoCorefileRegctx.py
new file mode 100644
index 0000000000000..9e84291816084
--- /dev/null
+++ b/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)

diff  --git a/lldb/test/API/macosx/arm-corefile-regctx/create-arm-corefiles.cpp b/lldb/test/API/macosx/arm-corefile-regctx/create-arm-corefiles.cpp
new file mode 100644
index 0000000000000..9f8dc65f8893f
--- /dev/null
+++ b/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);
+}


        


More information about the lldb-commits mailing list