[lldb] Prevent 'process connect' from using local-only plugins
authorMichał Górny <mgorny@moritz.systems>
Fri, 20 Nov 2020 16:12:22 +0000 (17:12 +0100)
committerMichał Górny <mgorny@moritz.systems>
Mon, 23 Nov 2020 08:48:55 +0000 (09:48 +0100)
Add a 'can_connect' parameter to Process plugin initialization, and use
it to filter plugins to these capable of remote connections.  This is
used to prevent 'process connect' from picking up a plugin that can only
be used locally, e.g. the legacy FreeBSD plugin.

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

30 files changed:
lldb/include/lldb/Target/Process.h
lldb/include/lldb/Target/ProcessTrace.h
lldb/include/lldb/Target/Target.h
lldb/include/lldb/lldb-private-interfaces.h
lldb/source/API/SBTarget.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.h
lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h
lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp
lldb/source/Plugins/Process/elf-core/ProcessElfCore.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
lldb/source/Plugins/Process/mach-core/ProcessMachCore.h
lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
lldb/source/Plugins/Process/minidump/ProcessMinidump.h
lldb/source/Target/Platform.cpp
lldb/source/Target/Process.cpp
lldb/source/Target/ProcessTrace.cpp
lldb/source/Target/Target.cpp
lldb/source/Target/TraceSessionFileParser.cpp
lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
lldb/test/Shell/Commands/command-process-connect.test

index 7f1e1e8..744deab 100644 (file)
@@ -537,7 +537,8 @@ public:
   static lldb::ProcessSP FindPlugin(lldb::TargetSP target_sp,
                                     llvm::StringRef plugin_name,
                                     lldb::ListenerSP listener_sp,
-                                    const FileSpec *crash_file_path);
+                                    const FileSpec *crash_file_path,
+                                    bool can_connect);
 
   /// Static function that can be used with the \b host function
   /// Host::StartMonitoringChildProcess ().
index f947fbe..53b3e70 100644 (file)
@@ -77,7 +77,8 @@ protected:
 private:
   static lldb::ProcessSP CreateInstance(lldb::TargetSP target_sp,
                                         lldb::ListenerSP listener_sp,
-                                        const FileSpec *crash_file_path);
+                                        const FileSpec *crash_file_path,
+                                        bool can_connect);
 };
 
 } // namespace lldb_private
index f77917a..47568c9 100644 (file)
@@ -573,7 +573,8 @@ public:
   // used.
   const lldb::ProcessSP &CreateProcess(lldb::ListenerSP listener_sp,
                                        llvm::StringRef plugin_name,
-                                       const FileSpec *crash_file);
+                                       const FileSpec *crash_file,
+                                       bool can_connect);
 
   const lldb::ProcessSP &GetProcessSP() const;
 
index 1a4eaba..df33f8a 100644 (file)
@@ -76,7 +76,7 @@ typedef lldb::PlatformSP (*PlatformCreateInstance)(bool force,
                                                    const ArchSpec *arch);
 typedef lldb::ProcessSP (*ProcessCreateInstance)(
     lldb::TargetSP target_sp, lldb::ListenerSP listener_sp,
-    const FileSpec *crash_file_path);
+    const FileSpec *crash_file_path, bool can_connect);
 typedef lldb::ScriptInterpreterSP (*ScriptInterpreterCreateInstance)(
     Debugger &debugger);
 typedef SymbolFile *(*SymbolFileCreateInstance)(lldb::ObjectFileSP objfile_sp);
index 34cab62..2a30515 100644 (file)
@@ -267,7 +267,7 @@ SBProcess SBTarget::LoadCore(const char *core_file, lldb::SBError &error) {
     FileSpec filespec(core_file);
     FileSystem::Instance().Resolve(filespec);
     ProcessSP process_sp(target_sp->CreateProcess(
-        target_sp->GetDebugger().GetListener(), "", &filespec));
+        target_sp->GetDebugger().GetListener(), "", &filespec, false));
     if (process_sp) {
       error.SetError(process_sp->LoadCore());
       if (error.Success())
@@ -567,10 +567,11 @@ lldb::SBProcess SBTarget::ConnectRemote(SBListener &listener, const char *url,
     std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
     if (listener.IsValid())
       process_sp =
-          target_sp->CreateProcess(listener.m_opaque_sp, plugin_name, nullptr);
+          target_sp->CreateProcess(listener.m_opaque_sp, plugin_name, nullptr,
+                                   true);
     else
       process_sp = target_sp->CreateProcess(
-          target_sp->GetDebugger().GetListener(), plugin_name, nullptr);
+          target_sp->GetDebugger().GetListener(), plugin_name, nullptr, true);
 
     if (process_sp) {
       sb_process.SetSP(process_sp);
index 9828528..eba129c 100644 (file)
@@ -401,7 +401,8 @@ protected:
           target_sp->AppendExecutableSearchPaths(core_file_dir);
 
           ProcessSP process_sp(target_sp->CreateProcess(
-              GetDebugger().GetListener(), llvm::StringRef(), &core_file));
+              GetDebugger().GetListener(), llvm::StringRef(), &core_file,
+              false));
 
           if (process_sp) {
             // Seems weird that we Launch a core file, but that is what we
index 7f6cbd7..47f62a9 100644 (file)
@@ -388,7 +388,8 @@ lldb::ProcessSP PlatformPOSIX::Attach(ProcessAttachInfo &attach_info,
 
       process_sp =
           target->CreateProcess(attach_info.GetListenerForProcess(debugger),
-                                attach_info.GetProcessPluginName(), nullptr);
+                                attach_info.GetProcessPluginName(), nullptr,
+                                false);
 
       if (process_sp) {
         ListenerSP listener_sp = attach_info.GetHijackListener();
@@ -468,7 +469,8 @@ PlatformPOSIX::DebugProcess(ProcessLaunchInfo &launch_info, Debugger &debugger,
   // Now create the gdb-remote process.
   LLDB_LOG(log, "having target create process with gdb-remote plugin");
   process_sp =
-      target->CreateProcess(launch_info.GetListener(), "gdb-remote", nullptr);
+      target->CreateProcess(launch_info.GetListener(), "gdb-remote", nullptr,
+                            true);
 
   if (!process_sp) {
     error.SetErrorString("CreateProcess() failed for gdb-remote process");
index 167a921..0ef4dcb 100644 (file)
@@ -233,7 +233,8 @@ ProcessSP PlatformWindows::DebugProcess(ProcessLaunchInfo &launch_info,
     return Attach(attach_info, debugger, target, error);
   } else {
     ProcessSP process_sp = target->CreateProcess(
-        launch_info.GetListener(), launch_info.GetProcessPluginName(), nullptr);
+        launch_info.GetListener(), launch_info.GetProcessPluginName(), nullptr,
+        false);
 
     // We need to launch and attach to the process.
     launch_info.GetFlags().Set(eLaunchFlagDebug);
@@ -275,7 +276,7 @@ lldb::ProcessSP PlatformWindows::Attach(ProcessAttachInfo &attach_info,
 
   const char *plugin_name = attach_info.GetProcessPluginName();
   process_sp = target->CreateProcess(
-      attach_info.GetListenerForProcess(debugger), plugin_name, nullptr);
+      attach_info.GetListenerForProcess(debugger), plugin_name, nullptr, false);
 
   process_sp->HijackProcessEvents(attach_info.GetHijackListener());
   if (process_sp)
index e8bfbd8..b3774b9 100644 (file)
@@ -500,7 +500,7 @@ lldb::ProcessSP PlatformRemoteGDBServer::DebugProcess(
           // The darwin always currently uses the GDB remote debugger plug-in
           // so even when debugging locally we are debugging remotely!
           process_sp = target->CreateProcess(launch_info.GetListener(),
-                                             "gdb-remote", nullptr);
+                                             "gdb-remote", nullptr, true);
 
           if (process_sp) {
             error = process_sp->ConnectRemote(connect_url.c_str());
@@ -587,7 +587,7 @@ lldb::ProcessSP PlatformRemoteGDBServer::Attach(
           // so even when debugging locally we are debugging remotely!
           process_sp =
               target->CreateProcess(attach_info.GetListenerForProcess(debugger),
-                                    "gdb-remote", nullptr);
+                                    "gdb-remote", nullptr, true);
           if (process_sp) {
             error = process_sp->ConnectRemote(connect_url.c_str());
             if (error.Success()) {
index 67a18bd..ed134d8 100644 (file)
@@ -70,9 +70,10 @@ UnixSignalsSP &GetFreeBSDSignals() {
 lldb::ProcessSP
 ProcessFreeBSD::CreateInstance(lldb::TargetSP target_sp,
                                lldb::ListenerSP listener_sp,
-                               const FileSpec *crash_file_path) {
+                               const FileSpec *crash_file_path,
+                               bool can_connect) {
   lldb::ProcessSP process_sp;
-  if (crash_file_path == NULL)
+  if (crash_file_path == NULL && !can_connect)
     process_sp.reset(
         new ProcessFreeBSD(target_sp, listener_sp, GetFreeBSDSignals()));
   return process_sp;
index 536da0c..dbb2acb 100644 (file)
@@ -26,7 +26,8 @@ public:
   // Static functions.
   static lldb::ProcessSP
   CreateInstance(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp,
-                 const lldb_private::FileSpec *crash_file_path);
+                 const lldb_private::FileSpec *crash_file_path,
+                 bool can_connect);
 
   static void Initialize();
 
index 6e394ea..c3f1d01 100644 (file)
@@ -111,7 +111,8 @@ void ProcessKDP::Terminate() {
 
 lldb::ProcessSP ProcessKDP::CreateInstance(TargetSP target_sp,
                                            ListenerSP listener_sp,
-                                           const FileSpec *crash_file_path) {
+                                           const FileSpec *crash_file_path,
+                                           bool can_connect) {
   lldb::ProcessSP process_sp;
   if (crash_file_path == NULL)
     process_sp = std::make_shared<ProcessKDP>(target_sp, listener_sp);
index 52af561..137b678 100644 (file)
@@ -32,7 +32,8 @@ public:
   // Constructors and Destructors
   static lldb::ProcessSP
   CreateInstance(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp,
-                 const lldb_private::FileSpec *crash_file_path);
+                 const lldb_private::FileSpec *crash_file_path,
+                 bool can_connect);
 
   static void Initialize();
 
index 96e2603..11bfc7c 100644 (file)
@@ -79,7 +79,8 @@ namespace lldb_private {
 
 ProcessSP ProcessWindows::CreateInstance(lldb::TargetSP target_sp,
                                          lldb::ListenerSP listener_sp,
-                                         const FileSpec *) {
+                                         const FileSpec *,
+                                         bool can_connect) {
   return ProcessSP(new ProcessWindows(target_sp, listener_sp));
 }
 
index a1085df..7d14431 100644 (file)
@@ -25,7 +25,8 @@ public:
   // Static functions.
   static lldb::ProcessSP CreateInstance(lldb::TargetSP target_sp,
                                         lldb::ListenerSP listener_sp,
-                                        const FileSpec *);
+                                        const FileSpec *,
+                                        bool can_connect);
 
   static void Initialize();
 
index dbc1a01..3bcf560 100644 (file)
@@ -52,9 +52,10 @@ void ProcessElfCore::Terminate() {
 
 lldb::ProcessSP ProcessElfCore::CreateInstance(lldb::TargetSP target_sp,
                                                lldb::ListenerSP listener_sp,
-                                               const FileSpec *crash_file) {
+                                               const FileSpec *crash_file,
+                                               bool can_connect) {
   lldb::ProcessSP process_sp;
-  if (crash_file) {
+  if (crash_file && !can_connect) {
     // Read enough data for a ELF32 header or ELF64 header Note: Here we care
     // about e_type field only, so it is safe to ignore possible presence of
     // the header extension.
index 9f796d6..793f3cd 100644 (file)
@@ -33,7 +33,8 @@ public:
   // Constructors and Destructors
   static lldb::ProcessSP
   CreateInstance(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp,
-                 const lldb_private::FileSpec *crash_file_path);
+                 const lldb_private::FileSpec *crash_file_path,
+                 bool can_connect);
 
   static void Initialize();
 
index 0cd97ab..4c00e9a 100644 (file)
@@ -205,7 +205,8 @@ void ProcessGDBRemote::Terminate() {
 lldb::ProcessSP
 ProcessGDBRemote::CreateInstance(lldb::TargetSP target_sp,
                                  ListenerSP listener_sp,
-                                 const FileSpec *crash_file_path) {
+                                 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);
index e47300f..5efe8b7 100644 (file)
@@ -55,7 +55,8 @@ public:
 
   static lldb::ProcessSP CreateInstance(lldb::TargetSP target_sp,
                                         lldb::ListenerSP listener_sp,
-                                        const FileSpec *crash_file_path);
+                                        const FileSpec *crash_file_path,
+                                        bool can_connect);
 
   static void Initialize();
 
index c1a8d16..6f03825 100644 (file)
@@ -61,9 +61,10 @@ void ProcessMachCore::Terminate() {
 
 lldb::ProcessSP ProcessMachCore::CreateInstance(lldb::TargetSP target_sp,
                                                 ListenerSP listener_sp,
-                                                const FileSpec *crash_file) {
+                                                const FileSpec *crash_file,
+                                                bool can_connect) {
   lldb::ProcessSP process_sp;
-  if (crash_file) {
+  if (crash_file && !can_connect) {
     const size_t header_size = sizeof(llvm::MachO::mach_header);
     auto data_sp = FileSystem::Instance().CreateDataBuffer(
         crash_file->GetPath(), header_size, 0);
index 9c85139..60463bc 100644 (file)
@@ -28,7 +28,8 @@ public:
 
   static lldb::ProcessSP
   CreateInstance(lldb::TargetSP target_sp, lldb::ListenerSP listener,
-                 const lldb_private::FileSpec *crash_file_path);
+                 const lldb_private::FileSpec *crash_file_path,
+                 bool can_connect);
 
   static void Initialize();
 
index 1850a36..c001547 100644 (file)
@@ -200,8 +200,9 @@ const char *ProcessMinidump::GetPluginDescriptionStatic() {
 
 lldb::ProcessSP ProcessMinidump::CreateInstance(lldb::TargetSP target_sp,
                                                 lldb::ListenerSP listener_sp,
-                                                const FileSpec *crash_file) {
-  if (!crash_file)
+                                                const FileSpec *crash_file,
+                                                bool can_connect) {
+  if (!crash_file || can_connect)
     return nullptr;
 
   lldb::ProcessSP process_sp;
index 1d4d535..9a68bd4 100644 (file)
@@ -30,7 +30,8 @@ class ProcessMinidump : public PostMortemProcess {
 public:
   static lldb::ProcessSP CreateInstance(lldb::TargetSP target_sp,
                                         lldb::ListenerSP listener_sp,
-                                        const FileSpec *crash_file_path);
+                                        const FileSpec *crash_file_path,
+                                        bool can_connect);
 
   static void Initialize();
 
index 685dd95..b5b673a 100644 (file)
@@ -1834,7 +1834,7 @@ lldb::ProcessSP Platform::DoConnectProcess(llvm::StringRef connect_url,
   debugger.GetTargetList().SetSelectedTarget(target);
 
   lldb::ProcessSP process_sp =
-      target->CreateProcess(debugger.GetListener(), plugin_name, nullptr);
+      target->CreateProcess(debugger.GetListener(), plugin_name, nullptr, true);
 
   if (!process_sp)
     return nullptr;
index c2effda..14f8326 100644 (file)
@@ -479,7 +479,8 @@ llvm::ArrayRef<OptionDefinition> ProcessLaunchCommandOptions::GetDefinitions() {
 ProcessSP Process::FindPlugin(lldb::TargetSP target_sp,
                               llvm::StringRef plugin_name,
                               ListenerSP listener_sp,
-                              const FileSpec *crash_file_path) {
+                              const FileSpec *crash_file_path,
+                              bool can_connect) {
   static uint32_t g_process_unique_id = 0;
 
   ProcessSP process_sp;
@@ -489,7 +490,8 @@ ProcessSP Process::FindPlugin(lldb::TargetSP target_sp,
     create_callback =
         PluginManager::GetProcessCreateCallbackForPluginName(const_plugin_name);
     if (create_callback) {
-      process_sp = create_callback(target_sp, listener_sp, crash_file_path);
+      process_sp = create_callback(target_sp, listener_sp, crash_file_path,
+                                   can_connect);
       if (process_sp) {
         if (process_sp->CanDebug(target_sp, true)) {
           process_sp->m_process_unique_id = ++g_process_unique_id;
@@ -502,7 +504,8 @@ ProcessSP Process::FindPlugin(lldb::TargetSP target_sp,
          (create_callback =
               PluginManager::GetProcessCreateCallbackAtIndex(idx)) != nullptr;
          ++idx) {
-      process_sp = create_callback(target_sp, listener_sp, crash_file_path);
+      process_sp = create_callback(target_sp, listener_sp, crash_file_path,
+                                   can_connect);
       if (process_sp) {
         if (process_sp->CanDebug(target_sp, false)) {
           process_sp->m_process_unique_id = ++g_process_unique_id;
index 85e5380..4f7a576 100644 (file)
@@ -34,7 +34,10 @@ void ProcessTrace::Terminate() {
 
 ProcessSP ProcessTrace::CreateInstance(TargetSP target_sp,
                                        ListenerSP listener_sp,
-                                       const FileSpec *crash_file) {
+                                       const FileSpec *crash_file,
+                                       bool can_connect) {
+  if (can_connect)
+    return nullptr;
   return std::make_shared<ProcessTrace>(target_sp, listener_sp);
 }
 
index 9bed203..c6bb989 100644 (file)
@@ -199,12 +199,13 @@ void Target::DeleteCurrentProcess() {
 
 const lldb::ProcessSP &Target::CreateProcess(ListenerSP listener_sp,
                                              llvm::StringRef plugin_name,
-                                             const FileSpec *crash_file) {
+                                             const FileSpec *crash_file,
+                                             bool can_connect) {
   if (!listener_sp)
     listener_sp = GetDebugger().GetListener();
   DeleteCurrentProcess();
   m_process_sp = Process::FindPlugin(shared_from_this(), plugin_name,
-                                     listener_sp, crash_file);
+                                     listener_sp, crash_file, can_connect);
   return m_process_sp;
 }
 
@@ -2975,7 +2976,7 @@ Status Target::Launch(ProcessLaunchInfo &launch_info, Stream *stream) {
     } else {
       // Use a Process plugin to construct the process.
       const char *plugin_name = launch_info.GetProcessPluginName();
-      CreateProcess(launch_info.GetListener(), plugin_name, nullptr);
+      CreateProcess(launch_info.GetListener(), plugin_name, nullptr, false);
     }
 
     // Since we didn't have a platform launch the process, launch it here.
@@ -3103,7 +3104,7 @@ Status Target::Attach(ProcessAttachInfo &attach_info, Stream *stream) {
       const char *plugin_name = attach_info.GetProcessPluginName();
       process_sp =
           CreateProcess(attach_info.GetListenerForProcess(GetDebugger()),
-                        plugin_name, nullptr);
+                        plugin_name, nullptr, false);
       if (process_sp == nullptr) {
         error.SetErrorStringWithFormat(
             "failed to create process using plugin %s",
index bf6b620..1cef818 100644 (file)
@@ -127,7 +127,8 @@ TraceSessionFileParser::ParseProcess(const JSONProcess &process) {
 
   ProcessSP process_sp = target_sp->CreateProcess(
       /*listener*/ nullptr, "trace",
-      /*crash_file*/ nullptr);
+      /*crash_file*/ nullptr,
+      /*can_connect*/ false);
 
   process_sp->SetID(static_cast<lldb::pid_t>(process.pid));
 
index 5e98e3b..e9152f0 100644 (file)
@@ -11,7 +11,6 @@ class TestProcessConnect(GDBRemoteTestBase):
 
     NO_DEBUG_INFO_TESTCASE = True
 
-    @skipIfWindows
     def test_gdb_remote_sync(self):
         """Test the gdb-remote command in synchronous mode"""
         try:
@@ -21,7 +20,6 @@ class TestProcessConnect(GDBRemoteTestBase):
         finally:
             self.dbg.GetSelectedPlatform().DisconnectRemote()
 
-    @skipIfWindows
     @skipIfReproducer # Reproducer don't support async.
     def test_gdb_remote_async(self):
         """Test the gdb-remote command in asynchronous mode"""
@@ -35,8 +33,6 @@ class TestProcessConnect(GDBRemoteTestBase):
         finally:
             self.dbg.GetSelectedPlatform().DisconnectRemote()
 
-    @skipIfWindows
-    @expectedFailureAll(oslist=["freebsd"])
     def test_process_connect_sync(self):
         """Test the gdb-remote command in synchronous mode"""
         try:
@@ -47,8 +43,6 @@ class TestProcessConnect(GDBRemoteTestBase):
         finally:
             self.dbg.GetSelectedPlatform().DisconnectRemote()
 
-    @skipIfWindows
-    @expectedFailureAll(oslist=["freebsd"])
     @skipIfReproducer # Reproducer don't support async.
     def test_process_connect_async(self):
         """Test the gdb-remote command in asynchronous mode"""
index 415cda1..c476136 100644 (file)
@@ -1,6 +1,3 @@
-# UNSUPPORTED: system-windows
-# XFAIL: system-freebsd
-
 # Synchronous
 # RUN: %lldb -o 'platform select remote-gdb-server' -o 'process connect connect://localhost:4321' 2>&1 | FileCheck %s