Skip to content

Commit 542eb53

Browse files
committed
[llvm-windres] Change the interpretation of --preprocessor to match Binutils 2.36
Binutils 2.36 had a somewhat controversial change in how the --preprocessor option was handled in GNU windres; previously, the option was interpreted as a part of the command string, potentially containing multiple arguments (which even was hinted at in the documentation). In Binutils 2.36, this was changed to interpret the --preprocessor argument as one argument (possibly containing spaces) pointing at the preprocessor executable. The existing behaviour where implicit arguments like -E -xc -DRC_INVOKED are dropped if --preprocessor is specified, was kept. This was a breaking change for some users of GNU windres, see https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=21c33bcbe36377abf01614fb1b9be439a3b6de20, https://sourceware.org/bugzilla/show_bug.cgi?id=27594, and https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=5edb8e3f5ad8d74a83fc0df7f6e4514eed0aa77f. As multiple years have passed since, the behaviour change seems to be here to stay, and any users of the previous form of the option have been forced to avoid this construct. Thus update llvm-windres to match the new way Binutils of handling this option. One construct for specifying the path to the preprocessor, which works both before and after binutils 2.36 (and this change in llvm-windres) is to specify options like this: --preprocessor path/to/executable --preprocessor-arg -E --preprocessor-arg -xc -DRC_INVOKED
1 parent 1709e8c commit 542eb53

File tree

2 files changed

+12
-41
lines changed

2 files changed

+12
-41
lines changed

llvm/test/tools/llvm-rc/windres-preproc.test

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@
66
; RUN: llvm-windres -### --include-dir %p/incdir1 --include %p/incdir2 "-DFOO1=\\\"foo bar\\\"" -UFOO2 -D FOO3 --preprocessor-arg "-DFOO4=\\\"baz baz\\\"" -DFOO5=\"bar\" %p/Inputs/empty.rc %t.res | FileCheck %s --check-prefix=CHECK1
77
; RUN: llvm-windres -### --include-dir %p/incdir1 --include %p/incdir2 "-DFOO1=\"foo bar\"" -UFOO2 -D FOO3 --preprocessor-arg "-DFOO4=\"baz baz\"" "-DFOO5=bar" %p/Inputs/empty.rc %t.res --use-temp-file | FileCheck %s --check-prefix=CHECK1
88
; CHECK1: {{^}} "clang" "--driver-mode=gcc" "-target" "{{.*}}-{{.*}}{{mingw32|windows-gnu}}" "-E" "-xc" "-DRC_INVOKED" "-I" "{{.*}}incdir1" "-I" "{{.*}}incdir2" "-D" "FOO1=\"foo bar\"" "-U" "FOO2" "-D" "FOO3" "-DFOO4=\"baz baz\"" "-D" "FOO5=bar" "{{.*}}empty.rc" "-o" "{{.*}}preproc-{{.*}}.rc"{{$}}
9-
; RUN: llvm-windres -### --preprocessor "i686-w64-mingw32-gcc -E -DFOO=\\\"foo\\ bar\\\"" %p/Inputs/empty.rc %t.res | FileCheck %s --check-prefix=CHECK2
10-
; CHECK2: {{^}} "{{.*}}i686-w64-mingw32-gcc" "-E" "-DFOO=\"foo bar\"" "{{.*}}empty.rc" "-o" "{{.*}}preproc-{{.*}}.rc"{{$}}
9+
; RUN: llvm-windres -### --preprocessor "i686-w64-mingw32-gcc" --preprocessor-arg -E "-DFOO=\\\"foo bar\\\"" %p/Inputs/empty.rc %t.res | FileCheck %s --check-prefix=CHECK2
10+
; CHECK2: {{^}} "{{.*}}i686-w64-mingw32-gcc" "-E" "-D" "FOO=\"foo bar\"" "{{.*}}empty.rc" "-o" "{{.*}}preproc-{{.*}}.rc"{{$}}
1111

1212
;; Test resolving the --preprocessor executable from PATH
1313

@@ -22,3 +22,8 @@
2222

2323
; RUN: not llvm-windres --preprocessor intentionally-missing-executable %p/Inputs/empty.rc %t.res 2>&1 | FileCheck %s --check-prefix=CHECK4
2424
; CHECK4: llvm-rc: Preprocessing failed: Executable "intentionally-missing-executable" doesn't exist!
25+
26+
;; Test --preprocessor with an argument with spaces.
27+
28+
; RUN: llvm-windres -### --preprocessor "path with spaces/gcc" %p/Inputs/empty.rc %t.res | FileCheck %s --check-prefix=CHECK5
29+
; CHECK5: {{^}} "path with spaces/gcc" "{{.*}}empty.rc" "-o" "{{.*}}preproc-{{.*}}.rc"{{$}}

llvm/tools/llvm-rc/llvm-rc.cpp

Lines changed: 5 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ struct RcOptions {
209209
bool Preprocess = true;
210210
bool PrintCmdAndExit = false;
211211
std::string Triple;
212-
std::vector<std::string> PreprocessCmd;
212+
std::optional<std::string> Preprocessor;
213213
std::vector<std::string> PreprocessArgs;
214214

215215
std::string InputFile;
@@ -229,7 +229,7 @@ struct RcOptions {
229229
void preprocess(StringRef Src, StringRef Dst, const RcOptions &Opts,
230230
const char *Argv0) {
231231
std::string Clang;
232-
if (Opts.PrintCmdAndExit || !Opts.PreprocessCmd.empty()) {
232+
if (Opts.PrintCmdAndExit || Opts.Preprocessor) {
233233
Clang = "clang";
234234
} else {
235235
ErrorOr<std::string> ClangOrErr = findClang(Argv0, Opts.Triple);
@@ -249,10 +249,9 @@ void preprocess(StringRef Src, StringRef Dst, const RcOptions &Opts,
249249
Clang, "--driver-mode=gcc", "-target", Opts.Triple, "-E",
250250
"-xc", "-DRC_INVOKED"};
251251
std::string PreprocessorExecutable;
252-
if (!Opts.PreprocessCmd.empty()) {
252+
if (Opts.Preprocessor) {
253253
Args.clear();
254-
for (const auto &S : Opts.PreprocessCmd)
255-
Args.push_back(S);
254+
Args.push_back(*Opts.Preprocessor);
256255
if (!sys::fs::can_execute(Args[0])) {
257256
if (auto P = sys::findProgramByName(Args[0])) {
258257
PreprocessorExecutable = *P;
@@ -342,36 +341,6 @@ std::string unescape(StringRef S) {
342341
return Out;
343342
}
344343

345-
std::vector<std::string> unescapeSplit(StringRef S) {
346-
std::vector<std::string> OutArgs;
347-
std::string Out;
348-
bool InQuote = false;
349-
for (int I = 0, E = S.size(); I < E; I++) {
350-
if (S[I] == '\\') {
351-
if (I + 1 < E)
352-
Out.push_back(S[++I]);
353-
else
354-
fatalError("Unterminated escape");
355-
continue;
356-
}
357-
if (S[I] == '"') {
358-
InQuote = !InQuote;
359-
continue;
360-
}
361-
if (S[I] == ' ' && !InQuote) {
362-
OutArgs.push_back(Out);
363-
Out.clear();
364-
continue;
365-
}
366-
Out.push_back(S[I]);
367-
}
368-
if (InQuote)
369-
fatalError("Unterminated quote");
370-
if (!Out.empty())
371-
OutArgs.push_back(Out);
372-
return OutArgs;
373-
}
374-
375344
RcOptions parseWindresOptions(ArrayRef<const char *> ArgsArr,
376345
ArrayRef<const char *> InputArgsArray,
377346
std::string Prefix) {
@@ -506,11 +475,8 @@ RcOptions parseWindresOptions(ArrayRef<const char *> ArgsArr,
506475
break;
507476
}
508477
}
509-
// TODO: If --use-temp-file is set, we shouldn't be unescaping
510-
// the --preprocessor argument either, only splitting it.
511478
if (InputArgs.hasArg(WINDRES_preprocessor))
512-
Opts.PreprocessCmd =
513-
unescapeSplit(InputArgs.getLastArgValue(WINDRES_preprocessor));
479+
Opts.Preprocessor = InputArgs.getLastArgValue(WINDRES_preprocessor);
514480

515481
Opts.Params.CodePage = CpWin1252; // Different default
516482
if (InputArgs.hasArg(WINDRES_codepage)) {

0 commit comments

Comments
 (0)