[llvm-windres] Change the interpretation of --preprocessor to match Binutils 2.36 (#75391)

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
This commit is contained in:
Martin Storsjö 2023-12-18 12:31:05 +02:00 committed by GitHub
parent d777504355
commit 9e3d915d8e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 12 additions and 41 deletions

View File

@ -6,8 +6,8 @@
; 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
; 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
; 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"{{$}}
; RUN: llvm-windres -### --preprocessor "i686-w64-mingw32-gcc -E -DFOO=\\\"foo\\ bar\\\"" %p/Inputs/empty.rc %t.res | FileCheck %s --check-prefix=CHECK2
; CHECK2: {{^}} "{{.*}}i686-w64-mingw32-gcc" "-E" "-DFOO=\"foo bar\"" "{{.*}}empty.rc" "-o" "{{.*}}preproc-{{.*}}.rc"{{$}}
; 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
; CHECK2: {{^}} "{{.*}}i686-w64-mingw32-gcc" "-E" "-D" "FOO=\"foo bar\"" "{{.*}}empty.rc" "-o" "{{.*}}preproc-{{.*}}.rc"{{$}}
;; Test resolving the --preprocessor executable from PATH
@ -22,3 +22,8 @@
; RUN: not llvm-windres --preprocessor intentionally-missing-executable %p/Inputs/empty.rc %t.res 2>&1 | FileCheck %s --check-prefix=CHECK4
; CHECK4: llvm-rc: Preprocessing failed: Executable "intentionally-missing-executable" doesn't exist!
;; Test --preprocessor with an argument with spaces.
; RUN: llvm-windres -### --preprocessor "path with spaces/gcc" %p/Inputs/empty.rc %t.res | FileCheck %s --check-prefix=CHECK5
; CHECK5: {{^}} "path with spaces/gcc" "{{.*}}empty.rc" "-o" "{{.*}}preproc-{{.*}}.rc"{{$}}

View File

@ -209,7 +209,7 @@ struct RcOptions {
bool Preprocess = true;
bool PrintCmdAndExit = false;
std::string Triple;
std::vector<std::string> PreprocessCmd;
std::optional<std::string> Preprocessor;
std::vector<std::string> PreprocessArgs;
std::string InputFile;
@ -229,7 +229,7 @@ struct RcOptions {
void preprocess(StringRef Src, StringRef Dst, const RcOptions &Opts,
const char *Argv0) {
std::string Clang;
if (Opts.PrintCmdAndExit || !Opts.PreprocessCmd.empty()) {
if (Opts.PrintCmdAndExit || Opts.Preprocessor) {
Clang = "clang";
} else {
ErrorOr<std::string> ClangOrErr = findClang(Argv0, Opts.Triple);
@ -249,10 +249,9 @@ void preprocess(StringRef Src, StringRef Dst, const RcOptions &Opts,
Clang, "--driver-mode=gcc", "-target", Opts.Triple, "-E",
"-xc", "-DRC_INVOKED"};
std::string PreprocessorExecutable;
if (!Opts.PreprocessCmd.empty()) {
if (Opts.Preprocessor) {
Args.clear();
for (const auto &S : Opts.PreprocessCmd)
Args.push_back(S);
Args.push_back(*Opts.Preprocessor);
if (!sys::fs::can_execute(Args[0])) {
if (auto P = sys::findProgramByName(Args[0])) {
PreprocessorExecutable = *P;
@ -342,36 +341,6 @@ std::string unescape(StringRef S) {
return Out;
}
std::vector<std::string> unescapeSplit(StringRef S) {
std::vector<std::string> OutArgs;
std::string Out;
bool InQuote = false;
for (int I = 0, E = S.size(); I < E; I++) {
if (S[I] == '\\') {
if (I + 1 < E)
Out.push_back(S[++I]);
else
fatalError("Unterminated escape");
continue;
}
if (S[I] == '"') {
InQuote = !InQuote;
continue;
}
if (S[I] == ' ' && !InQuote) {
OutArgs.push_back(Out);
Out.clear();
continue;
}
Out.push_back(S[I]);
}
if (InQuote)
fatalError("Unterminated quote");
if (!Out.empty())
OutArgs.push_back(Out);
return OutArgs;
}
RcOptions parseWindresOptions(ArrayRef<const char *> ArgsArr,
ArrayRef<const char *> InputArgsArray,
std::string Prefix) {
@ -506,11 +475,8 @@ RcOptions parseWindresOptions(ArrayRef<const char *> ArgsArr,
break;
}
}
// TODO: If --use-temp-file is set, we shouldn't be unescaping
// the --preprocessor argument either, only splitting it.
if (InputArgs.hasArg(WINDRES_preprocessor))
Opts.PreprocessCmd =
unescapeSplit(InputArgs.getLastArgValue(WINDRES_preprocessor));
Opts.Preprocessor = InputArgs.getLastArgValue(WINDRES_preprocessor);
Opts.Params.CodePage = CpWin1252; // Different default
if (InputArgs.hasArg(WINDRES_codepage)) {