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
s/regsiter/register/
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.
Repository:
rL LLVM
https://reviews.llvm.org/D25756
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
