mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste, jingham.
Herald added a subscriber: arichardson.
Herald added a project: All.
mgorny requested review of this revision.

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


https://reviews.llvm.org/D131275

Files:
  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

Index: lldb/unittests/Thread/ThreadTest.cpp
===================================================================
--- lldb/unittests/Thread/ThreadTest.cpp
+++ lldb/unittests/Thread/ThreadTest.cpp
@@ -40,7 +40,8 @@
 
 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;
Index: lldb/unittests/Target/ExecutionContextTest.cpp
===================================================================
--- lldb/unittests/Target/ExecutionContextTest.cpp
+++ lldb/unittests/Target/ExecutionContextTest.cpp
@@ -47,7 +47,8 @@
 
 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;
Index: lldb/unittests/Process/ProcessEventDataTest.cpp
===================================================================
--- lldb/unittests/Process/ProcessEventDataTest.cpp
+++ lldb/unittests/Process/ProcessEventDataTest.cpp
@@ -42,7 +42,8 @@
 
 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;
Index: lldb/unittests/Expression/DWARFExpressionTest.cpp
===================================================================
--- lldb/unittests/Expression/DWARFExpressionTest.cpp
+++ lldb/unittests/Expression/DWARFExpressionTest.cpp
@@ -328,7 +328,9 @@
   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 {
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.h
===================================================================
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.h
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -50,10 +50,12 @@
 
   static llvm::StringRef GetPluginDescriptionStatic();
 
+protected:
   ScriptedProcess(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp,
                   const ScriptedProcess::ScriptedProcessInfo &launch_info,
                   Status &error);
 
+public:
   ~ScriptedProcess() override;
 
   bool CanDebug(lldb::TargetSP target_sp,
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
===================================================================
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -62,8 +62,8 @@
   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()) {
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -48,9 +48,10 @@
 
 class ProcessGDBRemote : public Process,
                          private GDBRemoteClientBase::ContinueDelegate {
-public:
+protected:
   ProcessGDBRemote(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp);
 
+public:
   ~ProcessGDBRemote() override;
 
   static lldb::ProcessSP CreateInstance(lldb::TargetSP target_sp,
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -198,14 +198,13 @@
   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;
 }
 
Index: lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
===================================================================
--- lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
+++ lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
@@ -37,8 +37,10 @@
   static llvm::StringRef GetPluginDescriptionStatic();
 
   // Constructors and destructors
+protected:
   ProcessWindows(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp);
 
+public:
   ~ProcessWindows();
 
   size_t GetSTDOUT(char *buf, size_t buf_size, Status &error) override;
Index: lldb/include/lldb/Target/Process.h
===================================================================
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -475,6 +475,7 @@
     const ProcessEventData &operator=(const ProcessEventData &) = delete;
   };
 
+protected:
   /// 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);
@@ -484,6 +485,7 @@
   Process(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp,
           const lldb::UnixSignalsSP &unix_signals_sp);
 
+public:
   /// Destructor.
   ///
   /// The destructor is virtual since this class is designed to be inherited
Index: lldb/include/lldb/Target/PostMortemProcess.h
===================================================================
--- lldb/include/lldb/Target/PostMortemProcess.h
+++ lldb/include/lldb/Target/PostMortemProcess.h
@@ -21,9 +21,9 @@
 /// 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; }
 };
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to