[llvm-config] Quote and escape paths

[aganea]

If any of the printed paths by llvm-config contains quotes, spaces, backslashes or dollar sign characters, these paths will be quoted and the corresponding characters will be escaped.

Following discussion in #76304

Fixes #28117

[glandium]

It seems the "if necessary" part was not implemented, because it now quotes even when quotes are not necessary, which is likely to break a ton of things.

Edit: examples:

$ /builds/worker/fetches/clang/bin/llvm-config --libs
-l"LLVM-19git"
$ /builds/worker/fetches/clang/bin/llvm-config --cflags
-I"/builds/worker/fetches/clang/include"  -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS

[glandium]

Example of subtle breakage: cctools-port ld64's support for LTO breaks for some reason because of this change.

[aganea]

Example of subtle breakage: cctools-port ld64's support for LTO breaks for some reason because of this change.

Are you able to debug what is wrong there? Maybe because they are collating path components to an already quoted path, and the bintools don't like that? (this works on Windows with cl.exe and clang-cl.exe at least).

I think that if we don't introduce a breaking change with LLVM 19 in this regards (quoted strings emitted by llvm-config), this patch will be worthless. If we add quotes only when required, it won't work (as you're pointing out) in the downstream usages. Evidently some of the paths emitted by llvm-config might not require escaping, since they are alone on a single line, and could be quoted later by consuming scripts (like --libdir or --includedir).

Any other suggestions?

+@MaskRay

[glandium]

I gave one subtle example, but here's another one less subtle: https://github.com/AFLplusplus/AFLplusplus/blob/stable/GNUmakefile.llvm#L420 this simple straightforward use breaks completely (can't find includes when compiling).

[kongy]

Can you please revert the change while investigating a fix? Our downstream ToT builder is broken because of this change.

[aganea]

Reverted. @kongy do you have a reproducer that I could test locally by chance?

[kongy]

Our build script invokes llvm-config and feeds the target list to ninja. With the extra unnecessary quotes, ninja does not find the correct target.

DEBUG:utils:subprocess.run:02:40:16 /tmpfs/src/git/out/stage2-install/bin/llvm-config --libs object --libnames --link-static
DEBUG:utils:subprocess.run:02:40:16 /tmpfs/src/git/prebuilts/build-tools/linux_musl-x86/bin/ninja '"libLLVMObject.a"' '"libLLVMTextAPI.a"' '"libLLVMMCParser.a"' '"libLLVMIRReader.a"' '"libLLVMAsmParser.a"' '"libLLVMMC.a"' '"libLLVMDebugInfoCodeView.a"' '"libLLVMBitReader.a"' '"libLLVMCore.a"' '"libLLVMRemarks.a"' '"libLLVMBinaryFormat.a"' '"libLLVMTargetParser.a"' '"libLLVMBitstreamReader.a"' '"libLLVMSupport.a"' '"libLLVMDemangle.a"'
ninja: error: unknown target '"libLLVMObject.a"', did you mean 'libLLVMObject.a'?

[aganea]

While we could avoid unnecessary escaping for the libraries above, escaping paths in general (like this patch does) would need some modifications (I think) in scripts using llvm-config.

What do you all think about adding a new flag such as --escaped that allows potentially quoting and escaping paths, while keeping the old behavior when the flag is not provided?

[tothambrus11]

Another approach would have been to only add quotes when there is a space in the path. If old tooling relied on such things, it was broken anyways (there is no way of correctly splitting unless we add some hard assumptions). But --escaped is good for backwards compatibility in all cases.

[aganea]

Please see #103397