Centralized the launching of a process into Target::Launch()

While investigating test suite failures when running the test suite remotely, I noticed we had 3 copies of code that launched a process:
1 - in "process launch" command 
2 - SBTarget::Launch() with args
3 - SBTarget::Launch() with SBLaunchInfo

"process launch" was launching through the platform if it was supported (this is needed for remote debugging) and the 2 and 3 were not.

Now all code is in one place.

llvm-svn: 197247

4 files changed, 173 insertions, 216 deletions

diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index 642c5ce474f..52203f6b31b 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -689,57 +689,26 @@ SBTarget::Launch
                 return sb_process;
             }
         }
-        else
-        {
-            if (listener.IsValid())
-                process_sp = target_sp->CreateProcess (listener.ref(), NULL, NULL);
-            else
-                process_sp = target_sp->CreateProcess (target_sp->GetDebugger().GetListener(), NULL, NULL);
-        }
 
-        if (process_sp)
-        {
-            sb_process.SetSP (process_sp);
-            if (getenv("LLDB_LAUNCH_FLAG_DISABLE_STDIO"))
-                launch_flags |= eLaunchFlagDisableSTDIO;
+        if (getenv("LLDB_LAUNCH_FLAG_DISABLE_STDIO"))
+            launch_flags |= eLaunchFlagDisableSTDIO;
 
-            ProcessLaunchInfo launch_info (stdin_path, stdout_path, stderr_path, working_directory, launch_flags);
-            
-            Module *exe_module = target_sp->GetExecutableModulePointer();
-            if (exe_module)
-                launch_info.SetExecutableFile(exe_module->GetPlatformFileSpec(), true);
-            if (argv)
-                launch_info.GetArguments().AppendArguments (argv);
-            if (envp)
-                launch_info.GetEnvironmentEntries ().SetArguments (envp);
-
-            error.SetError (process_sp->Launch (launch_info));
-            if (error.Success())
-            {
-                // We we are stopping at the entry point, we can return now!
-                if (stop_at_entry)
-                    return sb_process;
-                
-                // Make sure we are stopped at the entry
-                StateType state = process_sp->WaitForProcessToStop (NULL);
-                if (state == eStateStopped)
-                {
-                    // resume the process to skip the entry point
-                    error.SetError (process_sp->Resume());
-                    if (error.Success())
-                    {
-                        // If we are doing synchronous mode, then wait for the
-                        // process to stop yet again!
-                        if (target_sp->GetDebugger().GetAsyncExecution () == false)
-                            process_sp->WaitForProcessToStop (NULL);
-                    }
-                }
-            }
-        }
+        ProcessLaunchInfo launch_info (stdin_path, stdout_path, stderr_path, working_directory, launch_flags);
+        
+        Module *exe_module = target_sp->GetExecutableModulePointer();
+        if (exe_module)
+            launch_info.SetExecutableFile(exe_module->GetPlatformFileSpec(), true);
+        if (argv)
+            launch_info.GetArguments().AppendArguments (argv);
+        if (envp)
+            launch_info.GetEnvironmentEntries ().SetArguments (envp);
+
+        if (listener.IsValid())
+            error.SetError (target_sp->Launch(listener.ref(), launch_info));
         else
-        {
-            error.SetErrorString ("unable to create lldb_private::Process");
-        }
+            error.SetError (target_sp->Launch(target_sp->GetDebugger().GetListener(), launch_info));
+
+        sb_process.SetSP(target_sp->GetProcessSP());
     }
     else
     {
@@ -750,7 +719,7 @@ SBTarget::Launch
     if (log)
     {
         log->Printf ("SBTarget(%p)::Launch (...) => SBProcess(%p)", 
-                     target_sp.get(), process_sp.get());
+                     target_sp.get(), sb_process.GetSP().get());
     }
 
     return sb_process;
@@ -762,7 +731,6 @@ SBTarget::Launch (SBLaunchInfo &sb_launch_info, SBError& error)
     Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_API));
     
     SBProcess sb_process;
-    ProcessSP process_sp;
     TargetSP target_sp(GetSP());
     
     if (log)
@@ -774,7 +742,8 @@ SBTarget::Launch (SBLaunchInfo &sb_launch_info, SBError& error)
     {
         Mutex::Locker api_locker (target_sp->GetAPIMutex());
         StateType state = eStateInvalid;
-        process_sp = target_sp->GetProcessSP();
+        {
+        ProcessSP process_sp = target_sp->GetProcessSP();
         if (process_sp)
         {
             state = process_sp->GetState();
@@ -788,58 +757,20 @@ SBTarget::Launch (SBLaunchInfo &sb_launch_info, SBError& error)
                 return sb_process;
             }            
         }
-        
-        if (state != eStateConnected)
-            process_sp = target_sp->CreateProcess (target_sp->GetDebugger().GetListener(), NULL, NULL);
-        
-        if (process_sp)
-        {
-            sb_process.SetSP (process_sp);
-            lldb_private::ProcessLaunchInfo &launch_info = sb_launch_info.ref();
+        }
 
-            Module *exe_module = target_sp->GetExecutableModulePointer();
-            if (exe_module)
-                launch_info.SetExecutableFile(exe_module->GetPlatformFileSpec(), true);
+        lldb_private::ProcessLaunchInfo &launch_info = sb_launch_info.ref();
 
-            const ArchSpec &arch_spec = target_sp->GetArchitecture();
-            if (arch_spec.IsValid())
-                launch_info.GetArchitecture () = arch_spec;
-    
-            error.SetError (process_sp->Launch (launch_info));
-            const bool synchronous_execution = target_sp->GetDebugger().GetAsyncExecution () == false;
-            if (error.Success())
-            {
-                if (launch_info.GetFlags().Test(eLaunchFlagStopAtEntry))
-                {
-                    // If we are doing synchronous mode, then wait for the initial
-                    // stop to happen, else, return and let the caller watch for
-                    // the stop
-                    if (synchronous_execution)
-                         process_sp->WaitForProcessToStop (NULL);
-                    // We we are stopping at the entry point, we can return now!
-                    return sb_process;
-                }
-                
-                // Make sure we are stopped at the entry
-                StateType state = process_sp->WaitForProcessToStop (NULL);
-                if (state == eStateStopped)
-                {
-                    // resume the process to skip the entry point
-                    error.SetError (process_sp->Resume());
-                    if (error.Success())
-                    {
-                        // If we are doing synchronous mode, then wait for the
-                        // process to stop yet again!
-                        if (synchronous_execution)
-                            process_sp->WaitForProcessToStop (NULL);
-                    }
-                }
-            }
-        }
-        else
-        {
-            error.SetErrorString ("unable to create lldb_private::Process");
-        }
+        Module *exe_module = target_sp->GetExecutableModulePointer();
+        if (exe_module)
+            launch_info.SetExecutableFile(exe_module->GetPlatformFileSpec(), true);
+
+        const ArchSpec &arch_spec = target_sp->GetArchitecture();
+        if (arch_spec.IsValid())
+            launch_info.GetArchitecture () = arch_spec;
+        
+        error.SetError (target_sp->Launch (target_sp->GetDebugger().GetListener(), launch_info));
+        sb_process.SetSP(target_sp->GetProcessSP());
     }
     else
     {
@@ -849,8 +780,8 @@ SBTarget::Launch (SBLaunchInfo &sb_launch_info, SBError& error)
     log = lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_API);
     if (log)
     {
-        log->Printf ("SBTarget(%p)::Launch (...) => SBProcess(%p)", 
-                     target_sp.get(), process_sp.get());
+        log->Printf ("SBTarget(%p)::Launch (...) => SBProcess(%p)",
+                     target_sp.get(), sb_process.GetSP().get());
     }
     
     return sb_process;
diff --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp
index 2933c78ca90..7c72ab8c347 100644
--- a/lldb/source/Commands/CommandObjectProcess.cpp
+++ b/lldb/source/Commands/CommandObjectProcess.cpp
@@ -49,7 +49,7 @@ public:
     virtual ~CommandObjectProcessLaunchOrAttach () {}
 protected:
     bool
-    StopProcessIfNecessary (Process *&process, StateType &state, CommandReturnObject &result)
+    StopProcessIfNecessary (Process *process, StateType &state, CommandReturnObject &result)
     {
         state = eStateInvalid;
         if (process)
@@ -187,12 +187,10 @@ protected:
     {
         Debugger &debugger = m_interpreter.GetDebugger();
         Target *target = debugger.GetSelectedTarget().get();
-        Error error;
         // If our listener is NULL, users aren't allows to launch
-        char filename[PATH_MAX];
-        const Module *exe_module = target->GetExecutableModulePointer();
+        ModuleSP exe_module_sp = target->GetExecutableModule();
 
-        if (exe_module == NULL)
+        if (exe_module_sp == NULL)
         {
             result.AppendError ("no file in target, create a debug target using the 'target create' command");
             result.SetStatus (eReturnStatusFailed);
@@ -200,23 +198,31 @@ protected:
         }
         
         StateType state = eStateInvalid;
-        Process *process = m_exe_ctx.GetProcessPtr();
         
-        if (!StopProcessIfNecessary(process, state, result))
+        if (!StopProcessIfNecessary(m_exe_ctx.GetProcessPtr(), state, result))
             return false;
         
         const char *target_settings_argv0 = target->GetArg0();
         
-        exe_module->GetFileSpec().GetPath (filename, sizeof(filename));
+        if (target->GetDisableASLR())
+            m_options.launch_info.GetFlags().Set (eLaunchFlagDisableASLR);
         
+        if (target->GetDisableSTDIO())
+            m_options.launch_info.GetFlags().Set (eLaunchFlagDisableSTDIO);
+        
+        Args environment;
+        target->GetEnvironmentAsArgs (environment);
+        if (environment.GetArgumentCount() > 0)
+            m_options.launch_info.GetEnvironmentEntries ().AppendArguments (environment);
+
         if (target_settings_argv0)
         {
             m_options.launch_info.GetArguments().AppendArgument (target_settings_argv0);
-            m_options.launch_info.SetExecutableFile(exe_module->GetPlatformFileSpec(), false);
+            m_options.launch_info.SetExecutableFile(exe_module_sp->GetPlatformFileSpec(), false);
         }
         else
         {
-            m_options.launch_info.SetExecutableFile(exe_module->GetPlatformFileSpec(), true);
+            m_options.launch_info.SetExecutableFile(exe_module_sp->GetPlatformFileSpec(), true);
         }
 
         if (launch_args.GetArgumentCount() == 0)
@@ -228,122 +234,33 @@ protected:
         else
         {
             m_options.launch_info.GetArguments().AppendArguments (launch_args);
-
             // Save the arguments for subsequent runs in the current target.
             target->SetRunArguments (launch_args);
         }
         
-        if (target->GetDisableASLR())
-            m_options.launch_info.GetFlags().Set (eLaunchFlagDisableASLR);
-    
-        if (target->GetDisableSTDIO())
-            m_options.launch_info.GetFlags().Set (eLaunchFlagDisableSTDIO);
-
-        m_options.launch_info.GetFlags().Set (eLaunchFlagDebug);
-
-        Args environment;
-        target->GetEnvironmentAsArgs (environment);
-        if (environment.GetArgumentCount() > 0)
-            m_options.launch_info.GetEnvironmentEntries ().AppendArguments (environment);
-
-        // Get the value of synchronous execution here.  If you wait till after you have started to
-        // run, then you could have hit a breakpoint, whose command might switch the value, and
-        // then you'll pick up that incorrect value.
-        bool synchronous_execution = m_interpreter.GetSynchronous ();
-
-        PlatformSP platform_sp (target->GetPlatform());
-
-        // Finalize the file actions, and if none were given, default to opening
-        // up a pseudo terminal
-        const bool default_to_use_pty = platform_sp ? platform_sp->IsHost() : false;
-        m_options.launch_info.FinalizeFileActions (target, default_to_use_pty);
-
-        if (state == eStateConnected)
-        {
-            if (m_options.launch_info.GetFlags().Test (eLaunchFlagLaunchInTTY))
-            {
-                result.AppendWarning("can't launch in tty when launching through a remote connection");
-                m_options.launch_info.GetFlags().Clear (eLaunchFlagLaunchInTTY);
-            }
-        }
-        
-        if (!m_options.launch_info.GetArchitecture().IsValid())
-            m_options.launch_info.GetArchitecture() = target->GetArchitecture();
+        Error error = target->Launch(debugger.GetListener(), m_options.launch_info);
         
-        if (platform_sp && platform_sp->CanDebugProcess ())
-        {
-            process = target->GetPlatform()->DebugProcess (m_options.launch_info, 
-                                                           debugger,
-                                                           target,
-                                                           debugger.GetListener(),
-                                                           error).get();
-        }
-        else
-        {
-            const char *plugin_name = m_options.launch_info.GetProcessPluginName();
-            process = target->CreateProcess (debugger.GetListener(), plugin_name, NULL).get();
-            if (process)
-                error = process->Launch (m_options.launch_info);
-        }
-
-        if (process == NULL)
-        {
-            result.SetError (error, "failed to launch or debug process");
-            return false;
-        }
-
-             
         if (error.Success())
         {
-            const char *archname = exe_module->GetArchitecture().GetArchitectureName();
-
-            result.AppendMessageWithFormat ("Process %" PRIu64 " launched: '%s' (%s)\n", process->GetID(), filename, archname);
-            result.SetDidChangeProcessState (true);
-            if (m_options.launch_info.GetFlags().Test(eLaunchFlagStopAtEntry) == false)
+            const char *archname = exe_module_sp->GetArchitecture().GetArchitectureName();
+            ProcessSP process_sp (target->GetProcessSP());
+            if (process_sp)
             {
-                result.SetStatus (eReturnStatusSuccessContinuingNoResult);
-                StateType state = process->WaitForProcessToStop (NULL, NULL, false);
-
-                if (state == eStateStopped)
-                {
-                    error = process->Resume();
-                    if (error.Success())
-                    {
-                        if (synchronous_execution)
-                        {
-                            state = process->WaitForProcessToStop (NULL);
-                            const bool must_be_alive = true;
-                            if (!StateIsStoppedState(state, must_be_alive))
-                            {
-                                result.AppendErrorWithFormat ("process isn't stopped: %s", StateAsCString(state));
-                            }                    
-                            result.SetDidChangeProcessState (true);
-                            result.SetStatus (eReturnStatusSuccessFinishResult);
-                        }
-                        else
-                        {
-                            result.SetStatus (eReturnStatusSuccessContinuingNoResult);
-                        }
-                    }
-                    else
-                    {
-                        result.AppendErrorWithFormat ("process resume at entry point failed: %s", error.AsCString());
-                        result.SetStatus (eReturnStatusFailed);
-                    }                    
-                }
-                else
-                {
-                    result.AppendErrorWithFormat ("initial process state wasn't stopped: %s", StateAsCString(state));
-                    result.SetStatus (eReturnStatusFailed);
-                }                    
+                result.AppendMessageWithFormat ("Process %" PRIu64 " launched: '%s' (%s)\n", process_sp->GetID(), exe_module_sp->GetFileSpec().GetPath().c_str(), archname);
+                result.SetStatus (eReturnStatusSuccessFinishResult);
+                result.SetDidChangeProcessState (true);
+            }
+            else
+            {
+                result.AppendError("no error returned from Target::Launch, and target has no process");
+                result.SetStatus (eReturnStatusFailed);
             }
         }
         else
         {
-            result.AppendErrorWithFormat ("process launch failed: %s", error.AsCString());
+            result.AppendError(error.AsCString());
             result.SetStatus (eReturnStatusFailed);
         }
-
         return result.Succeeded();
     }
 
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
index b3c59e9325e..a4121c7219e 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -254,7 +254,7 @@ protected:
     // Classes that inherit from GDBRemoteCommunication can see and modify these
     //------------------------------------------------------------------
     uint32_t m_packet_timeout;
-#ifdef LLDB_CONFIGURATION_DEBUG
+#ifdef ENABLE_MUTEX_ERROR_CHECKING
     lldb_private::TrackingMutex m_sequence_mutex;
 #else
     lldb_private::Mutex m_sequence_mutex;    // Restrict access to sending/receiving packets to a single thread at a time
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 0336f775859..7c850c6680a 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -28,6 +28,7 @@
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Core/SourceManager.h"
+#include "lldb/Core/State.h"
 #include "lldb/Core/StreamString.h"
 #include "lldb/Core/Timer.h"
 #include "lldb/Core/ValueObject.h"
@@ -2317,6 +2318,114 @@ Target::ClearAllLoadedSections ()
     m_section_load_history.Clear();
 }
 
+
+Error
+Target::Launch (Listener &listener, ProcessLaunchInfo &launch_info)
+{
+    Error error;
+    Error error2;
+    
+    StateType state = eStateInvalid;
+    
+    launch_info.GetFlags().Set (eLaunchFlagDebug);
+    
+    // Get the value of synchronous execution here.  If you wait till after you have started to
+    // run, then you could have hit a breakpoint, whose command might switch the value, and
+    // then you'll pick up that incorrect value.
+    Debugger &debugger = GetDebugger();
+    const bool synchronous_execution = debugger.GetCommandInterpreter().GetSynchronous ();
+    
+    PlatformSP platform_sp (GetPlatform());
+    
+    // Finalize the file actions, and if none were given, default to opening
+    // up a pseudo terminal
+    const bool default_to_use_pty = platform_sp ? platform_sp->IsHost() : false;
+    launch_info.FinalizeFileActions (this, default_to_use_pty);
+    
+    if (state == eStateConnected)
+    {
+        if (launch_info.GetFlags().Test (eLaunchFlagLaunchInTTY))
+        {
+            error.SetErrorString("can't launch in tty when launching through a remote connection");
+            return error;
+        }
+    }
+    
+    if (!launch_info.GetArchitecture().IsValid())
+        launch_info.GetArchitecture() = GetArchitecture();
+    
+    if (state != eStateConnected && platform_sp && platform_sp->CanDebugProcess ())
+    {
+        m_process_sp = GetPlatform()->DebugProcess (launch_info,
+                                                    debugger,
+                                                    this,
+                                                    listener,
+                                                    error);
+    }
+    else
+    {
+        if (state == eStateConnected)
+        {
+            assert(m_process_sp);
+        }
+        else
+        {
+            const char *plugin_name = launch_info.GetProcessPluginName();
+            CreateProcess (listener, plugin_name, NULL);
+        }
+        
+        if (m_process_sp)
+            error = m_process_sp->Launch (launch_info);
+    }
+    
+    if (!m_process_sp)
+    {
+        if (error.Success())
+            error.SetErrorString("failed to launch or debug process");
+        return error;
+    }
+
+    if (error.Success())
+    {
+        if (launch_info.GetFlags().Test(eLaunchFlagStopAtEntry) == false)
+        {
+            StateType state = m_process_sp->WaitForProcessToStop (NULL, NULL, false);
+            
+            if (state == eStateStopped)
+            {
+                error = m_process_sp->Resume();
+                if (error.Success())
+                {
+                    if (synchronous_execution)
+                    {
+                        state = m_process_sp->WaitForProcessToStop (NULL);
+                        const bool must_be_alive = false; // eStateExited is ok, so this must be false
+                        if (!StateIsStoppedState(state, must_be_alive))
+                        {
+                            error2.SetErrorStringWithFormat("process isn't stopped: %s", StateAsCString(state));
+                            return error2;
+                        }
+                    }
+                }
+                else
+                {
+                    error2.SetErrorStringWithFormat("process resume at entry point failed: %s", error.AsCString());
+                    return error2;
+                }
+            }
+            else
+            {
+                error.SetErrorStringWithFormat ("initial process state wasn't stopped: %s", StateAsCString(state));
+            }
+        }
+    }
+    else
+    {
+        error2.SetErrorStringWithFormat ("process launch failed: %s", error.AsCString());
+        return error2;
+    }
+    return error;
+}
 //--------------------------------------------------------------
 // Target::StopHook
 //--------------------------------------------------------------