[Lldb-commits] [lldb] 9b031d5 - [lldb] Make Process and subclass constructors protected

Michał Górny via lldb-commits lldb-commits at lists.llvm.org
Mon Aug 8 08:34:34 PDT 2022


Author: Michał Górny
Date: 2022-08-08T17:34:27+02:00
New Revision: 9b031d5e3a7b455308257a71116a603e76c8c679

URL: https://github.com/llvm/llvm-project/commit/9b031d5e3a7b455308257a71116a603e76c8c679
DIFF: https://github.com/llvm/llvm-project/commit/9b031d5e3a7b455308257a71116a603e76c8c679.diff

LOG: [lldb] Make Process and subclass constructors protected

Make constructors of the Process and its subclasses class protected,
to prevent accidentally constructing Process on stack when it could be
afterwards accessed via a shared_ptr (since it uses
std::enable_shared_from_this<>).

The only place where a stack allocation was used were unittests,
and fixing them via declaring an explicit public constructor
in the respective mock classes is trivial.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D131275

Added: 
    

Modified: 
    lldb/include/lldb/Target/PostMortemProcess.h
    lldb/include/lldb/Target/Process.h
    lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
    lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
    lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
    lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
    lldb/source/Plugins/Process/scripted/ScriptedProcess.h
    lldb/unittests/Expression/DWARFExpressionTest.cpp
    lldb/unittests/Process/ProcessEventDataTest.cpp
    lldb/unittests/Target/ExecutionContextTest.cpp
    lldb/unittests/Thread/ThreadTest.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Target/PostMortemProcess.h b/lldb/include/lldb/Target/PostMortemProcess.h
index 353bfc0919eec..7207fc99ef29a 100644
--- a/lldb/include/lldb/Target/PostMortemProcess.h
+++ b/lldb/include/lldb/Target/PostMortemProcess.h
@@ -21,9 +21,9 @@ namespace lldb_private {
 /// between these kinds of processes can have default implementations in this
 /// class.
 class PostMortemProcess : public Process {
-public:
   using Process::Process;
 
+public:
   bool IsLiveDebugSession() const override { return false; }
 };
 

diff  --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index 505e211e09b63..05b0eb6237c71 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -475,15 +475,6 @@ class Process : public std::enable_shared_from_this<Process>,
     const ProcessEventData &operator=(const ProcessEventData &) = delete;
   };
 
-  /// Construct with a shared pointer to a target, and the Process listener.
-  /// Uses the Host UnixSignalsSP by default.
-  Process(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp);
-
-  /// Construct with a shared pointer to a target, the Process listener, and
-  /// the appropriate UnixSignalsSP for the process.
-  Process(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp,
-          const lldb::UnixSignalsSP &unix_signals_sp);
-
   /// Destructor.
   ///
   /// The destructor is virtual since this class is designed to be inherited
@@ -2499,6 +2490,16 @@ void PruneThreadPlans();
 
 protected:
   friend class Trace;
+
+  /// Construct with a shared pointer to a target, and the Process listener.
+  /// Uses the Host UnixSignalsSP by default.
+  Process(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp);
+
+  /// Construct with a shared pointer to a target, the Process listener, and
+  /// the appropriate UnixSignalsSP for the process.
+  Process(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp,
+          const lldb::UnixSignalsSP &unix_signals_sp);
+
   ///  Get the processor tracing type supported for this process.
   ///  Responses might be 
diff erent depending on the architecture and
   ///  capabilities of the underlying OS.

diff  --git a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
index 043fd0f0f5a54..bf9e5fe7a633c 100644
--- a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
+++ b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
@@ -36,9 +36,6 @@ class ProcessWindows : public Process, public ProcessDebugger {
 
   static llvm::StringRef GetPluginDescriptionStatic();
 
-  // Constructors and destructors
-  ProcessWindows(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp);
-
   ~ProcessWindows();
 
   size_t GetSTDOUT(char *buf, size_t buf_size, Status &error) override;
@@ -104,6 +101,8 @@ class ProcessWindows : public Process, public ProcessDebugger {
   Status DisableWatchpoint(Watchpoint *wp, bool notify = true) override;
 
 protected:
+  ProcessWindows(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp);
+
   Status DoGetMemoryRegionInfo(lldb::addr_t vm_addr,
                                MemoryRegionInfo &info) override;
 

diff  --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 233a665f65904..30fb27932d192 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -193,14 +193,13 @@ void ProcessGDBRemote::Terminate() {
   PluginManager::UnregisterPlugin(ProcessGDBRemote::CreateInstance);
 }
 
-lldb::ProcessSP
-ProcessGDBRemote::CreateInstance(lldb::TargetSP target_sp,
-                                 ListenerSP listener_sp,
-                                 const FileSpec *crash_file_path,
-                                 bool can_connect) {
+lldb::ProcessSP ProcessGDBRemote::CreateInstance(
+    lldb::TargetSP target_sp, ListenerSP listener_sp,
+    const FileSpec *crash_file_path, bool can_connect) {
   lldb::ProcessSP process_sp;
   if (crash_file_path == nullptr)
-    process_sp = std::make_shared<ProcessGDBRemote>(target_sp, listener_sp);
+    process_sp = std::shared_ptr<ProcessGDBRemote>(
+        new ProcessGDBRemote(target_sp, listener_sp));
   return process_sp;
 }
 

diff  --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
index 50cef8e499dcc..6ad9b3effe7b0 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -49,8 +49,6 @@ class ThreadGDBRemote;
 class ProcessGDBRemote : public Process,
                          private GDBRemoteClientBase::ContinueDelegate {
 public:
-  ProcessGDBRemote(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp);
-
   ~ProcessGDBRemote() override;
 
   static lldb::ProcessSP CreateInstance(lldb::TargetSP target_sp,
@@ -239,6 +237,8 @@ class ProcessGDBRemote : public Process,
   friend class GDBRemoteCommunicationClient;
   friend class GDBRemoteRegisterContext;
 
+  ProcessGDBRemote(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp);
+
   bool SupportsMemoryTagging() override;
 
   /// Broadcaster event bits definitions.

diff  --git a/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp b/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
index cd2d4fe4fd234..11692cbb69d48 100644
--- a/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ b/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -62,8 +62,8 @@ lldb::ProcessSP ScriptedProcess::CreateInstance(lldb::TargetSP target_sp,
   ScriptedProcess::ScriptedProcessInfo scripted_process_info(
       target_sp->GetProcessLaunchInfo());
 
-  auto process_sp = std::make_shared<ScriptedProcess>(
-      target_sp, listener_sp, scripted_process_info, error);
+  auto process_sp = std::shared_ptr<ScriptedProcess>(new ScriptedProcess(
+      target_sp, listener_sp, scripted_process_info, error));
 
   if (error.Fail() || !process_sp || !process_sp->m_script_object_sp ||
       !process_sp->m_script_object_sp->IsValid()) {

diff  --git a/lldb/source/Plugins/Process/scripted/ScriptedProcess.h b/lldb/source/Plugins/Process/scripted/ScriptedProcess.h
index 7edd95e230a16..465ef7b64ecd7 100644
--- a/lldb/source/Plugins/Process/scripted/ScriptedProcess.h
+++ b/lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -50,10 +50,6 @@ class ScriptedProcess : public Process {
 
   static llvm::StringRef GetPluginDescriptionStatic();
 
-  ScriptedProcess(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp,
-                  const ScriptedProcess::ScriptedProcessInfo &launch_info,
-                  Status &error);
-
   ~ScriptedProcess() override;
 
   bool CanDebug(lldb::TargetSP target_sp,
@@ -93,6 +89,10 @@ class ScriptedProcess : public Process {
   GetLoadedDynamicLibrariesInfos() override;
 
 protected:
+  ScriptedProcess(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp,
+                  const ScriptedProcess::ScriptedProcessInfo &launch_info,
+                  Status &error);
+
   Status DoStop();
 
   void Clear();

diff  --git a/lldb/unittests/Expression/DWARFExpressionTest.cpp b/lldb/unittests/Expression/DWARFExpressionTest.cpp
index 06f79186bad76..de8087f46af4f 100644
--- a/lldb/unittests/Expression/DWARFExpressionTest.cpp
+++ b/lldb/unittests/Expression/DWARFExpressionTest.cpp
@@ -328,7 +328,9 @@ TEST_F(DWARFExpressionMockProcessTest, DW_OP_deref) {
   EXPECT_THAT_EXPECTED(Evaluate({DW_OP_lit0, DW_OP_deref}), llvm::Failed());
 
   struct MockProcess : Process {
-    using Process::Process;
+    MockProcess(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp)
+        : Process(target_sp, listener_sp) {}
+
     llvm::StringRef GetPluginName() override { return "mock process"; }
     bool CanDebug(lldb::TargetSP target,
                   bool plugin_specified_by_name) override {

diff  --git a/lldb/unittests/Process/ProcessEventDataTest.cpp b/lldb/unittests/Process/ProcessEventDataTest.cpp
index e5d6167da6b42..3accb44d9e9b7 100644
--- a/lldb/unittests/Process/ProcessEventDataTest.cpp
+++ b/lldb/unittests/Process/ProcessEventDataTest.cpp
@@ -42,7 +42,8 @@ class ProcessEventDataTest : public ::testing::Test {
 
 class DummyProcess : public Process {
 public:
-  using Process::Process;
+  DummyProcess(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp)
+      : Process(target_sp, listener_sp) {}
 
   bool CanDebug(lldb::TargetSP target, bool plugin_specified_by_name) override {
     return true;

diff  --git a/lldb/unittests/Target/ExecutionContextTest.cpp b/lldb/unittests/Target/ExecutionContextTest.cpp
index ec9e42fa4b192..3b0e296555fe7 100644
--- a/lldb/unittests/Target/ExecutionContextTest.cpp
+++ b/lldb/unittests/Target/ExecutionContextTest.cpp
@@ -47,7 +47,8 @@ class ExecutionContextTest : public ::testing::Test {
 
 class DummyProcess : public Process {
 public:
-  using Process::Process;
+  DummyProcess(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp)
+      : Process(target_sp, listener_sp) {}
 
   bool CanDebug(lldb::TargetSP target, bool plugin_specified_by_name) override {
     return true;

diff  --git a/lldb/unittests/Thread/ThreadTest.cpp b/lldb/unittests/Thread/ThreadTest.cpp
index 9851d98f496d4..729e5ca74861b 100644
--- a/lldb/unittests/Thread/ThreadTest.cpp
+++ b/lldb/unittests/Thread/ThreadTest.cpp
@@ -40,7 +40,8 @@ class ThreadTest : public ::testing::Test {
 
 class DummyProcess : public Process {
 public:
-  using Process::Process;
+  DummyProcess(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp)
+      : Process(target_sp, listener_sp) {}
 
   bool CanDebug(lldb::TargetSP target, bool plugin_specified_by_name) override {
     return true;


        


More information about the lldb-commits mailing list