[Lldb-commits] [PATCH] D25756: FreeBSD ARM support for software single step.

Ed Maste via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Wed Nov 30 12:41:47 PST 2016

emaste added inline comments.

Comment at: source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp:551-557
+    } else {
+      static const uint8_t g_arm_breakpoint_opcode[] = {0xFE,0xDE,0xFF,0xE7};
+      size_t trap_opcode_size = sizeof(g_arm_breakpoint_opcode);
+      assert(bp_site);
+      if (bp_site->SetTrapOpcode(g_arm_breakpoint_opcode, trap_opcode_size))
+        return trap_opcode_size;
according to LLVM style this should be promoted out of an `else` block because of the `return` above (use early exits / don't use else after a return).

As an aside it appears 0xe7fddefe is ARM-recommended breakpoint opcode and Linux is the outlier. So the generic Platform::GetSoftwareBreakpointTrapOpcode ought to use 0xFE,0xDE,0xFF,0xE7, with PlatformLinux::GetSoftwareBreakpointTrapOpcode having the override?

Comment at: source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp:957
+  // The emulator only fill in the dwarf regsiter numbers (and in some case
+  // the generic register numbers). Get the full register info from the
Few typo / nits:

only fill**s** in
in some case**s**

Comment at: source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp:1063
+  if (emulator_ap == nullptr) {
+    printf("Error: Instruction emulator not found!\n");
+    return;
We should really be returning an `Error` from this fn instead, no?

Comment at: source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp:1101-1104
+    // Emulate instruction failed and it haven't changed PC. Advance PC
+    // with the size of the current opcode because the emulation of all
+    // PC modifying instruction should be successful. The failure most
+    // likely caused by a not supported instruction which don't modify PC.
Hope you don't mind a minor rewording of the comment:

The emulated instruction failed and it did not change the PC. Advance the PC by the size of the current opcode, as all PC-modifying instructions should be successfully emulated. The failure was most likely caused by an unsupported instruction.



More information about the lldb-commits mailing list