LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 15802 - On windows %INCLUDE% ending in \ leads to clang hanging when trying to kick of new process
Summary: On windows %INCLUDE% ending in \ leads to clang hanging when trying to kick o...
Status: RESOLVED FIXED
Alias: None
Product: clang
Classification: Unclassified
Component: -New Bugs (show other bugs)
Version: trunk
Hardware: PC Windows NT
: P normal
Assignee: Reid Kleckner
URL:
Keywords: build-problem, portability
Depends on:
Blocks:
 
Reported: 2013-04-20 18:48 PDT by Stefan van Kessel
Modified: 2013-04-22 14:10 PDT (History)
6 users (show)

See Also:
Fixed By Commit(s):


Attachments
possible patch (1.22 KB, application/octet-stream)
2013-04-20 18:48 PDT, Stefan van Kessel
Details
fixed issue with first patch (setting pointers in args) (1.97 KB, patch)
2013-04-20 20:38 PDT, Stefan van Kessel
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Stefan van Kessel 2013-04-20 18:48:05 PDT
Created attachment 10389 [details]
possible patch

On Windows, invoking "clang test.c" makes clang hang during startup if %INCLUDE% ends in a \ and presumably some similar conditions as well.

The problem has been described by user qble on http://stackoverflow.com/questions/15992645/clang-compiler-hangs-on-windows

When clang gets invoked with actual work, it assembles a list of arguments and kicks of a new process. Among those arguments is the include environment variable on windows. In case this variable contains spaces or some other (e.g. "C:\Program Files\SomeVendor\Include\") or some other special characters, clang adds quotation marks around this string so that it stays a single argument to the new process. If the argument ends in an uneven number (typically one) of backslashes though, the quotation mark becomes escaped and thus instead of a invocation like

clang "C:\Program Files\SomeVendor\Include\\" someOtherArg

which makes 

argv[1] = "C:\\Program Files\\SomeVendor\\Include\\"
argv[2] = "someOtherArg"

it becomes

clang "C:\Program Files\SomeVendor\Include\" someOtherArg

which is a unterminated 

argv[1] =  "C:\\Program Files\\SomeVendor\\Include\" someOtherArg ...


The problem can easily be fixed by escaping uneven trailing backslashes by just adding another one. I have added a pach to llvm\lib\Support\Windows which fixes the problem, I didn't feel like messing around with c-strings like the rest of that code does. If you feel strongly about it, I could also write a patch in keeping with the c-style code and without unnecessary copies (though I don't think that code is remotely performance critical)
Comment 1 Stefan van Kessel 2013-04-20 20:38:15 PDT
Created attachment 10390 [details]
fixed issue with first patch (setting pointers in args)
Comment 2 Reid Kleckner 2013-04-22 09:27:17 PDT
Thanks for the report.  BTW, here's the best reference I could find on MSDN for the quoting rules:
http://msdn.microsoft.com/en-us/library/17w5ykft%28v=vs.85%29.aspx

I think your solution may be flawed for strings with odd numbers of backslashes like "pretend\arg\\\".  Your code will transform that to "pretend\arg\\\\", which when unquoted in the child process should become "pretend\arg\\".

I'll look into implementing the proper quoting rules.
Comment 3 Stefan van Kessel 2013-04-22 09:56:50 PDT
Yes, you're right. I was mistaken about the parsing rules. Your link is very helpful indeed.

My patch is incorrect for arguments ending in more than one backslash and not only when the number is uneven. For example:

C:\dev\test\Win32\Debug>test.exe this isatest\\
argv[0] = "qt_console_project.exe"
argv[1] = "this"
argv[2] = "isatest\\"

C:\dev\test\Win32\Debug>test.exe "this isatest\\"
argv[0] = "qt_console_project.exe"
argv[1] = "this isatest\"

I believe the number of trailing \ has to be doubled if and only if quotes will be added, right?
Comment 4 Reid Kleckner 2013-04-22 14:10:55 PDT
Right, as well as before any quote in the original string.

I implemented that in r180035, and your test case works for me with clang now.