[Lldb-commits] [lldb] r346093 - NativeProcessProtocol: Simplify breakpoint setting code

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Sun Nov 4 02:58:08 PST 2018


Author: labath
Date: Sun Nov  4 02:58:08 2018
New Revision: 346093

URL: http://llvm.org/viewvc/llvm-project?rev=346093&view=rev
Log:
NativeProcessProtocol: Simplify breakpoint setting code

Summary:
A fairly simple operation as setting a breakpoint (writing a breakpoint
opcode) at a given address was going through three classes:
NativeProcessProtocol which called NativeBreakpointList, which then
called SoftwareBrekpoint, only to end up again in NativeProcessProtocol
to do the actual writing itself. This is unnecessarily complex and can
be simplified by moving all of the logic into NativeProcessProtocol
class itself, removing a lot of boilerplate.

One of the reeasons for this complexity was that (it seems)
NativeBreakpointList class was meant to hold both software and hardware
breakpoints. However, that never materialized, and hardware breakpoints
are stored in a separate map holding only hardware breakpoints.
Essentially, this patch makes software breakpoints follow that approach
by replacing the heavy SoftwareBraekpoint with a light struct of the
same name, which holds only the data necessary to describe one
breakpoint. The rest of the logic is in the main class. As, at the
lldb-server level, handling software and hardware breakpoints is very
different, this seems like a reasonable state of things.

Reviewers: krytarowski, zturner, clayborg

Subscribers: mgorny, lldb-commits

Differential Revision: https://reviews.llvm.org/D52941

Removed:
    lldb/trunk/include/lldb/Host/common/NativeBreakpoint.h
    lldb/trunk/include/lldb/Host/common/SoftwareBreakpoint.h
    lldb/trunk/source/Host/common/NativeBreakpoint.cpp
    lldb/trunk/source/Host/common/NativeBreakpointList.cpp
    lldb/trunk/source/Host/common/SoftwareBreakpoint.cpp
Modified:
    lldb/trunk/include/lldb/Host/common/NativeBreakpointList.h
    lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h
    lldb/trunk/include/lldb/lldb-private-forward.h
    lldb/trunk/lldb.xcodeproj/project.pbxproj
    lldb/trunk/source/Host/CMakeLists.txt
    lldb/trunk/source/Host/common/NativeProcessProtocol.cpp
    lldb/trunk/source/Host/common/NativeThreadProtocol.cpp
    lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
    lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp

Removed: lldb/trunk/include/lldb/Host/common/NativeBreakpoint.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/common/NativeBreakpoint.h?rev=346092&view=auto
==============================================================================
--- lldb/trunk/include/lldb/Host/common/NativeBreakpoint.h (original)
+++ lldb/trunk/include/lldb/Host/common/NativeBreakpoint.h (removed)
@@ -1,56 +0,0 @@
-//===-- NativeBreakpoint.h --------------------------------------*- C++ -*-===//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef liblldb_NativeBreakpoint_h_
-#define liblldb_NativeBreakpoint_h_
-
-#include "lldb/lldb-types.h"
-
-namespace lldb_private {
-class NativeBreakpointList;
-
-class NativeBreakpoint {
-  friend class NativeBreakpointList;
-
-public:
-  // The assumption is that derived breakpoints are enabled when created.
-  NativeBreakpoint(lldb::addr_t addr);
-
-  virtual ~NativeBreakpoint();
-
-  Status Enable();
-
-  Status Disable();
-
-  lldb::addr_t GetAddress() const { return m_addr; }
-
-  bool IsEnabled() const { return m_enabled; }
-
-  virtual bool IsSoftwareBreakpoint() const = 0;
-
-protected:
-  const lldb::addr_t m_addr;
-  int32_t m_ref_count;
-
-  virtual Status DoEnable() = 0;
-
-  virtual Status DoDisable() = 0;
-
-private:
-  bool m_enabled;
-
-  // ----------------------------------------------------------- interface for
-  // NativeBreakpointList
-  // -----------------------------------------------------------
-  void AddRef();
-  int32_t DecRef();
-};
-}
-
-#endif // ifndef liblldb_NativeBreakpoint_h_

Modified: lldb/trunk/include/lldb/Host/common/NativeBreakpointList.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/common/NativeBreakpointList.h?rev=346093&r1=346092&r2=346093&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Host/common/NativeBreakpointList.h (original)
+++ lldb/trunk/include/lldb/Host/common/NativeBreakpointList.h Sun Nov  4 02:58:08 2018
@@ -10,13 +10,9 @@
 #ifndef liblldb_NativeBreakpointList_h_
 #define liblldb_NativeBreakpointList_h_
 
-#include "lldb/Utility/Status.h"
 #include "lldb/lldb-private-forward.h"
-// #include "lldb/Host/NativeBreakpoint.h"
-
-#include <functional>
+#include "lldb/lldb-types.h"
 #include <map>
-#include <mutex>
 
 namespace lldb_private {
 
@@ -26,35 +22,6 @@ struct HardwareBreakpoint {
 };
 
 using HardwareBreakpointMap = std::map<lldb::addr_t, HardwareBreakpoint>;
-
-class NativeBreakpointList {
-public:
-  typedef std::function<Status(lldb::addr_t addr, size_t size_hint,
-                               bool hardware,
-                               NativeBreakpointSP &breakpoint_sp)>
-      CreateBreakpointFunc;
-
-  NativeBreakpointList();
-
-  Status AddRef(lldb::addr_t addr, size_t size_hint, bool hardware,
-                CreateBreakpointFunc create_func);
-
-  Status DecRef(lldb::addr_t addr);
-
-  Status EnableBreakpoint(lldb::addr_t addr);
-
-  Status DisableBreakpoint(lldb::addr_t addr);
-
-  Status GetBreakpoint(lldb::addr_t addr, NativeBreakpointSP &breakpoint_sp);
-
-  Status RemoveTrapsFromBuffer(lldb::addr_t addr, void *buf, size_t size) const;
-
-private:
-  typedef std::map<lldb::addr_t, NativeBreakpointSP> BreakpointMap;
-
-  std::recursive_mutex m_mutex;
-  BreakpointMap m_breakpoints;
-};
 }
 
 #endif // ifndef liblldb_NativeBreakpointList_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=346093&r1=346092&r2=346093&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h (original)
+++ lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h Sun Nov  4 02:58:08 2018
@@ -25,6 +25,8 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include <mutex>
+#include <unordered_map>
 #include <vector>
 
 namespace lldb_private {
@@ -35,8 +37,6 @@ class ResumeActionList;
 // NativeProcessProtocol
 //------------------------------------------------------------------
 class NativeProcessProtocol {
-  friend class SoftwareBreakpoint;
-
 public:
   virtual ~NativeProcessProtocol() {}
 
@@ -111,10 +111,6 @@ public:
 
   virtual Status RemoveBreakpoint(lldb::addr_t addr, bool hardware = false);
 
-  virtual Status EnableBreakpoint(lldb::addr_t addr);
-
-  virtual Status DisableBreakpoint(lldb::addr_t addr);
-
   //----------------------------------------------------------------------
   // Hardware Breakpoint functions
   //----------------------------------------------------------------------
@@ -402,6 +398,13 @@ public:
   }
 
 protected:
+  struct SoftwareBreakpoint {
+    uint32_t ref_count;
+    llvm::SmallVector<uint8_t, 4> saved_opcodes;
+    llvm::ArrayRef<uint8_t> breakpoint_opcodes;
+  };
+
+  std::unordered_map<lldb::addr_t, SoftwareBreakpoint> m_software_breakpoints;
   lldb::pid_t m_pid;
 
   std::vector<std::unique_ptr<NativeThreadProtocol>> m_threads;
@@ -415,7 +418,6 @@ protected:
 
   std::recursive_mutex m_delegates_mutex;
   std::vector<NativeDelegate *> m_delegates;
-  NativeBreakpointList m_breakpoint_list;
   NativeWatchpointList m_watchpoint_list;
   HardwareBreakpointMap m_hw_breakpoints_map;
   int m_terminal_fd;
@@ -446,7 +448,9 @@ protected:
   // ----------------------------------------------------------- Internal
   // interface for software breakpoints
   // -----------------------------------------------------------
+
   Status SetSoftwareBreakpoint(lldb::addr_t addr, uint32_t size_hint);
+  Status RemoveSoftwareBreakpoint(lldb::addr_t addr);
 
   virtual llvm::Expected<llvm::ArrayRef<uint8_t>>
   GetSoftwareBreakpointTrapOpcode(size_t size_hint);
@@ -474,6 +478,8 @@ protected:
 
 private:
   void SynchronouslyNotifyProcessStateChanged(lldb::StateType state);
+  llvm::Expected<SoftwareBreakpoint>
+  EnableSoftwareBreakpoint(lldb::addr_t addr, uint32_t size_hint);
 };
 } // namespace lldb_private
 

Removed: lldb/trunk/include/lldb/Host/common/SoftwareBreakpoint.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/common/SoftwareBreakpoint.h?rev=346092&view=auto
==============================================================================
--- lldb/trunk/include/lldb/Host/common/SoftwareBreakpoint.h (original)
+++ lldb/trunk/include/lldb/Host/common/SoftwareBreakpoint.h (removed)
@@ -1,53 +0,0 @@
-//===-- SoftwareBreakpoint.h ------------------------------------*- C++ -*-===//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef liblldb_SoftwareBreakpoint_h_
-#define liblldb_SoftwareBreakpoint_h_
-
-#include "NativeBreakpoint.h"
-#include "lldb/lldb-private-forward.h"
-
-namespace lldb_private {
-class SoftwareBreakpoint : public NativeBreakpoint {
-  friend class NativeBreakpointList;
-
-public:
-  static Status CreateSoftwareBreakpoint(NativeProcessProtocol &process,
-                                         lldb::addr_t addr, size_t size_hint,
-                                         NativeBreakpointSP &breakpoint_spn);
-
-  SoftwareBreakpoint(NativeProcessProtocol &process, lldb::addr_t addr,
-                     const uint8_t *saved_opcodes, const uint8_t *trap_opcodes,
-                     size_t opcode_size);
-
-protected:
-  Status DoEnable() override;
-
-  Status DoDisable() override;
-
-  bool IsSoftwareBreakpoint() const override;
-
-private:
-  /// Max number of bytes that a software trap opcode sequence can occupy.
-  static const size_t MAX_TRAP_OPCODE_SIZE = 8;
-
-  NativeProcessProtocol &m_process;
-  uint8_t m_saved_opcodes[MAX_TRAP_OPCODE_SIZE];
-  uint8_t m_trap_opcodes[MAX_TRAP_OPCODE_SIZE];
-  const size_t m_opcode_size;
-
-  static Status EnableSoftwareBreakpoint(NativeProcessProtocol &process,
-                                         lldb::addr_t addr,
-                                         size_t bp_opcode_size,
-                                         const uint8_t *bp_opcode_bytes,
-                                         uint8_t *saved_opcode_bytes);
-};
-}
-
-#endif // #ifndef liblldb_SoftwareBreakpoint_h_

Modified: lldb/trunk/include/lldb/lldb-private-forward.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/lldb-private-forward.h?rev=346093&r1=346092&r2=346093&view=diff
==============================================================================
--- lldb/trunk/include/lldb/lldb-private-forward.h (original)
+++ lldb/trunk/include/lldb/lldb-private-forward.h Sun Nov  4 02:58:08 2018
@@ -10,26 +10,15 @@
 #ifndef LLDB_lldb_private_forward_h_
 #define LLDB_lldb_private_forward_h_
 
-#if defined(__cplusplus)
-
-#include <memory>
-
 namespace lldb_private {
 // --------------------------------------------------------------- Class
 // forward decls.
 // ---------------------------------------------------------------
-class NativeBreakpoint;
-class NativeBreakpointList;
 class NativeProcessProtocol;
 class NativeRegisterContext;
 class NativeThreadProtocol;
 class ResumeActionList;
 class UnixSignals;
-
-// --------------------------------------------------------------- SP/WP decls.
-// ---------------------------------------------------------------
-typedef std::shared_ptr<NativeBreakpoint> NativeBreakpointSP;
 }
 
-#endif // #if defined(__cplusplus)
 #endif // #ifndef LLDB_lldb_private_forward_h_

Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/project.pbxproj?rev=346093&r1=346092&r2=346093&view=diff
==============================================================================
--- lldb/trunk/lldb.xcodeproj/project.pbxproj (original)
+++ lldb/trunk/lldb.xcodeproj/project.pbxproj Sun Nov  4 02:58:08 2018
@@ -523,8 +523,6 @@
 		3F81691A1ABA2419001DA9DF /* NameMatches.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 3F8169181ABA2419001DA9DF /* NameMatches.cpp */; };
 		9A3D43D81F3151C400EB767C /* NameMatchesTest.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 9A3D43CB1F3150D200EB767C /* NameMatchesTest.cpp */; };
 		268900C913353E5F00698AC0 /* NameToDIE.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 2618D9EA12406FE600F2B8FE /* NameToDIE.cpp */; };
-		232CB615191E00CD00EF39FC /* NativeBreakpoint.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 232CB60B191E00CC00EF39FC /* NativeBreakpoint.cpp */; };
-		232CB617191E00CD00EF39FC /* NativeBreakpointList.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 232CB60D191E00CC00EF39FC /* NativeBreakpointList.cpp */; };
 		232CB619191E00CD00EF39FC /* NativeProcessProtocol.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 232CB60F191E00CC00EF39FC /* NativeProcessProtocol.cpp */; };
 		267A47FB1B1411C40021A5BC /* NativeRegisterContext.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 267A47FA1B1411C40021A5BC /* NativeRegisterContext.cpp */; };
 		23F4034D1926E0F60046DC9B /* NativeRegisterContextRegisterInfo.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 23F403481926CC250046DC9B /* NativeRegisterContextRegisterInfo.cpp */; };
@@ -870,7 +868,6 @@
 		26D7E45D13D5E30A007FD12B /* SocketAddress.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 26D7E45C13D5E30A007FD12B /* SocketAddress.cpp */; };
 		23CB15451D66DA9300EDDDE1 /* SocketAddressTest.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 2321F9391BDD332400BA9A93 /* SocketAddressTest.cpp */; };
 		23CB153B1D66DA9300EDDDE1 /* SocketTest.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 2321F93A1BDD332400BA9A93 /* SocketTest.cpp */; };
-		232CB61D191E00CD00EF39FC /* SoftwareBreakpoint.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 232CB613191E00CC00EF39FC /* SoftwareBreakpoint.cpp */; };
 		26603879211CA90F00329572 /* SourceBreakpoint.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 26603870211CA90D00329572 /* SourceBreakpoint.cpp */; };
 		2689004C13353E0400698AC0 /* SourceManager.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 26BC7E8F10F1B85900F91463 /* SourceManager.cpp */; };
 		268900F313353E6F00698AC0 /* StackFrame.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 26BC7F3810F1B90C00F91463 /* StackFrame.cpp */; };
@@ -2299,9 +2296,6 @@
 		9A3D43CB1F3150D200EB767C /* NameMatchesTest.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = NameMatchesTest.cpp; sourceTree = "<group>"; };
 		2618D9EA12406FE600F2B8FE /* NameToDIE.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = NameToDIE.cpp; sourceTree = "<group>"; };
 		2618D957124056C700F2B8FE /* NameToDIE.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = NameToDIE.h; sourceTree = "<group>"; };
-		232CB60B191E00CC00EF39FC /* NativeBreakpoint.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = NativeBreakpoint.cpp; path = source/Host/common/NativeBreakpoint.cpp; sourceTree = "<group>"; };
-		267A47F31B14116E0021A5BC /* NativeBreakpoint.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = NativeBreakpoint.h; path = include/lldb/Host/common/NativeBreakpoint.h; sourceTree = "<group>"; };
-		232CB60D191E00CC00EF39FC /* NativeBreakpointList.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = NativeBreakpointList.cpp; path = source/Host/common/NativeBreakpointList.cpp; sourceTree = "<group>"; };
 		267A47F41B1411750021A5BC /* NativeBreakpointList.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = NativeBreakpointList.h; path = include/lldb/Host/common/NativeBreakpointList.h; sourceTree = "<group>"; };
 		232CB60F191E00CC00EF39FC /* NativeProcessProtocol.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; lineEnding = 0; name = NativeProcessProtocol.cpp; path = source/Host/common/NativeProcessProtocol.cpp; sourceTree = "<group>"; xcLanguageSpecificationIdentifier = xcode.lang.cpp; };
 		267A47F51B14117F0021A5BC /* NativeProcessProtocol.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = NativeProcessProtocol.h; path = include/lldb/Host/common/NativeProcessProtocol.h; sourceTree = "<group>"; };
@@ -2925,8 +2919,6 @@
 		26D7E45B13D5E2F9007FD12B /* SocketAddress.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SocketAddress.h; path = include/lldb/Host/SocketAddress.h; sourceTree = "<group>"; };
 		2321F9391BDD332400BA9A93 /* SocketAddressTest.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = SocketAddressTest.cpp; sourceTree = "<group>"; };
 		2321F93A1BDD332400BA9A93 /* SocketTest.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = SocketTest.cpp; sourceTree = "<group>"; };
-		232CB613191E00CC00EF39FC /* SoftwareBreakpoint.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = SoftwareBreakpoint.cpp; path = source/Host/common/SoftwareBreakpoint.cpp; sourceTree = "<group>"; };
-		267A47F21B14115A0021A5BC /* SoftwareBreakpoint.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SoftwareBreakpoint.h; path = include/lldb/Host/common/SoftwareBreakpoint.h; sourceTree = "<group>"; };
 		26603870211CA90D00329572 /* SourceBreakpoint.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = SourceBreakpoint.cpp; path = "tools/lldb-vscode/SourceBreakpoint.cpp"; sourceTree = "<group>"; };
 		2660386D211CA90C00329572 /* SourceBreakpoint.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = SourceBreakpoint.h; path = "tools/lldb-vscode/SourceBreakpoint.h"; sourceTree = "<group>"; };
 		26BC7E8F10F1B85900F91463 /* SourceManager.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = SourceManager.cpp; path = source/Core/SourceManager.cpp; sourceTree = "<group>"; };
@@ -5486,10 +5478,7 @@
 				3FDFED2119BA6D55009756A7 /* HostNativeThreadBase.h */,
 				3FDFE57419AFABFD009756A7 /* HostProcess.h */,
 				3FDFE57519AFABFD009756A7 /* HostThread.h */,
-				267A47F31B14116E0021A5BC /* NativeBreakpoint.h */,
-				232CB60B191E00CC00EF39FC /* NativeBreakpoint.cpp */,
 				267A47F41B1411750021A5BC /* NativeBreakpointList.h */,
-				232CB60D191E00CC00EF39FC /* NativeBreakpointList.cpp */,
 				267A47F51B14117F0021A5BC /* NativeProcessProtocol.h */,
 				232CB60F191E00CC00EF39FC /* NativeProcessProtocol.cpp */,
 				267A47F61B14118F0021A5BC /* NativeRegisterContext.h */,
@@ -5508,8 +5497,6 @@
 				236124A71986B50E004EFC37 /* Socket.h */,
 				26D7E45B13D5E2F9007FD12B /* SocketAddress.h */,
 				26D7E45C13D5E30A007FD12B /* SocketAddress.cpp */,
-				267A47F21B14115A0021A5BC /* SoftwareBreakpoint.h */,
-				232CB613191E00CC00EF39FC /* SoftwareBreakpoint.cpp */,
 				2689B0A4113EE3CD00A4AEDB /* Symbols.h */,
 				6DEC6F3A1BD66D950091ABA6 /* TaskPool.h */,
 				6DEC6F381BD66D750091ABA6 /* TaskPool.cpp */,
@@ -8277,11 +8264,9 @@
 				2689010113353E6F00698AC0 /* ThreadPlanStepOut.cpp in Sources */,
 				2689010213353E6F00698AC0 /* ThreadPlanStepOverBreakpoint.cpp in Sources */,
 				3FDFED2919BA6D96009756A7 /* ThreadLauncher.cpp in Sources */,
-				232CB617191E00CD00EF39FC /* NativeBreakpointList.cpp in Sources */,
 				4C719395207D235400FDF430 /* OptionArgParser.cpp in Sources */,
 				942829561A89614C00521B30 /* JSON.cpp in Sources */,
 				267F68531CC02E920086832B /* RegisterContextLinux_s390x.cpp in Sources */,
-				232CB615191E00CD00EF39FC /* NativeBreakpoint.cpp in Sources */,
 				2689010313353E6F00698AC0 /* ThreadPlanStepRange.cpp in Sources */,
 				2689010413353E6F00698AC0 /* ThreadPlanStepInRange.cpp in Sources */,
 				2689010513353E6F00698AC0 /* ThreadPlanStepOverRange.cpp in Sources */,
@@ -8369,7 +8354,6 @@
 				26F006561B4DD86700B872E5 /* DynamicLoaderWindowsDYLD.cpp in Sources */,
 				26DB3E1C1379E7AD0080DC73 /* ABIMacOSX_i386.cpp in Sources */,
 				26DB3E1F1379E7AD0080DC73 /* ABISysV_x86_64.cpp in Sources */,
-				232CB61D191E00CD00EF39FC /* SoftwareBreakpoint.cpp in Sources */,
 				26A69C5F137A17A500262477 /* RegisterValue.cpp in Sources */,
 				2690B3711381D5C300ECFBAE /* Memory.cpp in Sources */,
 				4C562CC71CC07DF700C52EAC /* PDBASTParser.cpp in Sources */,

Modified: lldb/trunk/source/Host/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/CMakeLists.txt?rev=346093&r1=346092&r2=346093&view=diff
==============================================================================
--- lldb/trunk/source/Host/CMakeLists.txt (original)
+++ lldb/trunk/source/Host/CMakeLists.txt Sun Nov  4 02:58:08 2018
@@ -28,8 +28,6 @@ add_host_subdirectory(common
   common/LockFileBase.cpp
   common/MainLoop.cpp
   common/MonitoringProcessLauncher.cpp
-  common/NativeBreakpoint.cpp
-  common/NativeBreakpointList.cpp
   common/NativeWatchpointList.cpp
   common/NativeProcessProtocol.cpp
   common/NativeRegisterContext.cpp
@@ -40,7 +38,6 @@ add_host_subdirectory(common
   common/PseudoTerminal.cpp
   common/Socket.cpp
   common/SocketAddress.cpp
-  common/SoftwareBreakpoint.cpp
   common/StringConvert.cpp
   common/Symbols.cpp
   common/TaskPool.cpp

Removed: lldb/trunk/source/Host/common/NativeBreakpoint.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/NativeBreakpoint.cpp?rev=346092&view=auto
==============================================================================
--- lldb/trunk/source/Host/common/NativeBreakpoint.cpp (original)
+++ lldb/trunk/source/Host/common/NativeBreakpoint.cpp (removed)
@@ -1,109 +0,0 @@
-//===-- NativeBreakpoint.cpp ------------------------------------*- C++ -*-===//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-
-#include "lldb/Host/common/NativeBreakpoint.h"
-
-#include "lldb/Utility/Log.h"
-#include "lldb/Utility/Status.h"
-#include "lldb/lldb-defines.h"
-
-using namespace lldb_private;
-
-NativeBreakpoint::NativeBreakpoint(lldb::addr_t addr)
-    : m_addr(addr), m_ref_count(1), m_enabled(true) {
-  assert(addr != LLDB_INVALID_ADDRESS && "breakpoint set for invalid address");
-}
-
-NativeBreakpoint::~NativeBreakpoint() {}
-
-void NativeBreakpoint::AddRef() {
-  ++m_ref_count;
-
-  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_BREAKPOINTS));
-  if (log)
-    log->Printf("NativeBreakpoint::%s addr = 0x%" PRIx64
-                " bumped up, new ref count %" PRIu32,
-                __FUNCTION__, m_addr, m_ref_count);
-}
-
-int32_t NativeBreakpoint::DecRef() {
-  --m_ref_count;
-
-  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_BREAKPOINTS));
-  if (log)
-    log->Printf("NativeBreakpoint::%s addr = 0x%" PRIx64
-                " ref count decremented, new ref count %" PRIu32,
-                __FUNCTION__, m_addr, m_ref_count);
-
-  return m_ref_count;
-}
-
-Status NativeBreakpoint::Enable() {
-  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_BREAKPOINTS));
-
-  if (m_enabled) {
-    // We're already enabled. Just log and exit.
-    if (log)
-      log->Printf("NativeBreakpoint::%s addr = 0x%" PRIx64
-                  " already enabled, ignoring.",
-                  __FUNCTION__, m_addr);
-    return Status();
-  }
-
-  // Log and enable.
-  if (log)
-    log->Printf("NativeBreakpoint::%s addr = 0x%" PRIx64 " enabling...",
-                __FUNCTION__, m_addr);
-
-  Status error = DoEnable();
-  if (error.Success()) {
-    m_enabled = true;
-    if (log)
-      log->Printf("NativeBreakpoint::%s addr = 0x%" PRIx64 " enable SUCCESS.",
-                  __FUNCTION__, m_addr);
-  } else {
-    if (log)
-      log->Printf("NativeBreakpoint::%s addr = 0x%" PRIx64 " enable FAIL: %s",
-                  __FUNCTION__, m_addr, error.AsCString());
-  }
-
-  return error;
-}
-
-Status NativeBreakpoint::Disable() {
-  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_BREAKPOINTS));
-
-  if (!m_enabled) {
-    // We're already disabled. Just log and exit.
-    if (log)
-      log->Printf("NativeBreakpoint::%s addr = 0x%" PRIx64
-                  " already disabled, ignoring.",
-                  __FUNCTION__, m_addr);
-    return Status();
-  }
-
-  // Log and disable.
-  if (log)
-    log->Printf("NativeBreakpoint::%s addr = 0x%" PRIx64 " disabling...",
-                __FUNCTION__, m_addr);
-
-  Status error = DoDisable();
-  if (error.Success()) {
-    m_enabled = false;
-    if (log)
-      log->Printf("NativeBreakpoint::%s addr = 0x%" PRIx64 " disable SUCCESS.",
-                  __FUNCTION__, m_addr);
-  } else {
-    if (log)
-      log->Printf("NativeBreakpoint::%s addr = 0x%" PRIx64 " disable FAIL: %s",
-                  __FUNCTION__, m_addr, error.AsCString());
-  }
-
-  return error;
-}

Removed: lldb/trunk/source/Host/common/NativeBreakpointList.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/NativeBreakpointList.cpp?rev=346092&view=auto
==============================================================================
--- lldb/trunk/source/Host/common/NativeBreakpointList.cpp (original)
+++ lldb/trunk/source/Host/common/NativeBreakpointList.cpp (removed)
@@ -1,240 +0,0 @@
-//===-- NativeBreakpointList.cpp --------------------------------*- C++ -*-===//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-
-#include "lldb/Host/common/NativeBreakpointList.h"
-
-#include "lldb/Utility/Log.h"
-
-#include "lldb/Host/common/NativeBreakpoint.h"
-#include "lldb/Host/common/SoftwareBreakpoint.h"
-
-using namespace lldb;
-using namespace lldb_private;
-
-NativeBreakpointList::NativeBreakpointList() : m_mutex() {}
-
-Status NativeBreakpointList::AddRef(lldb::addr_t addr, size_t size_hint,
-                                    bool hardware,
-                                    CreateBreakpointFunc create_func) {
-  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_BREAKPOINTS));
-  if (log)
-    log->Printf("NativeBreakpointList::%s addr = 0x%" PRIx64
-                ", size_hint = %zu, hardware = %s",
-                __FUNCTION__, addr, size_hint, hardware ? "true" : "false");
-
-  std::lock_guard<std::recursive_mutex> guard(m_mutex);
-
-  // Check if the breakpoint is already set.
-  auto iter = m_breakpoints.find(addr);
-  if (iter != m_breakpoints.end()) {
-    // Yes - bump up ref count.
-    if (log)
-      log->Printf("NativeBreakpointList::%s addr = 0x%" PRIx64
-                  " -- already enabled, upping ref count",
-                  __FUNCTION__, addr);
-
-    iter->second->AddRef();
-    return Status();
-  }
-
-  // Create a new breakpoint using the given create func.
-  if (log)
-    log->Printf(
-        "NativeBreakpointList::%s creating breakpoint for addr = 0x%" PRIx64
-        ", size_hint = %zu, hardware = %s",
-        __FUNCTION__, addr, size_hint, hardware ? "true" : "false");
-
-  NativeBreakpointSP breakpoint_sp;
-  Status error = create_func(addr, size_hint, hardware, breakpoint_sp);
-  if (error.Fail()) {
-    if (log)
-      log->Printf(
-          "NativeBreakpointList::%s creating breakpoint for addr = 0x%" PRIx64
-          ", size_hint = %zu, hardware = %s -- FAILED: %s",
-          __FUNCTION__, addr, size_hint, hardware ? "true" : "false",
-          error.AsCString());
-    return error;
-  }
-
-  // Remember the breakpoint.
-  assert(breakpoint_sp && "NativeBreakpoint create function succeeded but "
-                          "returned NULL breakpoint");
-  m_breakpoints.insert(BreakpointMap::value_type(addr, breakpoint_sp));
-
-  return error;
-}
-
-Status NativeBreakpointList::DecRef(lldb::addr_t addr) {
-  Status error;
-
-  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_BREAKPOINTS));
-  if (log)
-    log->Printf("NativeBreakpointList::%s addr = 0x%" PRIx64, __FUNCTION__,
-                addr);
-
-  std::lock_guard<std::recursive_mutex> guard(m_mutex);
-
-  // Check if the breakpoint is already set.
-  auto iter = m_breakpoints.find(addr);
-  if (iter == m_breakpoints.end()) {
-    // Not found!
-    if (log)
-      log->Printf("NativeBreakpointList::%s addr = 0x%" PRIx64 " -- NOT FOUND",
-                  __FUNCTION__, addr);
-    error.SetErrorString("breakpoint not found");
-    return error;
-  }
-
-  // Decrement ref count.
-  const int32_t new_ref_count = iter->second->DecRef();
-  assert(new_ref_count >= 0 && "NativeBreakpoint ref count went negative");
-
-  if (new_ref_count > 0) {
-    // Still references to this breakpoint.  Leave it alone.
-    if (log)
-      log->Printf("NativeBreakpointList::%s addr = 0x%" PRIx64
-                  " -- new breakpoint ref count %" PRIu32,
-                  __FUNCTION__, addr, new_ref_count);
-    return error;
-  }
-
-  // Breakpoint has no more references.  Disable it if it's not already
-  // disabled.
-  if (log)
-    log->Printf("NativeBreakpointList::%s addr = 0x%" PRIx64
-                " -- removing due to no remaining references",
-                __FUNCTION__, addr);
-
-  // If it's enabled, we need to disable it.
-  if (iter->second->IsEnabled()) {
-    if (log)
-      log->Printf("NativeBreakpointList::%s addr = 0x%" PRIx64
-                  " -- currently enabled, now disabling",
-                  __FUNCTION__, addr);
-    error = iter->second->Disable();
-    if (error.Fail()) {
-      if (log)
-        log->Printf("NativeBreakpointList::%s addr = 0x%" PRIx64
-                    " -- removal FAILED: %s",
-                    __FUNCTION__, addr, error.AsCString());
-      // Continue since we still want to take it out of the breakpoint list.
-    }
-  } else {
-    if (log)
-      log->Printf("NativeBreakpointList::%s addr = 0x%" PRIx64
-                  " -- already disabled, nothing to do",
-                  __FUNCTION__, addr);
-  }
-
-  // Take the breakpoint out of the list.
-  if (log)
-    log->Printf("NativeBreakpointList::%s addr = 0x%" PRIx64
-                " -- removed from breakpoint map",
-                __FUNCTION__, addr);
-
-  m_breakpoints.erase(iter);
-  return error;
-}
-
-Status NativeBreakpointList::EnableBreakpoint(lldb::addr_t addr) {
-  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_BREAKPOINTS));
-  if (log)
-    log->Printf("NativeBreakpointList::%s addr = 0x%" PRIx64, __FUNCTION__,
-                addr);
-
-  std::lock_guard<std::recursive_mutex> guard(m_mutex);
-
-  // Ensure we have said breakpoint.
-  auto iter = m_breakpoints.find(addr);
-  if (iter == m_breakpoints.end()) {
-    // Not found!
-    if (log)
-      log->Printf("NativeBreakpointList::%s addr = 0x%" PRIx64 " -- NOT FOUND",
-                  __FUNCTION__, addr);
-    return Status("breakpoint not found");
-  }
-
-  // Enable it.
-  return iter->second->Enable();
-}
-
-Status NativeBreakpointList::DisableBreakpoint(lldb::addr_t addr) {
-  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_BREAKPOINTS));
-  if (log)
-    log->Printf("NativeBreakpointList::%s addr = 0x%" PRIx64, __FUNCTION__,
-                addr);
-
-  std::lock_guard<std::recursive_mutex> guard(m_mutex);
-
-  // Ensure we have said breakpoint.
-  auto iter = m_breakpoints.find(addr);
-  if (iter == m_breakpoints.end()) {
-    // Not found!
-    if (log)
-      log->Printf("NativeBreakpointList::%s addr = 0x%" PRIx64 " -- NOT FOUND",
-                  __FUNCTION__, addr);
-    return Status("breakpoint not found");
-  }
-
-  // Disable it.
-  return iter->second->Disable();
-}
-
-Status NativeBreakpointList::GetBreakpoint(lldb::addr_t addr,
-                                           NativeBreakpointSP &breakpoint_sp) {
-  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_BREAKPOINTS));
-  if (log)
-    log->Printf("NativeBreakpointList::%s addr = 0x%" PRIx64, __FUNCTION__,
-                addr);
-
-  std::lock_guard<std::recursive_mutex> guard(m_mutex);
-
-  // Ensure we have said breakpoint.
-  auto iter = m_breakpoints.find(addr);
-  if (iter == m_breakpoints.end()) {
-    // Not found!
-    breakpoint_sp.reset();
-    return Status("breakpoint not found");
-  }
-
-  // Disable it.
-  breakpoint_sp = iter->second;
-  return Status();
-}
-
-Status NativeBreakpointList::RemoveTrapsFromBuffer(lldb::addr_t addr, void *buf,
-                                                   size_t size) const {
-  auto data = llvm::makeMutableArrayRef(static_cast<uint8_t *>(buf), size);
-  for (const auto &map : m_breakpoints) {
-    const auto &bp_sp = map.second;
-    // Not software breakpoint, ignore
-    if (!bp_sp->IsSoftwareBreakpoint())
-      continue;
-    auto software_bp_sp = std::static_pointer_cast<SoftwareBreakpoint>(bp_sp);
-
-    lldb::addr_t bp_addr = map.first;
-    auto opcode_size = software_bp_sp->m_opcode_size;
-
-    // Breapoint not in range, ignore
-    if (bp_addr + opcode_size < addr || addr + size <= bp_addr)
-      continue;
-
-    auto saved_opcodes =
-        llvm::makeArrayRef(software_bp_sp->m_saved_opcodes, opcode_size);
-    if (bp_addr < addr) {
-      saved_opcodes = saved_opcodes.drop_front(addr - bp_addr);
-      bp_addr = addr;
-    }
-    auto bp_data = data.drop_front(bp_addr - addr);
-    std::copy_n(saved_opcodes.begin(),
-                std::min(saved_opcodes.size(), bp_data.size()),
-                bp_data.begin());
-  }
-  return Status();
-}

Modified: lldb/trunk/source/Host/common/NativeProcessProtocol.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/NativeProcessProtocol.cpp?rev=346093&r1=346092&r2=346093&view=diff
==============================================================================
--- lldb/trunk/source/Host/common/NativeProcessProtocol.cpp (original)
+++ lldb/trunk/source/Host/common/NativeProcessProtocol.cpp Sun Nov  4 02:58:08 2018
@@ -9,9 +9,9 @@
 
 #include "lldb/Host/common/NativeProcessProtocol.h"
 #include "lldb/Host/Host.h"
+#include "lldb/Host/common/NativeBreakpointList.h"
 #include "lldb/Host/common/NativeRegisterContext.h"
 #include "lldb/Host/common/NativeThreadProtocol.h"
-#include "lldb/Host/common/SoftwareBreakpoint.h"
 #include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/State.h"
@@ -359,17 +359,161 @@ void NativeProcessProtocol::NotifyDidExe
 Status NativeProcessProtocol::SetSoftwareBreakpoint(lldb::addr_t addr,
                                                     uint32_t size_hint) {
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_BREAKPOINTS));
-  if (log)
-    log->Printf("NativeProcessProtocol::%s addr = 0x%" PRIx64, __FUNCTION__,
-                addr);
+  LLDB_LOG(log, "addr = {0:x}, size_hint = {1}", addr, size_hint);
+
+  auto it = m_software_breakpoints.find(addr);
+  if (it != m_software_breakpoints.end()) {
+    ++it->second.ref_count;
+    return Status();
+  }
+  auto expected_bkpt = EnableSoftwareBreakpoint(addr, size_hint);
+  if (!expected_bkpt)
+    return Status(expected_bkpt.takeError());
 
-  return m_breakpoint_list.AddRef(
-      addr, size_hint, false,
-      [this](lldb::addr_t addr, size_t size_hint, bool /* hardware */,
-             NativeBreakpointSP &breakpoint_sp) -> Status {
-        return SoftwareBreakpoint::CreateSoftwareBreakpoint(
-            *this, addr, size_hint, breakpoint_sp);
-      });
+  m_software_breakpoints.emplace(addr, std::move(*expected_bkpt));
+  return Status();
+}
+
+Status NativeProcessProtocol::RemoveSoftwareBreakpoint(lldb::addr_t addr) {
+  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_BREAKPOINTS));
+  LLDB_LOG(log, "addr = {0:x}", addr);
+  auto it = m_software_breakpoints.find(addr);
+  if (it == m_software_breakpoints.end())
+    return Status("Breakpoint not found.");
+  assert(it->second.ref_count > 0);
+  if (--it->second.ref_count > 0)
+    return Status();
+
+  // This is the last reference. Let's remove the breakpoint.
+  Status error;
+
+  // Clear a software breakpoint instruction
+  llvm::SmallVector<uint8_t, 4> curr_break_op(
+      it->second.breakpoint_opcodes.size(), 0);
+
+  // Read the breakpoint opcode
+  size_t bytes_read = 0;
+  error =
+      ReadMemory(addr, curr_break_op.data(), curr_break_op.size(), bytes_read);
+  if (error.Fail() || bytes_read < curr_break_op.size()) {
+    return Status("addr=0x%" PRIx64
+                  ": tried to read %zu bytes but only read %zu",
+                  addr, curr_break_op.size(), bytes_read);
+  }
+  const auto &saved = it->second.saved_opcodes;
+  // Make sure the breakpoint opcode exists at this address
+  if (makeArrayRef(curr_break_op) != it->second.breakpoint_opcodes) {
+    if (curr_break_op != it->second.saved_opcodes)
+      return Status("Original breakpoint trap is no longer in memory.");
+    LLDB_LOG(log,
+             "Saved opcodes ({0:@[x]}) have already been restored at {1:x}.",
+             llvm::make_range(saved.begin(), saved.end()), addr);
+  } else {
+    // We found a valid breakpoint opcode at this address, now restore the
+    // saved opcode.
+    size_t bytes_written = 0;
+    error = WriteMemory(addr, saved.data(), saved.size(), bytes_written);
+    if (error.Fail() || bytes_written < saved.size()) {
+      return Status("addr=0x%" PRIx64
+                    ": tried to write %zu bytes but only wrote %zu",
+                    addr, saved.size(), bytes_written);
+    }
+
+    // Verify that our original opcode made it back to the inferior
+    llvm::SmallVector<uint8_t, 4> verify_opcode(saved.size(), 0);
+    size_t verify_bytes_read = 0;
+    error = ReadMemory(addr, verify_opcode.data(), verify_opcode.size(),
+                       verify_bytes_read);
+    if (error.Fail() || verify_bytes_read < verify_opcode.size()) {
+      return Status("addr=0x%" PRIx64
+                    ": tried to read %zu verification bytes but only read %zu",
+                    addr, verify_opcode.size(), verify_bytes_read);
+    }
+    if (verify_opcode != saved)
+      LLDB_LOG(log, "Restoring bytes at {0:x}: {1:@[x]}", addr,
+               llvm::make_range(saved.begin(), saved.end()));
+  }
+
+  m_software_breakpoints.erase(it);
+  return Status();
+}
+
+llvm::Expected<NativeProcessProtocol::SoftwareBreakpoint>
+NativeProcessProtocol::EnableSoftwareBreakpoint(lldb::addr_t addr,
+                                                uint32_t size_hint) {
+  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_BREAKPOINTS));
+
+  auto expected_trap = GetSoftwareBreakpointTrapOpcode(size_hint);
+  if (!expected_trap)
+    return expected_trap.takeError();
+
+  llvm::SmallVector<uint8_t, 4> saved_opcode_bytes(expected_trap->size(), 0);
+  // Save the original opcodes by reading them so we can restore later.
+  size_t bytes_read = 0;
+  Status error = ReadMemory(addr, saved_opcode_bytes.data(),
+                            saved_opcode_bytes.size(), bytes_read);
+  if (error.Fail())
+    return error.ToError();
+
+  // Ensure we read as many bytes as we expected.
+  if (bytes_read != saved_opcode_bytes.size()) {
+    return llvm::createStringError(
+        llvm::inconvertibleErrorCode(),
+        "Failed to read memory while attempting to set breakpoint: attempted "
+        "to read {0} bytes but only read {1}.",
+        saved_opcode_bytes.size(), bytes_read);
+  }
+
+  LLDB_LOG(
+      log, "Overwriting bytes at {0:x}: {1:@[x]}", addr,
+      llvm::make_range(saved_opcode_bytes.begin(), saved_opcode_bytes.end()));
+
+  // Write a software breakpoint in place of the original opcode.
+  size_t bytes_written = 0;
+  error = WriteMemory(addr, expected_trap->data(), expected_trap->size(),
+                      bytes_written);
+  if (error.Fail())
+    return error.ToError();
+
+  // Ensure we wrote as many bytes as we expected.
+  if (bytes_written != expected_trap->size()) {
+    return llvm::createStringError(
+        llvm::inconvertibleErrorCode(),
+        "Failed write memory while attempting to set "
+        "breakpoint: attempted to write {0} bytes but only wrote {1}",
+        expected_trap->size(), bytes_written);
+  }
+
+  llvm::SmallVector<uint8_t, 4> verify_bp_opcode_bytes(expected_trap->size(),
+                                                       0);
+  size_t verify_bytes_read = 0;
+  error = ReadMemory(addr, verify_bp_opcode_bytes.data(),
+                     verify_bp_opcode_bytes.size(), verify_bytes_read);
+  if (error.Fail())
+    return error.ToError();
+
+  // Ensure we read as many verification bytes as we expected.
+  if (verify_bytes_read != verify_bp_opcode_bytes.size()) {
+    return llvm::createStringError(
+        llvm::inconvertibleErrorCode(),
+        "Failed to read memory while "
+        "attempting to verify breakpoint: attempted to read {0} bytes "
+        "but only read {1}",
+        verify_bp_opcode_bytes.size(), verify_bytes_read);
+  }
+
+  if (llvm::makeArrayRef(verify_bp_opcode_bytes.data(), verify_bytes_read) !=
+      *expected_trap) {
+    return llvm::createStringError(
+        llvm::inconvertibleErrorCode(),
+        "Verification of software breakpoint "
+        "writing failed - trap opcodes not successfully read back "
+        "after writing when setting breakpoint at {0:x}",
+        addr);
+  }
+
+  LLDB_LOG(log, "addr = {0:x}: SUCCESS");
+  return SoftwareBreakpoint{1, saved_opcode_bytes, *expected_trap};
 }
 
 llvm::Expected<llvm::ArrayRef<uint8_t>>
@@ -455,27 +599,15 @@ void NativeProcessProtocol::FixupBreakpo
   if (breakpoint_addr >= breakpoint_size)
     breakpoint_addr -= breakpoint_size;
 
-  // Check if we stopped because of a breakpoint.
-  NativeBreakpointSP breakpoint_sp;
-  error = m_breakpoint_list.GetBreakpoint(breakpoint_addr, breakpoint_sp);
-  if (!error.Success() || !breakpoint_sp) {
+  if (m_software_breakpoints.count(breakpoint_addr) == 0) {
     // We didn't find one at a software probe location.  Nothing to do.
     LLDB_LOG(log,
-             "pid {0} no lldb breakpoint found at current pc with "
+             "pid {0} no lldb software breakpoint found at current pc with "
              "adjustment: {1}",
              GetID(), breakpoint_addr);
     return;
   }
 
-  // If the breakpoint is not a software breakpoint, nothing to do.
-  if (!breakpoint_sp->IsSoftwareBreakpoint()) {
-    LLDB_LOG(
-        log,
-        "pid {0} breakpoint found at {1:x}, not software, nothing to adjust",
-        GetID(), breakpoint_addr);
-    return;
-  }
-
   //
   // We have a software breakpoint and need to adjust the PC.
   //
@@ -499,20 +631,7 @@ Status NativeProcessProtocol::RemoveBrea
   if (hardware)
     return RemoveHardwareBreakpoint(addr);
   else
-    return m_breakpoint_list.DecRef(addr);
-}
-
-Status NativeProcessProtocol::EnableBreakpoint(lldb::addr_t addr) {
-  return m_breakpoint_list.EnableBreakpoint(addr);
-}
-
-Status NativeProcessProtocol::DisableBreakpoint(lldb::addr_t addr) {
-  return m_breakpoint_list.DisableBreakpoint(addr);
-}
-
-lldb::StateType NativeProcessProtocol::GetState() const {
-  std::lock_guard<std::recursive_mutex> guard(m_state_mutex);
-  return m_state;
+    return RemoveSoftwareBreakpoint(addr);
 }
 
 Status NativeProcessProtocol::ReadMemoryWithoutTrap(lldb::addr_t addr,
@@ -521,7 +640,31 @@ Status NativeProcessProtocol::ReadMemory
   Status error = ReadMemory(addr, buf, size, bytes_read);
   if (error.Fail())
     return error;
-  return m_breakpoint_list.RemoveTrapsFromBuffer(addr, buf, size);
+
+  auto data =
+      llvm::makeMutableArrayRef(static_cast<uint8_t *>(buf), bytes_read);
+  for (const auto &pair : m_software_breakpoints) {
+    lldb::addr_t bp_addr = pair.first;
+    auto saved_opcodes = makeArrayRef(pair.second.saved_opcodes);
+
+    if (bp_addr + saved_opcodes.size() < addr || addr + bytes_read <= bp_addr)
+      continue; // Breapoint not in range, ignore
+
+    if (bp_addr < addr) {
+      saved_opcodes = saved_opcodes.drop_front(addr - bp_addr);
+      bp_addr = addr;
+    }
+    auto bp_data = data.drop_front(bp_addr - addr);
+    std::copy_n(saved_opcodes.begin(),
+                std::min(saved_opcodes.size(), bp_data.size()),
+                bp_data.begin());
+  }
+  return Status();
+}
+
+lldb::StateType NativeProcessProtocol::GetState() const {
+  std::lock_guard<std::recursive_mutex> guard(m_state_mutex);
+  return m_state;
 }
 
 void NativeProcessProtocol::SetState(lldb::StateType state,

Modified: lldb/trunk/source/Host/common/NativeThreadProtocol.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/NativeThreadProtocol.cpp?rev=346093&r1=346092&r2=346093&view=diff
==============================================================================
--- lldb/trunk/source/Host/common/NativeThreadProtocol.cpp (original)
+++ lldb/trunk/source/Host/common/NativeThreadProtocol.cpp Sun Nov  4 02:58:08 2018
@@ -11,7 +11,6 @@
 
 #include "lldb/Host/common/NativeProcessProtocol.h"
 #include "lldb/Host/common/NativeRegisterContext.h"
-#include "lldb/Host/common/SoftwareBreakpoint.h"
 
 using namespace lldb;
 using namespace lldb_private;

Removed: lldb/trunk/source/Host/common/SoftwareBreakpoint.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/SoftwareBreakpoint.cpp?rev=346092&view=auto
==============================================================================
--- lldb/trunk/source/Host/common/SoftwareBreakpoint.cpp (original)
+++ lldb/trunk/source/Host/common/SoftwareBreakpoint.cpp (removed)
@@ -1,313 +0,0 @@
-//===-- SoftwareBreakpoint.cpp ----------------------------------*- C++ -*-===//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-
-#include "lldb/Host/common/SoftwareBreakpoint.h"
-
-#include "lldb/Host/Debug.h"
-#include "lldb/Utility/Log.h"
-#include "lldb/Utility/Status.h"
-
-#include "lldb/Host/common/NativeProcessProtocol.h"
-
-using namespace lldb_private;
-
-// ------------------------------------------------------------------- static
-// members -------------------------------------------------------------------
-
-Status SoftwareBreakpoint::CreateSoftwareBreakpoint(
-    NativeProcessProtocol &process, lldb::addr_t addr, size_t size_hint,
-    NativeBreakpointSP &breakpoint_sp) {
-  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_BREAKPOINTS));
-  if (log)
-    log->Printf("SoftwareBreakpoint::%s addr = 0x%" PRIx64, __FUNCTION__, addr);
-
-  // Validate the address.
-  if (addr == LLDB_INVALID_ADDRESS)
-    return Status("SoftwareBreakpoint::%s invalid load address specified.",
-                  __FUNCTION__);
-
-  // 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());
-
-  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];
-  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 "
-                  "0x%" PRIx64 ": %s",
-                  __FUNCTION__, addr, error.AsCString());
-    return error;
-  }
-
-  if (log)
-    log->Printf("SoftwareBreakpoint::%s addr = 0x%" PRIx64 " -- SUCCESS",
-                __FUNCTION__, addr);
-
-  // 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()));
-  return Status();
-}
-
-Status SoftwareBreakpoint::EnableSoftwareBreakpoint(
-    NativeProcessProtocol &process, lldb::addr_t addr, size_t bp_opcode_size,
-    const uint8_t *bp_opcode_bytes, uint8_t *saved_opcode_bytes) {
-  assert(bp_opcode_size <= MAX_TRAP_OPCODE_SIZE &&
-         "bp_opcode_size out of valid range");
-  assert(bp_opcode_bytes && "bp_opcode_bytes is NULL");
-  assert(saved_opcode_bytes && "saved_opcode_bytes is NULL");
-
-  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_BREAKPOINTS));
-  if (log)
-    log->Printf("SoftwareBreakpoint::%s addr = 0x%" PRIx64, __FUNCTION__, addr);
-
-  // Save the original opcodes by reading them so we can restore later.
-  size_t bytes_read = 0;
-
-  Status error =
-      process.ReadMemory(addr, saved_opcode_bytes, bp_opcode_size, bytes_read);
-  if (error.Fail()) {
-    if (log)
-      log->Printf("SoftwareBreakpoint::%s failed to read memory while "
-                  "attempting to set breakpoint: %s",
-                  __FUNCTION__, error.AsCString());
-    return error;
-  }
-
-  // Ensure we read as many bytes as we expected.
-  if (bytes_read != bp_opcode_size) {
-    if (log)
-      log->Printf("SoftwareBreakpoint::%s failed to read memory while "
-                  "attempting to set breakpoint: attempted to read %zu bytes "
-                  "but only read %zu",
-                  __FUNCTION__, bp_opcode_size, bytes_read);
-    return Status("SoftwareBreakpoint::%s failed to read memory while "
-                  "attempting to set breakpoint: attempted to read %zu bytes "
-                  "but only read %zu",
-                  __FUNCTION__, bp_opcode_size, bytes_read);
-  }
-
-  // Log what we read.
-  if (log) {
-    int i = 0;
-    for (const uint8_t *read_byte = saved_opcode_bytes;
-         read_byte < saved_opcode_bytes + bp_opcode_size; ++read_byte) {
-      log->Printf("SoftwareBreakpoint::%s addr = 0x%" PRIx64
-                  " ovewriting byte index %d (was 0x%hhx)",
-                  __FUNCTION__, addr, i++, *read_byte);
-    }
-  }
-
-  // Write a software breakpoint in place of the original opcode.
-  size_t bytes_written = 0;
-  error =
-      process.WriteMemory(addr, bp_opcode_bytes, bp_opcode_size, bytes_written);
-  if (error.Fail()) {
-    if (log)
-      log->Printf("SoftwareBreakpoint::%s failed to write memory while "
-                  "attempting to set breakpoint: %s",
-                  __FUNCTION__, error.AsCString());
-    return error;
-  }
-
-  // Ensure we wrote as many bytes as we expected.
-  if (bytes_written != bp_opcode_size) {
-    error.SetErrorStringWithFormat(
-        "SoftwareBreakpoint::%s failed write memory while attempting to set "
-        "breakpoint: attempted to write %zu bytes but only wrote %zu",
-        __FUNCTION__, bp_opcode_size, bytes_written);
-    if (log)
-      log->PutCString(error.AsCString());
-    return error;
-  }
-
-  uint8_t verify_bp_opcode_bytes[MAX_TRAP_OPCODE_SIZE];
-  size_t verify_bytes_read = 0;
-  error = process.ReadMemory(addr, verify_bp_opcode_bytes, bp_opcode_size,
-                             verify_bytes_read);
-  if (error.Fail()) {
-    if (log)
-      log->Printf("SoftwareBreakpoint::%s failed to read memory while "
-                  "attempting to verify the breakpoint set: %s",
-                  __FUNCTION__, error.AsCString());
-    return error;
-  }
-
-  // Ensure we read as many verification bytes as we expected.
-  if (verify_bytes_read != bp_opcode_size) {
-    if (log)
-      log->Printf("SoftwareBreakpoint::%s failed to read memory while "
-                  "attempting to verify breakpoint: attempted to read %zu "
-                  "bytes but only read %zu",
-                  __FUNCTION__, bp_opcode_size, verify_bytes_read);
-    return Status(
-        "SoftwareBreakpoint::%s failed to read memory while "
-        "attempting to verify breakpoint: attempted to read %zu bytes "
-        "but only read %zu",
-        __FUNCTION__, bp_opcode_size, verify_bytes_read);
-  }
-
-  if (::memcmp(bp_opcode_bytes, verify_bp_opcode_bytes, bp_opcode_size) != 0) {
-    if (log)
-      log->Printf("SoftwareBreakpoint::%s: verification of software breakpoint "
-                  "writing failed - trap opcodes not successfully read back "
-                  "after writing when setting breakpoint at 0x%" PRIx64,
-                  __FUNCTION__, addr);
-    return Status("SoftwareBreakpoint::%s: verification of software breakpoint "
-                  "writing failed - trap opcodes not successfully read back "
-                  "after writing when setting breakpoint at 0x%" PRIx64,
-                  __FUNCTION__, addr);
-  }
-
-  if (log)
-    log->Printf("SoftwareBreakpoint::%s addr = 0x%" PRIx64 " -- SUCCESS",
-                __FUNCTION__, addr);
-
-  return Status();
-}
-
-// -------------------------------------------------------------------
-// instance-level members
-// -------------------------------------------------------------------
-
-SoftwareBreakpoint::SoftwareBreakpoint(NativeProcessProtocol &process,
-                                       lldb::addr_t addr,
-                                       const uint8_t *saved_opcodes,
-                                       const uint8_t *trap_opcodes,
-                                       size_t opcode_size)
-    : NativeBreakpoint(addr), m_process(process), m_saved_opcodes(),
-      m_trap_opcodes(), m_opcode_size(opcode_size) {
-  assert(opcode_size > 0 && "setting software breakpoint with no trap opcodes");
-  assert(opcode_size <= MAX_TRAP_OPCODE_SIZE && "trap opcode size too large");
-
-  ::memcpy(m_saved_opcodes, saved_opcodes, opcode_size);
-  ::memcpy(m_trap_opcodes, trap_opcodes, opcode_size);
-}
-
-Status SoftwareBreakpoint::DoEnable() {
-  return EnableSoftwareBreakpoint(m_process, m_addr, m_opcode_size,
-                                  m_trap_opcodes, m_saved_opcodes);
-}
-
-Status SoftwareBreakpoint::DoDisable() {
-  Status error;
-  assert(m_addr && (m_addr != LLDB_INVALID_ADDRESS) &&
-         "can't remove a software breakpoint for an invalid address");
-
-  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_BREAKPOINTS));
-  if (log)
-    log->Printf("SoftwareBreakpoint::%s addr = 0x%" PRIx64, __FUNCTION__,
-                m_addr);
-
-  assert((m_opcode_size > 0) &&
-         "cannot restore opcodes when there are no opcodes");
-
-  if (m_opcode_size > 0) {
-    // Clear a software breakpoint instruction
-    uint8_t curr_break_op[MAX_TRAP_OPCODE_SIZE];
-    bool break_op_found = false;
-    assert(m_opcode_size <= sizeof(curr_break_op));
-
-    // Read the breakpoint opcode
-    size_t bytes_read = 0;
-    error =
-        m_process.ReadMemory(m_addr, curr_break_op, m_opcode_size, bytes_read);
-    if (error.Success() && bytes_read < m_opcode_size) {
-      error.SetErrorStringWithFormat(
-          "SoftwareBreakpointr::%s addr=0x%" PRIx64
-          ": tried to read %zu bytes but only read %zu",
-          __FUNCTION__, m_addr, m_opcode_size, bytes_read);
-    }
-    if (error.Success()) {
-      bool verify = false;
-      // Make sure the breakpoint opcode exists at this address
-      if (::memcmp(curr_break_op, m_trap_opcodes, m_opcode_size) == 0) {
-        break_op_found = true;
-        // We found a valid breakpoint opcode at this address, now restore the
-        // saved opcode.
-        size_t bytes_written = 0;
-        error = m_process.WriteMemory(m_addr, m_saved_opcodes, m_opcode_size,
-                                      bytes_written);
-        if (error.Success() && bytes_written < m_opcode_size) {
-          error.SetErrorStringWithFormat(
-              "SoftwareBreakpoint::%s addr=0x%" PRIx64
-              ": tried to write %zu bytes but only wrote %zu",
-              __FUNCTION__, m_addr, m_opcode_size, bytes_written);
-        }
-        if (error.Success()) {
-          verify = true;
-        }
-      } else {
-        error.SetErrorString(
-            "Original breakpoint trap is no longer in memory.");
-        // Set verify to true and so we can check if the original opcode has
-        // already been restored
-        verify = true;
-      }
-
-      if (verify) {
-        uint8_t verify_opcode[MAX_TRAP_OPCODE_SIZE];
-        assert(m_opcode_size <= sizeof(verify_opcode));
-        // Verify that our original opcode made it back to the inferior
-
-        size_t verify_bytes_read = 0;
-        error = m_process.ReadMemory(m_addr, verify_opcode, m_opcode_size,
-                                     verify_bytes_read);
-        if (error.Success() && verify_bytes_read < m_opcode_size) {
-          error.SetErrorStringWithFormat(
-              "SoftwareBreakpoint::%s addr=0x%" PRIx64
-              ": tried to read %zu verification bytes but only read %zu",
-              __FUNCTION__, m_addr, m_opcode_size, verify_bytes_read);
-        }
-        if (error.Success()) {
-          // compare the memory we just read with the original opcode
-          if (::memcmp(m_saved_opcodes, verify_opcode, m_opcode_size) == 0) {
-            // SUCCESS
-            if (log) {
-              int i = 0;
-              for (const uint8_t *verify_byte = verify_opcode;
-                   verify_byte < verify_opcode + m_opcode_size; ++verify_byte) {
-                log->Printf("SoftwareBreakpoint::%s addr = 0x%" PRIx64
-                            " replaced byte index %d with 0x%hhx",
-                            __FUNCTION__, m_addr, i++, *verify_byte);
-              }
-              log->Printf("SoftwareBreakpoint::%s addr = 0x%" PRIx64
-                          " -- SUCCESS",
-                          __FUNCTION__, m_addr);
-            }
-            return error;
-          } else {
-            if (break_op_found)
-              error.SetErrorString("Failed to restore original opcode.");
-          }
-        } else
-          error.SetErrorString("Failed to read memory to verify that "
-                               "breakpoint trap was restored.");
-      }
-    }
-  }
-
-  if (log && error.Fail())
-    log->Printf("SoftwareBreakpoint::%s addr = 0x%" PRIx64 " -- FAILED: %s",
-                __FUNCTION__, m_addr, error.AsCString());
-  return error;
-}
-
-bool SoftwareBreakpoint::IsSoftwareBreakpoint() const { return true; }

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=346093&r1=346092&r2=346093&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp Sun Nov  4 02:58:08 2018
@@ -29,7 +29,6 @@
 #include "lldb/Host/HostProcess.h"
 #include "lldb/Host/PseudoTerminal.h"
 #include "lldb/Host/ThreadLauncher.h"
-#include "lldb/Host/common/NativeBreakpoint.h"
 #include "lldb/Host/common/NativeRegisterContext.h"
 #include "lldb/Host/linux/Ptrace.h"
 #include "lldb/Host/linux/Uio.h"

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=346093&r1=346092&r2=346093&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp (original)
+++ lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp Sun Nov  4 02:58:08 2018
@@ -16,7 +16,6 @@
 // Other libraries and framework includes
 #include "Plugins/Process/POSIX/ProcessPOSIXLog.h"
 #include "lldb/Host/HostProcess.h"
-#include "lldb/Host/common/NativeBreakpoint.h"
 #include "lldb/Host/common/NativeRegisterContext.h"
 #include "lldb/Host/posix/ProcessLauncherPosixFork.h"
 #include "lldb/Target/Process.h"




More information about the lldb-commits mailing list