Author: enrico
Date: Fri Feb  5 18:43:07 2016
New Revision: 259964

URL: http://llvm.org/viewvc/llvm-project?rev=259964&view=rev
Log:
Fix an issue where certain CommandObjects (or Options thereof) were being 
created once, bound to a specific CommandInterpreter (and hence a specific 
Debugger), and then cached for reuse across different Debugger instances

Obviously, if the original Debugger goes away, those commands are holding on to 
now stale memory, which has the potential to cause crashes

Fixes rdar://24460882


Added:
    
lldb/trunk/packages/Python/lldbsuite/test/functionalities/multidebugger_commands/
    
lldb/trunk/packages/Python/lldbsuite/test/functionalities/multidebugger_commands/TestMultipleDebuggersCommands.py
Modified:
    
lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
    lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
    lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.h
    lldb/trunk/source/Target/LanguageRuntime.cpp

Added: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/multidebugger_commands/TestMultipleDebuggersCommands.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/multidebugger_commands/TestMultipleDebuggersCommands.py?rev=259964&view=auto
==============================================================================
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/multidebugger_commands/TestMultipleDebuggersCommands.py
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/multidebugger_commands/TestMultipleDebuggersCommands.py
 Fri Feb  5 18:43:07 2016
@@ -0,0 +1,48 @@
+"""
+Test that commands do not try and hold on to stale CommandInterpreters in a 
multiple debuggers scenario
+"""
+
+from __future__ import print_function
+
+
+
+import os, time
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class MultipleDebuggersCommandsTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    @no_debug_info_test
+    def test_multipledebuggers_commands(self):
+        """Test that commands do not try and hold on to stale 
CommandInterpreters in a multiple debuggers scenario"""
+        source_init_files = False
+        magic_text = "The following built-in commands may relate to 'env'"
+        
+        debugger_1 = lldb.SBDebugger.Create(source_init_files)
+        interpreter_1 = debugger_1.GetCommandInterpreter()
+        
+        retobj = lldb.SBCommandReturnObject()
+        interpreter_1.HandleCommand("apropos env", retobj)
+        self.assertTrue(magic_text in str(retobj), "[interpreter_1]: the 
output does not contain the correct words")
+        
+        if self.TraceOn(): print(str(retobj))
+        
+        lldb.SBDebugger.Destroy(debugger_1)
+        
+        # now do this again with a different debugger - we shouldn't crash
+        
+        debugger_2 = lldb.SBDebugger.Create(source_init_files)
+        interpreter_2 = debugger_2.GetCommandInterpreter()
+        
+        retobj = lldb.SBCommandReturnObject()
+        interpreter_2.HandleCommand("apropos env", retobj)
+        self.assertTrue(magic_text in str(retobj), "[interpreter_2]: the 
output does not contain the correct words")
+        
+        if self.TraceOn(): print(str(retobj))
+        
+        lldb.SBDebugger.Destroy(debugger_2)
+        

Modified: 
lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp?rev=259964&r1=259963&r2=259964&view=diff
==============================================================================
--- 
lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
 Fri Feb  5 18:43:07 2016
@@ -4167,12 +4167,7 @@ RenderScriptRuntime::RenderScriptRuntime
 lldb::CommandObjectSP
 RenderScriptRuntime::GetCommandObject(lldb_private::CommandInterpreter 
&interpreter)
 {
-    static CommandObjectSP command_object;
-    if (!command_object)
-    {
-        command_object.reset(new 
CommandObjectRenderScriptRuntime(interpreter));
-    }
-    return command_object;
+    return CommandObjectSP(new CommandObjectRenderScriptRuntime(interpreter));
 }
 
 RenderScriptRuntime::~RenderScriptRuntime() = default;

Modified: lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp?rev=259964&r1=259963&r2=259964&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp (original)
+++ lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp Fri Feb  5 
18:43:07 2016
@@ -41,6 +41,9 @@ using namespace lldb_private;
 //------------------------------------------------------------------
 PlatformPOSIX::PlatformPOSIX (bool is_host) :
 Platform(is_host),  // This is the local host platform
+m_option_group_platform_rsync(new OptionGroupPlatformRSync()),
+m_option_group_platform_ssh(new OptionGroupPlatformSSH()),
+m_option_group_platform_caching(new OptionGroupPlatformCaching()),
 m_remote_platform_sp ()
 {
 }
@@ -69,14 +72,17 @@ PlatformPOSIX::GetModuleSpec (const File
 lldb_private::OptionGroupOptions*
 PlatformPOSIX::GetConnectionOptions (lldb_private::CommandInterpreter& 
interpreter)
 {
-    if (m_options.get() == NULL)
+    auto iter = m_options.find(&interpreter), end = m_options.end();
+    if (iter == end)
     {
-        m_options.reset(new OptionGroupOptions(interpreter));
-        m_options->Append(new OptionGroupPlatformRSync());
-        m_options->Append(new OptionGroupPlatformSSH());
-        m_options->Append(new OptionGroupPlatformCaching());
+        std::unique_ptr<lldb_private::OptionGroupOptions> options(new 
OptionGroupOptions(interpreter));
+        options->Append(m_option_group_platform_rsync.get());
+        options->Append(m_option_group_platform_ssh.get());
+        options->Append(m_option_group_platform_caching.get());
+        m_options[&interpreter] = std::move(options);
     }
-    return m_options.get();
+    
+    return m_options.at(&interpreter).get();
 }
 
 bool
@@ -675,29 +681,21 @@ PlatformPOSIX::ConnectRemote (Args& args
 
     if (error.Success() && m_remote_platform_sp)
     {
-        if (m_options.get())
+        if (m_option_group_platform_rsync.get() && 
m_option_group_platform_ssh.get() && m_option_group_platform_caching.get())
         {
-            OptionGroupOptions* options = m_options.get();
-            const OptionGroupPlatformRSync *m_rsync_options =
-                static_cast<const OptionGroupPlatformRSync 
*>(options->GetGroupWithOption('r'));
-            const OptionGroupPlatformSSH *m_ssh_options =
-                static_cast<const OptionGroupPlatformSSH 
*>(options->GetGroupWithOption('s'));
-            const OptionGroupPlatformCaching *m_cache_options =
-                static_cast<const OptionGroupPlatformCaching 
*>(options->GetGroupWithOption('c'));
-
-            if (m_rsync_options->m_rsync)
+            if (m_option_group_platform_rsync->m_rsync)
             {
                 SetSupportsRSync(true);
-                SetRSyncOpts(m_rsync_options->m_rsync_opts.c_str());
-                SetRSyncPrefix(m_rsync_options->m_rsync_prefix.c_str());
-                
SetIgnoresRemoteHostname(m_rsync_options->m_ignores_remote_hostname);
+                
SetRSyncOpts(m_option_group_platform_rsync->m_rsync_opts.c_str());
+                
SetRSyncPrefix(m_option_group_platform_rsync->m_rsync_prefix.c_str());
+                
SetIgnoresRemoteHostname(m_option_group_platform_rsync->m_ignores_remote_hostname);
             }
-            if (m_ssh_options->m_ssh)
+            if (m_option_group_platform_ssh->m_ssh)
             {
                 SetSupportsSSH(true);
-                SetSSHOpts(m_ssh_options->m_ssh_opts.c_str());
+                SetSSHOpts(m_option_group_platform_ssh->m_ssh_opts.c_str());
             }
-            SetLocalCacheDirectory(m_cache_options->m_cache_dir.c_str());
+            
SetLocalCacheDirectory(m_option_group_platform_caching->m_cache_dir.c_str());
         }
     }
 

Modified: lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.h?rev=259964&r1=259963&r2=259964&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.h (original)
+++ lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.h Fri Feb  5 
18:43:07 2016
@@ -12,6 +12,7 @@
 
 // C Includes
 // C++ Includes
+#include <map>
 #include <memory>
 
 // Other libraries and framework includes
@@ -192,7 +193,11 @@ public:
     ConnectToWaitingProcesses(lldb_private::Debugger& debugger, 
lldb_private::Error& error) override;
 
 protected:
-    std::unique_ptr<lldb_private::OptionGroupOptions> m_options;
+    std::unique_ptr<lldb_private::OptionGroupPlatformRSync> 
m_option_group_platform_rsync;
+    std::unique_ptr<lldb_private::OptionGroupPlatformSSH> 
m_option_group_platform_ssh;
+    std::unique_ptr<lldb_private::OptionGroupPlatformCaching> 
m_option_group_platform_caching;
+    
+    
std::map<lldb_private::CommandInterpreter*,std::unique_ptr<lldb_private::OptionGroupOptions>>
 m_options;
     lldb::PlatformSP m_remote_platform_sp; // Allow multiple ways to connect 
to a remote POSIX-compliant OS
 
     lldb_private::Error

Modified: lldb/trunk/source/Target/LanguageRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/LanguageRuntime.cpp?rev=259964&r1=259963&r2=259964&view=diff
==============================================================================
--- lldb/trunk/source/Target/LanguageRuntime.cpp (original)
+++ lldb/trunk/source/Target/LanguageRuntime.cpp Fri Feb  5 18:43:07 2016
@@ -336,6 +336,10 @@ LanguageRuntime::InitializeCommands (Com
             CommandObjectSP command = 
command_callback(parent->GetCommandInterpreter());
             if (command)
             {
+                // the CommandObject vended by a Language plugin cannot be 
created once and cached because
+                // we may create multiple debuggers and need one instance of 
the command each - the implementing function
+                // is meant to create a new instance of the command each time 
it is invoked
+                assert(&command->GetCommandInterpreter() == 
&parent->GetCommandInterpreter() && "language plugin returned command for a 
mismatched CommandInterpreter");
                 parent->LoadSubCommand(command->GetCommandName(), command);
             }
         }


_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to