From ef1b1b5d1715acf2f5f4b483dcffec4221292d87 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Wed, 5 Sep 2018 18:08:56 +0000 Subject: [PATCH] Modernize NativeProcessProtocol::GetSoftwareBreakpointTrapOpcode return the opcode as a Expected instead of a Status+pointer+size combo. I also move the linux implementation to the base class, as the trap opcodes are likely to be the same for all/most implementations of the class (except the arm one, where linux chooses a different opcode than what the arm spec recommends, which I keep linux-specific). llvm-svn: 341487 --- .../lldb/Host/common/NativeProcessProtocol.h | 6 +- lldb/source/Host/common/NativeProcessProtocol.cpp | 32 ++++++++++ lldb/source/Host/common/SoftwareBreakpoint.cpp | 57 +++-------------- .../Plugins/Process/Linux/NativeProcessLinux.cpp | 73 ++++------------------ .../Plugins/Process/Linux/NativeProcessLinux.h | 9 +-- .../Plugins/Process/NetBSD/NativeProcessNetBSD.cpp | 17 ----- .../Plugins/Process/NetBSD/NativeProcessNetBSD.h | 10 --- 7 files changed, 58 insertions(+), 146 deletions(-) diff --git a/lldb/include/lldb/Host/common/NativeProcessProtocol.h b/lldb/include/lldb/Host/common/NativeProcessProtocol.h index d96835d..43e732c 100644 --- a/lldb/include/lldb/Host/common/NativeProcessProtocol.h +++ b/lldb/include/lldb/Host/common/NativeProcessProtocol.h @@ -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> + GetSoftwareBreakpointTrapOpcode(size_t size_hint); // ----------------------------------------------------------- /// Notify the delegate that an exec occurred. diff --git a/lldb/source/Host/common/NativeProcessProtocol.cpp b/lldb/source/Host/common/NativeProcessProtocol.cpp index 7ccf846..073257c 100644 --- a/lldb/source/Host/common/NativeProcessProtocol.cpp +++ b/lldb/source/Host/common/NativeProcessProtocol.cpp @@ -372,6 +372,38 @@ Status NativeProcessProtocol::SetSoftwareBreakpoint(lldb::addr_t addr, }); } +llvm::Expected> +NativeProcessProtocol::GetSoftwareBreakpointTrapOpcode(size_t size_hint) { + using ArrayRef = llvm::ArrayRef; + + switch (GetArchitecture().GetMachine()) { + case llvm::Triple::aarch64: + return ArrayRef{0x00, 0x00, 0x20, 0xd4}; + + case llvm::Triple::x86: + case llvm::Triple::x86_64: + return ArrayRef{0xcc}; + + case llvm::Triple::mips: + case llvm::Triple::mips64: + return ArrayRef{0x00, 0x00, 0x00, 0x0d}; + + case llvm::Triple::mipsel: + case llvm::Triple::mips64el: + return ArrayRef{0x0d, 0x00, 0x00, 0x00}; + + case llvm::Triple::systemz: + return ArrayRef{0x00, 0x01}; + + case llvm::Triple::ppc64le: + return ArrayRef{0x08, 0x00, 0xe0, 0x7f}; // trap + + default: + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "CPU type not supported!"); + } +} + Status NativeProcessProtocol::RemoveBreakpoint(lldb::addr_t addr, bool hardware) { if (hardware) diff --git a/lldb/source/Host/common/SoftwareBreakpoint.cpp b/lldb/source/Host/common/SoftwareBreakpoint.cpp index 353dadf..836ab5e 100644 --- a/lldb/source/Host/common/SoftwareBreakpoint.cpp +++ b/lldb/source/Host/common/SoftwareBreakpoint.cpp @@ -34,56 +34,18 @@ Status SoftwareBreakpoint::CreateSoftwareBreakpoint( // 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::CreateSoftwareBreakpoint( // 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(); } diff --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp index b474e12..781d5ac 100644 --- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp +++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp @@ -1551,74 +1551,25 @@ Status NativeProcessLinux::RemoveBreakpoint(lldb::addr_t addr, bool hardware) { 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}; - // 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(); +llvm::Expected> +NativeProcessLinux::GetSoftwareBreakpointTrapOpcode(size_t size_hint) { + using ArrayRef = llvm::ArrayRef; + switch (GetArchitecture().GetMachine()) { case llvm::Triple::arm: - switch (trap_opcode_size_hint) { + // The ARM reference recommends the use of 0xe7fddefe and 0xdefe but the + // linux kernel does otherwise. + switch (size_hint) { case 2: - trap_opcode_bytes = g_thumb_breakpoint_opcode; - actual_opcode_size = sizeof(g_thumb_breakpoint_opcode); - return Status(); + return ArrayRef{0x01, 0xde}; case 4: - trap_opcode_bytes = g_arm_breakpoint_opcode; - actual_opcode_size = sizeof(g_arm_breakpoint_opcode); - return Status(); + return ArrayRef{0xf0, 0x01, 0xf0, 0xe7}; 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); } } diff --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.h b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.h index 1c2f786..97ad571 100644 --- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.h +++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.h @@ -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> + GetSoftwareBreakpointTrapOpcode(size_t size_hint) override; private: MainLoop::SignalHandleUP m_sigchld_handle; diff --git a/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp b/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp index 54d2e8b..8c0ba80 100644 --- a/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp +++ b/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp @@ -682,23 +682,6 @@ Status NativeProcessNetBSD::SetBreakpoint(lldb::addr_t addr, uint32_t size, 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"); diff --git a/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h b/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h index 142f74e..a08da8e 100644 --- a/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h +++ b/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h @@ -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; -- 2.7.4