[lldb-dev] problem with quoted strings in setting target.run-args

[Greg Clayton via lldb-dev]

> On Feb 18, 2016, at 4:16 PM, Ted Woodward via lldb-dev <lldb-dev at lists.llvm.org> wrote:
> 
> Quoted strings in target.run-args aren’t handled correctly.
>  
> (lldb) settings set target.run-args "foo bar"
> (lldb) settings show target.run-args
> target.run-args (array of strings) =
>   [0]: "foo bar"
>  
> This looks correct, but the Args in the ProcessLaunchInfo passed to the Platform doesn’t have m_args_quote_char set, so if the Args is later pulled out with GetQuotedCommandString() it won’t get “foo bar”, but will instead get foo and bar unquoted. This is masked when talking to debugserver or lldb_server because run-args are sent to the server using an RSP packet, but on systems like Windows or the Hexagon Simulator, where run-args are on the command line, you get 2 args, foo and bar, instead of 1 arg “foo bar”.
>  
> The first problem is in OptionValueArray::SetArgs(), in the eVarSetOperationAppend case. It calls Args::GetArgumentAtIndex(), which doesn’t return a quoted argument. I added a function GetQuotedArgumentAtIndex() and called that, which revealed the second problem. The string is passed into OptionValue::CreateValueFromCStringForTypeMask(), which calls OptionValueString::SetValueFromString(). In that function it explicitly strips quotes. Changing it to not strip quotes leads to the third problem – when TargetProperties::RunArgsValueChangedCallback() pulls the data from the OptionValueArray to make a new Args, it calls OptionValueArray::GetArgs(), which doesn’t handle quoting like the Args ctor does.
>  
> I think changing the OptionValue classes to handle quoting could lead to problems with other use cases. So that leaves me with the option of going through the Args before launch and adding quotes around anything with spaces, which seems hackish. Any thoughts on how to solve this issue?

Any changes that are made need to know a few things:
1 - Many things that take arguments don't need the quotes for the arguments, the quotes are there to help us split arguments that contain things that must be quoted. Things like exec and posix_spawn take a "const char **" NULL terminate array of C strings. And the quotes are not needed, nor are they wanted and if you add them, they will hose things up.
2 - Anyone launching via an API that launches through a shell will need to quote correctly for your given shell or launch mechanism. There are no guarantees that the original quotes (ours mimic bash and other shell quoting) will be what you will want/need when you launch (launch in command.exe in windows).

What OptionValueArgs should contain is a valid list of strings that has been broken up into args. If that is currently true, I don't see a bug here. I am fine with you adding a method to OptionValueArgs that is something like "GetQuotedCommandString(...)" that would add the quotes as needed, but again, this might be specific to the shell. I know what bash and tcsh expect, but what does windows expect? Can you use single quoted strings if your arguments contain double quotes? Can you use double quotes if your argument has single quotes? Can you escape the quote characters with a '\' character? That seems like a lot of arguments to pass to the GetQuotedCommandString() function, but you will need to make it this way if so...

But the _only_ client of OptionValueArgs is the "run-args" so the other option would be to switch OptionValueArgs over to use lldb_private::Args instead of inheriting from OptionValueArray. If you do this, you will need to implement many of the OptionValue virtual functions like:

        virtual void
        DumpValue (const ExecutionContext *exe_ctx, Stream &strm, uint32_t dump_mask) = 0;
        
        virtual Error
        SetValueFromString (llvm::StringRef value, VarSetOperationType op = eVarSetOperationAssign);
        
        virtual bool
        Clear () = 0;

        virtual lldb::OptionValueSP
        DeepCopy () const = 0;

Then you would have something that still has the strings split up and yet still knows what the original quoting was like since you would store your arguments in a lldb_private::Args array that would be a member variable of OptionValueArgs.