[Lldb-commits] [lldb] r341487 - Modernize NativeProcessProtocol::GetSoftwareBreakpointTrapOpcode

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Wed Sep 5 11:08:56 PDT 2018


Author: labath
Date: Wed Sep  5 11:08:56 2018
New Revision: 341487

URL: http://llvm.org/viewvc/llvm-project?rev=341487&view=rev
Log:
Modernize NativeProcessProtocol::GetSoftwareBreakpointTrapOpcode

return the opcode as a Expected<ArrayRef> 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).

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=341487&r1=341486&r2=341487&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h (original)
+++ lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h Wed Sep  5 11:08:56 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=341487&r1=341486&r2=341487&view=diff
==============================================================================
--- lldb/trunk/source/Host/common/NativeProcessProtocol.cpp (original)
+++ lldb/trunk/source/Host/common/NativeProcessProtocol.cpp Wed Sep  5 11:08:56 2018
@@ -372,6 +372,38 @@ Status NativeProcessProtocol::SetSoftwar
       });
 }
 
+llvm::Expected<llvm::ArrayRef<uint8_t>>
+NativeProcessProtocol::GetSoftwareBreakpointTrapOpcode(size_t size_hint) {
+  using ArrayRef = llvm::ArrayRef<uint8_t>;
+
+  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)

Modified: lldb/trunk/source/Host/common/SoftwareBreakpoint.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/SoftwareBreakpoint.cpp?rev=341487&r1=341486&r2=341487&view=diff
==============================================================================
--- lldb/trunk/source/Host/common/SoftwareBreakpoint.cpp (original)
+++ lldb/trunk/source/Host/common/SoftwareBreakpoint.cpp Wed Sep  5 11:08:56 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=341487&r1=341486&r2=341487&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp Wed Sep  5 11:08:56 2018
@@ -1551,74 +1551,25 @@ 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};
-  // 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<llvm::ArrayRef<uint8_t>>
+NativeProcessLinux::GetSoftwareBreakpointTrapOpcode(size_t size_hint) {
+  using ArrayRef = llvm::ArrayRef<uint8_t>;
 
+  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);
   }
 }
 

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=341487&r1=341486&r2=341487&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h (original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h Wed Sep  5 11:08:56 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=341487&r1=341486&r2=341487&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp (original)
+++ lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp Wed Sep  5 11:08:56 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=341487&r1=341486&r2=341487&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h (original)
+++ lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h Wed Sep  5 11:08:56 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