[Lldb-commits] [lldb] r341758 - Re-commit "Modernize NativeProcessProtocol::GetSoftwareBreakpointTrapOpcode"
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Sat Sep 8 23:01:12 PDT 2018
Author: labath
Date: Sat Sep 8 23:01:12 2018
New Revision: 341758
URL: http://llvm.org/viewvc/llvm-project?rev=341758&view=rev
Log:
Re-commit "Modernize NativeProcessProtocol::GetSoftwareBreakpointTrapOpcode"
This recommits r341487, which was reverted due to failing tests with
clang. It turned out I had incorrectly expected that the literal arrays
passed to ArrayRef constructor will have static (permanent) storage.
This was only the case with gcc, while clang was constructing them on
stack, leading to dangling pointers when the function returns.
The fix is to explicitly assign static storage duration to the opcode
arrays.
Modified:
lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h
lldb/trunk/source/Host/common/NativeProcessProtocol.cpp
lldb/trunk/source/Host/common/SoftwareBreakpoint.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h
lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
Modified: lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h?rev=341758&r1=341757&r2=341758&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h (original)
+++ lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h Sat Sep 8 23:01:12 2018
@@ -448,10 +448,8 @@ protected:
// -----------------------------------------------------------
Status SetSoftwareBreakpoint(lldb::addr_t addr, uint32_t size_hint);
- virtual Status
- GetSoftwareBreakpointTrapOpcode(size_t trap_opcode_size_hint,
- size_t &actual_opcode_size,
- const uint8_t *&trap_opcode_bytes) = 0;
+ virtual llvm::Expected<llvm::ArrayRef<uint8_t>>
+ GetSoftwareBreakpointTrapOpcode(size_t size_hint);
// -----------------------------------------------------------
/// Notify the delegate that an exec occurred.
Modified: lldb/trunk/source/Host/common/NativeProcessProtocol.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/NativeProcessProtocol.cpp?rev=341758&r1=341757&r2=341758&view=diff
==============================================================================
--- lldb/trunk/source/Host/common/NativeProcessProtocol.cpp (original)
+++ lldb/trunk/source/Host/common/NativeProcessProtocol.cpp Sat Sep 8 23:01:12 2018
@@ -372,6 +372,43 @@ Status NativeProcessProtocol::SetSoftwar
});
}
+llvm::Expected<llvm::ArrayRef<uint8_t>>
+NativeProcessProtocol::GetSoftwareBreakpointTrapOpcode(size_t size_hint) {
+ static const uint8_t g_aarch64_opcode[] = {0x00, 0x00, 0x20, 0xd4};
+ static const uint8_t g_i386_opcode[] = {0xCC};
+ static const uint8_t g_mips64_opcode[] = {0x00, 0x00, 0x00, 0x0d};
+ static const uint8_t g_mips64el_opcode[] = {0x0d, 0x00, 0x00, 0x00};
+ static const uint8_t g_s390x_opcode[] = {0x00, 0x01};
+ static const uint8_t g_ppc64le_opcode[] = {0x08, 0x00, 0xe0, 0x7f}; // trap
+
+ switch (GetArchitecture().GetMachine()) {
+ case llvm::Triple::aarch64:
+ return g_aarch64_opcode;
+
+ case llvm::Triple::x86:
+ case llvm::Triple::x86_64:
+ return g_i386_opcode;
+
+ case llvm::Triple::mips:
+ case llvm::Triple::mips64:
+ return g_mips64_opcode;
+
+ case llvm::Triple::mipsel:
+ case llvm::Triple::mips64el:
+ return g_mips64el_opcode;;
+
+ case llvm::Triple::systemz:
+ return g_s390x_opcode;
+
+ case llvm::Triple::ppc64le:
+ return g_ppc64le_opcode;
+
+ default:
+ return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "CPU type not supported!");
+ }
+}
+
Status NativeProcessProtocol::RemoveBreakpoint(lldb::addr_t addr,
bool hardware) {
if (hardware)
Modified: lldb/trunk/source/Host/common/SoftwareBreakpoint.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/SoftwareBreakpoint.cpp?rev=341758&r1=341757&r2=341758&view=diff
==============================================================================
--- lldb/trunk/source/Host/common/SoftwareBreakpoint.cpp (original)
+++ lldb/trunk/source/Host/common/SoftwareBreakpoint.cpp Sat Sep 8 23:01:12 2018
@@ -34,56 +34,18 @@ Status SoftwareBreakpoint::CreateSoftwar
// Ask the NativeProcessProtocol subclass to fill in the correct software
// breakpoint trap for the breakpoint site.
- size_t bp_opcode_size = 0;
- const uint8_t *bp_opcode_bytes = NULL;
- Status error = process.GetSoftwareBreakpointTrapOpcode(
- size_hint, bp_opcode_size, bp_opcode_bytes);
+ auto expected_opcode = process.GetSoftwareBreakpointTrapOpcode(size_hint);
+ if (!expected_opcode)
+ return Status(expected_opcode.takeError());
- if (error.Fail()) {
- if (log)
- log->Printf("SoftwareBreakpoint::%s failed to retrieve software "
- "breakpoint trap opcode: %s",
- __FUNCTION__, error.AsCString());
- return error;
- }
-
- // Validate size of trap opcode.
- if (bp_opcode_size == 0) {
- if (log)
- log->Printf("SoftwareBreakpoint::%s failed to retrieve any trap opcodes",
- __FUNCTION__);
- return Status("SoftwareBreakpoint::GetSoftwareBreakpointTrapOpcode() "
- "returned zero, unable to get breakpoint trap for address "
- "0x%" PRIx64,
- addr);
- }
-
- if (bp_opcode_size > MAX_TRAP_OPCODE_SIZE) {
- if (log)
- log->Printf("SoftwareBreakpoint::%s cannot support %zu trapcode bytes, "
- "max size is %zu",
- __FUNCTION__, bp_opcode_size, MAX_TRAP_OPCODE_SIZE);
- return Status("SoftwareBreakpoint::GetSoftwareBreakpointTrapOpcode() "
- "returned too many trap opcode bytes: requires %zu but we "
- "only support a max of %zu",
- bp_opcode_size, MAX_TRAP_OPCODE_SIZE);
- }
-
- // Validate that we received opcodes.
- if (!bp_opcode_bytes) {
- if (log)
- log->Printf("SoftwareBreakpoint::%s failed to retrieve trap opcode bytes",
- __FUNCTION__);
- return Status("SoftwareBreakpoint::GetSoftwareBreakpointTrapOpcode() "
- "returned NULL trap opcode bytes, unable to get breakpoint "
- "trap for address 0x%" PRIx64,
- addr);
- }
+ assert(expected_opcode->size() > 0);
+ assert(expected_opcode->size() <= MAX_TRAP_OPCODE_SIZE);
// Enable the breakpoint.
uint8_t saved_opcode_bytes[MAX_TRAP_OPCODE_SIZE];
- error = EnableSoftwareBreakpoint(process, addr, bp_opcode_size,
- bp_opcode_bytes, saved_opcode_bytes);
+ Status error =
+ EnableSoftwareBreakpoint(process, addr, expected_opcode->size(),
+ expected_opcode->data(), saved_opcode_bytes);
if (error.Fail()) {
if (log)
log->Printf("SoftwareBreakpoint::%s: failed to enable new breakpoint at "
@@ -99,7 +61,8 @@ Status SoftwareBreakpoint::CreateSoftwar
// Set the breakpoint and verified it was written properly. Now create a
// breakpoint remover that understands how to undo this breakpoint.
breakpoint_sp.reset(new SoftwareBreakpoint(process, addr, saved_opcode_bytes,
- bp_opcode_bytes, bp_opcode_size));
+ expected_opcode->data(),
+ expected_opcode->size()));
return Status();
}
Modified: lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp?rev=341758&r1=341757&r2=341758&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp Sat Sep 8 23:01:12 2018
@@ -1551,74 +1551,26 @@ Status NativeProcessLinux::RemoveBreakpo
return NativeProcessProtocol::RemoveBreakpoint(addr);
}
-Status NativeProcessLinux::GetSoftwareBreakpointTrapOpcode(
- size_t trap_opcode_size_hint, size_t &actual_opcode_size,
- const uint8_t *&trap_opcode_bytes) {
- // FIXME put this behind a breakpoint protocol class that can be set per
- // architecture. Need MIPS support here.
- static const uint8_t g_aarch64_opcode[] = {0x00, 0x00, 0x20, 0xd4};
+llvm::Expected<llvm::ArrayRef<uint8_t>>
+NativeProcessLinux::GetSoftwareBreakpointTrapOpcode(size_t size_hint) {
// The ARM reference recommends the use of 0xe7fddefe and 0xdefe but the
// linux kernel does otherwise.
- static const uint8_t g_arm_breakpoint_opcode[] = {0xf0, 0x01, 0xf0, 0xe7};
- static const uint8_t g_i386_opcode[] = {0xCC};
- static const uint8_t g_mips64_opcode[] = {0x00, 0x00, 0x00, 0x0d};
- static const uint8_t g_mips64el_opcode[] = {0x0d, 0x00, 0x00, 0x00};
- static const uint8_t g_s390x_opcode[] = {0x00, 0x01};
- static const uint8_t g_thumb_breakpoint_opcode[] = {0x01, 0xde};
- static const uint8_t g_ppc64le_opcode[] = {0x08, 0x00, 0xe0, 0x7f}; // trap
-
- switch (m_arch.GetMachine()) {
- case llvm::Triple::aarch64:
- trap_opcode_bytes = g_aarch64_opcode;
- actual_opcode_size = sizeof(g_aarch64_opcode);
- return Status();
+ static const uint8_t g_arm_opcode[] = {0xf0, 0x01, 0xf0, 0xe7};
+ static const uint8_t g_thumb_opcode[] = {0x01, 0xde};
+ switch (GetArchitecture().GetMachine()) {
case llvm::Triple::arm:
- switch (trap_opcode_size_hint) {
+ switch (size_hint) {
case 2:
- trap_opcode_bytes = g_thumb_breakpoint_opcode;
- actual_opcode_size = sizeof(g_thumb_breakpoint_opcode);
- return Status();
+ return g_thumb_opcode;
case 4:
- trap_opcode_bytes = g_arm_breakpoint_opcode;
- actual_opcode_size = sizeof(g_arm_breakpoint_opcode);
- return Status();
+ return g_arm_opcode;
default:
- assert(false && "Unrecognised trap opcode size hint!");
- return Status("Unrecognised trap opcode size hint!");
+ return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "Unrecognised trap opcode size hint!");
}
-
- case llvm::Triple::x86:
- case llvm::Triple::x86_64:
- trap_opcode_bytes = g_i386_opcode;
- actual_opcode_size = sizeof(g_i386_opcode);
- return Status();
-
- case llvm::Triple::mips:
- case llvm::Triple::mips64:
- trap_opcode_bytes = g_mips64_opcode;
- actual_opcode_size = sizeof(g_mips64_opcode);
- return Status();
-
- case llvm::Triple::mipsel:
- case llvm::Triple::mips64el:
- trap_opcode_bytes = g_mips64el_opcode;
- actual_opcode_size = sizeof(g_mips64el_opcode);
- return Status();
-
- case llvm::Triple::systemz:
- trap_opcode_bytes = g_s390x_opcode;
- actual_opcode_size = sizeof(g_s390x_opcode);
- return Status();
-
- case llvm::Triple::ppc64le:
- trap_opcode_bytes = g_ppc64le_opcode;
- actual_opcode_size = sizeof(g_ppc64le_opcode);
- return Status();
-
default:
- assert(false && "CPU type not supported!");
- return Status("CPU type not supported");
+ return NativeProcessProtocol::GetSoftwareBreakpointTrapOpcode(size_hint);
}
}
Modified: lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h?rev=341758&r1=341757&r2=341758&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h (original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h Sat Sep 8 23:01:12 2018
@@ -134,13 +134,8 @@ public:
bool SupportHardwareSingleStepping() const;
protected:
- // ---------------------------------------------------------------------
- // NativeProcessProtocol protected interface
- // ---------------------------------------------------------------------
- Status
- GetSoftwareBreakpointTrapOpcode(size_t trap_opcode_size_hint,
- size_t &actual_opcode_size,
- const uint8_t *&trap_opcode_bytes) override;
+ llvm::Expected<llvm::ArrayRef<uint8_t>>
+ GetSoftwareBreakpointTrapOpcode(size_t size_hint) override;
private:
MainLoop::SignalHandleUP m_sigchld_handle;
Modified: lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp?rev=341758&r1=341757&r2=341758&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp (original)
+++ lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp Sat Sep 8 23:01:12 2018
@@ -682,23 +682,6 @@ Status NativeProcessNetBSD::SetBreakpoin
return SetSoftwareBreakpoint(addr, size);
}
-Status NativeProcessNetBSD::GetSoftwareBreakpointTrapOpcode(
- size_t trap_opcode_size_hint, size_t &actual_opcode_size,
- const uint8_t *&trap_opcode_bytes) {
- static const uint8_t g_i386_opcode[] = {0xCC};
-
- switch (m_arch.GetMachine()) {
- case llvm::Triple::x86:
- case llvm::Triple::x86_64:
- trap_opcode_bytes = g_i386_opcode;
- actual_opcode_size = sizeof(g_i386_opcode);
- return Status();
- default:
- assert(false && "CPU type not supported!");
- return Status("CPU type not supported");
- }
-}
-
Status NativeProcessNetBSD::GetLoadedModuleFileSpec(const char *module_path,
FileSpec &file_spec) {
return Status("Unimplemented");
Modified: lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h?rev=341758&r1=341757&r2=341758&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h (original)
+++ lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h Sat Sep 8 23:01:12 2018
@@ -93,16 +93,6 @@ public:
static Status PtraceWrapper(int req, lldb::pid_t pid, void *addr = nullptr,
int data = 0, int *result = nullptr);
-protected:
- // ---------------------------------------------------------------------
- // NativeProcessProtocol protected interface
- // ---------------------------------------------------------------------
-
- Status
- GetSoftwareBreakpointTrapOpcode(size_t trap_opcode_size_hint,
- size_t &actual_opcode_size,
- const uint8_t *&trap_opcode_bytes) override;
-
private:
MainLoop::SignalHandleUP m_sigchld_handle;
ArchSpec m_arch;
More information about the lldb-commits
mailing list