Revert "Modernize NativeProcessProtocol::GetSoftwareBreakpointTrapOpcode"

This reverts commit r341487. Jan Kratochvil reports it breaks LLDB when
compiling with clang.

llvm-svn: 341747

6 files changed, 142 insertions, 56 deletions

diff --git a/lldb/source/Host/common/NativeProcessProtocol.cpp b/lldb/source/Host/common/NativeProcessProtocol.cpp
index 073257c04e8..7ccf8461272 100644
--- a/lldb/source/Host/common/NativeProcessProtocol.cpp
+++ b/lldb/source/Host/common/NativeProcessProtocol.cpp
@@ -372,38 +372,6 @@ Status NativeProcessProtocol::SetSoftwareBreakpoint(lldb::addr_t addr,
       });
 }
 
-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)
diff --git a/lldb/source/Host/common/SoftwareBreakpoint.cpp b/lldb/source/Host/common/SoftwareBreakpoint.cpp
index 836ab5e318f..353dadf6ce6 100644
--- a/lldb/source/Host/common/SoftwareBreakpoint.cpp
+++ b/lldb/source/Host/common/SoftwareBreakpoint.cpp
@@ -34,18 +34,56 @@ Status SoftwareBreakpoint::CreateSoftwareBreakpoint(
 
   // Ask the NativeProcessProtocol subclass to fill in the correct software
   // breakpoint trap for the breakpoint site.
-  auto expected_opcode = process.GetSoftwareBreakpointTrapOpcode(size_hint);
-  if (!expected_opcode)
-    return Status(expected_opcode.takeError());
+  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);
 
-  assert(expected_opcode->size() > 0);
-  assert(expected_opcode->size() <= MAX_TRAP_OPCODE_SIZE);
+  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);
+  }
 
   // Enable the breakpoint.
   uint8_t saved_opcode_bytes[MAX_TRAP_OPCODE_SIZE];
-  Status error =
-      EnableSoftwareBreakpoint(process, addr, expected_opcode->size(),
-                               expected_opcode->data(), saved_opcode_bytes);
+  error = EnableSoftwareBreakpoint(process, addr, bp_opcode_size,
+                                   bp_opcode_bytes, saved_opcode_bytes);
   if (error.Fail()) {
     if (log)
       log->Printf("SoftwareBreakpoint::%s: failed to enable new breakpoint at "
@@ -61,8 +99,7 @@ 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,
-                                             expected_opcode->data(),
-                                             expected_opcode->size()));
+                                             bp_opcode_bytes, bp_opcode_size));
   return Status();
 }
 
diff --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
index 781d5ac553f..b474e12d071 100644
--- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -1551,25 +1551,74 @@ Status NativeProcessLinux::RemoveBreakpoint(lldb::addr_t addr, bool hardware) {
     return NativeProcessProtocol::RemoveBreakpoint(addr);
 }
 
-llvm::Expected<llvm::ArrayRef<uint8_t>>
-NativeProcessLinux::GetSoftwareBreakpointTrapOpcode(size_t size_hint) {
-  using ArrayRef = llvm::ArrayRef<uint8_t>;
+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();
 
-  switch (GetArchitecture().GetMachine()) {
   case llvm::Triple::arm:
-    // The ARM reference recommends the use of 0xe7fddefe and 0xdefe but the
-    // linux kernel does otherwise.
-    switch (size_hint) {
+    switch (trap_opcode_size_hint) {
     case 2:
-      return ArrayRef{0x01, 0xde};
+      trap_opcode_bytes = g_thumb_breakpoint_opcode;
+      actual_opcode_size = sizeof(g_thumb_breakpoint_opcode);
+      return Status();
     case 4:
-      return ArrayRef{0xf0, 0x01, 0xf0, 0xe7};
+      trap_opcode_bytes = g_arm_breakpoint_opcode;
+      actual_opcode_size = sizeof(g_arm_breakpoint_opcode);
+      return Status();
     default:
-      return llvm::createStringError(llvm::inconvertibleErrorCode(),
-                                     "Unrecognised trap opcode size hint!");
+      assert(false && "Unrecognised trap opcode size hint!");
+      return Status("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:
-    return NativeProcessProtocol::GetSoftwareBreakpointTrapOpcode(size_hint);
+    assert(false && "CPU type not supported!");
+    return Status("CPU type not supported");
   }
 }
 
diff --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.h b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
index 97ad5711ff1..1c2f786e8d6 100644
--- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
+++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
@@ -134,8 +134,13 @@ public:
   bool SupportHardwareSingleStepping() const;
 
 protected:
-  llvm::Expected<llvm::ArrayRef<uint8_t>>
-  GetSoftwareBreakpointTrapOpcode(size_t size_hint) override;
+  // ---------------------------------------------------------------------
+  // 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;
diff --git a/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp b/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
index 8c0ba8079c0..54d2e8b552f 100644
--- a/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
+++ b/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
@@ -682,6 +682,23 @@ 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 a08da8efa80..142f74ecf19 100644
--- a/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
+++ b/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
@@ -93,6 +93,16 @@ 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;