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

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

@@ -534,21 +534,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;
 

lldb/test/API/macosx/arm-corefile-regctx/Makefile

@@ -0,0 +1,6 @@
+MAKE_DSYM := NO
+
+CXX_SOURCES := create-arm-corefiles.cpp
+
+include Makefile.rules
+

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)

lldb/test/API/macosx/arm-corefile-regctx/create-arm-corefiles.cpp

@@ -0,0 +1,214 @@
+#include <mach-o/loader.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string>
+#include <vector>
+
+
+// Normally these are picked up by including <mach/thread_status.h>
+// but that does a compile time check for the build host arch and
+// only defines the ARM register context constants when building on
+// an arm system.  We're creating fake corefiles, and might be
+// creating them on an intel system.
+#define ARM_THREAD_STATE 1
+#define ARM_THREAD_STATE_COUNT 17
+#define ARM_EXCEPTION_STATE 3
+#define ARM_EXCEPTION_STATE_COUNT 3
+#define ARM_THREAD_STATE64 6
+#define ARM_THREAD_STATE64_COUNT 68
+#define ARM_EXCEPTION_STATE64 7
+#define ARM_EXCEPTION_STATE64_COUNT 4
+
+
+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);
+}