labath added a comment.
In D56230#1365898 <https://reviews.llvm.org/D56230#1365898>, @Hui wrote:
> Hello labath, I am thinking maybe it is just because the windows command
> 'dir' can't interpret "\\" in the root directory path. For example,
>
> This is working
> "C:\\WINDOWS\\system32\\cmd.exe" /C " dir **c:\**Users\\abc"
>
> This is not working
>
> "C:\\WINDOWS\\system32\\cmd.exe" /C " dir **c:\\**Users\\abc"
>
> Other commands, like **ls** seems not have this problem.
>
> So could we just go ahead with llvm::sys::flattenWindowsCommandLine but set
> a red light on 'dir' command only, at least handle it specifically?
Sorry about the delay.
I've done some experiments of my own, and my conclusion is that the "special"
part here is not the "dir" command but cmd.exe itself. It handles quotes
differently than typical windows applications. It behavior is documented like
this:
1. If all of the following conditions are met, then quote characters
on the command line are preserved:
- no /S switch
- exactly two quote characters
- no special characters between the two quote characters,
where special is one of: &<>()@^|
- there are one or more whitespace characters between the
the two quote characters
- the string between the two quote characters is the name
of an executable file.
2. Otherwise, old behavior is to see if the first character is
a quote character and if so, strip the leading character and
remove the last quote character on the command line, preserving
any text after the last quote character.
So given these rules, and the lldb command `platform shell dir c:\`, the
correct string to pass to CreateProcess would be `cmd.exe /c "dir c:\"`.
Conceptually, that is easy to achieve, but it practice that's a bit hard
because the road from "platform shell" to CreateProcess is long and winding:
1. first, the string `dir c:\` gets converted to a Args array in
Host::RunShellCommand (the `const char *` version). This step uses the "lldb"
quoting rules and results in something like `dir`, `c:\`.
2. then the Args array gets transmogrified in
ProcessLaunchInfo::ConvertArgumentsForLaunchingInShell. This uses some
approximation of the posix shell quoting rules, but I believe that is not fully
correct for posix shells either. The result is `cmd.exe`, `/c`, `dir c:\`
(IIRC).
3. finally, we call `flattenWindowsCommandLine`. This uses the standard windows
quoting rules, which gives us the string `cmd.exe /c "dir c:\\"
So, I think the proper solution here would be to find a way to skip steps 1.
and 2. They're completely unnecessary even in the posix case -- the user types
a string, the shell expects a string, we should be able to just pass that
string from one place to the other and not play around with parsing and
reconstituting it. Then, it will be easy to construct the proper command line
for windows, or the right argv array in the posix case. This will probably
involve having ProcessLaunchInfo be backed by either an argv array or a plain
string.
However, this is getting pretty far from the original intention of your patch.
For the time being I would be happy with checking in the present version of
your patch, with the knowledge that this breaks some "platform shell" use
cases. I don't think that's too bad, because "platform shell" is not the most
important feature in lldb, and it's not fully correct now anyway. In return for
that, we would get all tests in TestQuoting.py passing.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56230/new/
https://reviews.llvm.org/D56230
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
