Move argument globbing to Target::Launch from Platform::LaunchProcess.

[sivachandra]

Summary

This way, the process will recieve globbed arguments even if it was
launched via the process plugin.

[zturner]

Will let Enrico comment on the overall correctness of this. I'll just mention that I pushed a change to the original version of this that added some stuff for fixing Windows paths. You'll likely get a merge conflict when you rebase. Please make sure the changes I made stay in, because they will be needed for Windows even with this patch.

[ovyalov]

Looks good to me - could you check whether this test passes remotely Linux<->Linux?

[sivachandra]

Thanks for the review ovyalov and zturner.

I will test remote Linux->Linux and report.

[sivachandra]

Rebase to take in windows related changes.

[sivachandra]

Per the current design, the globbing feature will not work for remote debugging. Enrico has mentioned that he is working on the generalizing this feature. Until then, I do not see any harm in putting this in. Will commit shortly.

[zturner]

Enrico, will your planned changes address the original issue? Namely, that
if a process is launched through the process plugin with the
eLaunchFlagGlobArguments flag set, after your changes will the arguments
correctly go through the globber first?

[sivachandra]

I saw Zach's question while I typing a similar one.

[zturner]

There's currently a lot of ways to launch processes, and the interactions
that lead to specific methods being chosen can be a bit confusing sometimes.

Out of curiosity, if a plugin supports launching under a debugger directly,
why is the separate start stopped / attach / resume algorithm used? It
seems more straightforward.

If someone gets themselves through that codepath, argdumper won't be used.
Which is confusing if you're outside looking in at the API. From the
outside you just see the process gets launched using a supported mechanism
for launching processes, and it ignores the flag.

[sivachandra]

Abandoning this in favor other solutions proposed on the mailing list.

source/Target/Platform.cpp

if (IsHost())
 
             if (!launch_info.ConvertArgumentsForLaunchingInShell (error,
                                                                   is_localhost,
                                                                   will_debug,
                                                                   first_arg_is_full_shell_command,
                                                                   num_resumes))
                 return error;
         }
-        else if (launch_info.GetFlags().Test(eLaunchFlagGlobArguments))
-        {
-            FileSpec glob_tool_spec;
-            if (!HostInfo::GetLLDBPath(lldb::ePathTypeSupportExecutableDir, glob_tool_spec))
-            {
-                error.SetErrorString("could not find argdumper tool");
-                return error;
-            }
-#if defined(_WIN32)
-            glob_tool_spec.AppendPathComponent("argdumper.exe");
-#else
-            glob_tool_spec.AppendPathComponent("argdumper");
-#endif
-            if (!glob_tool_spec.Exists())
-            {
-                error.SetErrorString("could not find argdumper tool");
-                return error;
-            }
-
-            std::string quoted_cmd_string;
-            launch_info.GetArguments().GetQuotedCommandString(quoted_cmd_string);
-#if defined(_WIN32)
-            std::replace(quoted_cmd_string.begin(), quoted_cmd_string.end(), '\\', '/');
-#endif
-            StreamString glob_command;
-            
-            glob_command.Printf("%s %s",
-                                glob_tool_spec.GetPath().c_str(),
-                                quoted_cmd_string.c_str());
-            
-            int status;
-            std::string output;
-            RunShellCommand(glob_command.GetData(), launch_info.GetWorkingDirectory(), &status, nullptr, &output, 10);
-            
-            if (status != 0)
-            {
-                error.SetErrorStringWithFormat("argdumper exited with error %d", status);
-                return error;
-            }
-            
-            auto data_sp = StructuredData::ParseJSON(output);
-            if (!data_sp)
-            {
-                error.SetErrorString("invalid JSON");
-                return error;
-            }
-            
-            auto dict_sp = data_sp->GetAsDictionary();
-            if (!data_sp)
-            {
-                error.SetErrorString("invalid JSON");
-                return error;
-            }
-
-            auto args_sp = dict_sp->GetObjectForDotSeparatedPath("arguments");
-            if (!args_sp)
-            {
-                error.SetErrorString("invalid JSON");
-                return error;
-            }
-            
-            auto args_array_sp = args_sp->GetAsArray();
-            if (!args_array_sp)
-            {
-                error.SetErrorString("invalid JSON");
-                return error;
-            }
-            
-            launch_info.GetArguments().Clear();
-            
-            for (size_t i = 0;
-                 i < args_array_sp->GetSize();
-                 i++)
-            {
-                auto item_sp = args_array_sp->GetItemAtIndex(i);
-                if (!item_sp)
-                    continue;
-                auto str_sp = item_sp->GetAsString();
-                if (!str_sp)
-                    continue;
-                
-                launch_info.GetArguments().AppendArgument(str_sp->GetValue().c_str());
-            }
-        }
 
         if (log)
             log->Printf ("Platform::%s final launch_info resume count: %" PRIu32, __FUNCTION__, launch_info.GetResumeCount ());
 
         error = Host::LaunchProcess (launch_info);
     }
     else
         error.SetErrorString ("base lldb_private::Platform class can't launch remote processes");

source/Target/Target.cpp

 #include "lldb/Core/StreamFile.h"
 #include "lldb/Core/StreamString.h"
 #include "lldb/Core/Timer.h"
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Expression/ClangASTSource.h"
 #include "lldb/Expression/ClangUserExpression.h"
 #include "lldb/Host/FileSpec.h"
 #include "lldb/Host/Host.h"
+#include "lldb/Host/HostInfo.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Interpreter/OptionGroupWatchpoint.h"
 #include "lldb/Interpreter/OptionValues.h"
 #include "lldb/Interpreter/Property.h"
 #include "lldb/lldb-private-log.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Target/Process.h"
if (state == eStateConnected)
             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 (launch_info.GetFlags().Test(eLaunchFlagGlobArguments))
+    {
+        FileSpec glob_tool_spec;
+        if (!HostInfo::GetLLDBPath(lldb::ePathTypeSupportExecutableDir, glob_tool_spec))
+        {
+            error.SetErrorString("could not find argdumper tool");
+            return error;
+        }
+#if defined(_WIN32)
+        glob_tool_spec.AppendPathComponent("argdumper.exe");
+#else
+        glob_tool_spec.AppendPathComponent("argdumper");
+#endif
+        if (!glob_tool_spec.Exists())
+        {
+            error.SetErrorString("could not find argdumper tool");
+            return error;
+        }
+
+        std::string quoted_cmd_string;
+        launch_info.GetArguments().GetQuotedCommandString(quoted_cmd_string);
+#if defined(_WIN32)
+        std::replace(quoted_cmd_string.begin(), quoted_cmd_string.end(), '\\', '/');
+#endif
+        StreamString glob_command;
+        
+        glob_command.Printf("%s %s",
+                            glob_tool_spec.GetPath().c_str(),
+                            quoted_cmd_string.c_str());
+        
+        int status;
+        std::string output;
+        Host::RunShellCommand(glob_command.GetData(), launch_info.GetWorkingDirectory(), &status, nullptr, &output, 10);
+        
+        if (status != 0)
+        {
+            error.SetErrorStringWithFormat("argdumper exited with error %d", status);
+            return error;
+        }
+        
+        auto data_sp = StructuredData::ParseJSON(output);
+        if (!data_sp)
+        {
+            error.SetErrorString("invalid JSON");
+            return error;
+        }
+        
+        auto dict_sp = data_sp->GetAsDictionary();
+        if (!data_sp)
+        {
+            error.SetErrorString("invalid JSON");
+            return error;
+        }
+
+        auto args_sp = dict_sp->GetObjectForDotSeparatedPath("arguments");
+        if (!args_sp)
+        {
+            error.SetErrorString("invalid JSON");
+            return error;
+        }
+        
+        auto args_array_sp = args_sp->GetAsArray();
+        if (!args_array_sp)
+        {
+            error.SetErrorString("invalid JSON");
+            return error;
+        }
+        
+        launch_info.GetArguments().Clear();
+        
+        for (size_t i = 0;
+             i < args_array_sp->GetSize();
+             i++)
+        {
+            auto item_sp = args_array_sp->GetItemAtIndex(i);
+            if (!item_sp)
+                continue;
+            auto str_sp = item_sp->GetAsString();
+            if (!str_sp)
+                continue;
+            
+            launch_info.GetArguments().AppendArgument(str_sp->GetValue().c_str());
+        }
+    }
+
     // If we're not already connected to the process, and if we have a platform that can launch a process for debugging, go ahead and do that here.
     if (state != eStateConnected && platform_sp && platform_sp->CanDebugProcess ())
     {
         if (log)
             log->Printf ("Target::%s asking the platform to debug the process", __FUNCTION__);
 
         m_process_sp = GetPlatform()->DebugProcess (launch_info,
                                                     debugger,