[llvm] [llvm-windres] Change the interpretation of --preprocessor to match Binutils 2.36 (PR #75391)
Martin Storsjö via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 15 10:17:30 PST 2023
https://github.com/mstorsjo updated https://github.com/llvm/llvm-project/pull/75391
>From 542eb5309a892e9ab981369cb7acf530486ae0d8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?= <martin at martin.st>
Date: Wed, 13 Dec 2023 14:39:18 +0200
Subject: [PATCH] [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
---
llvm/test/tools/llvm-rc/windres-preproc.test | 9 +++-
llvm/tools/llvm-rc/llvm-rc.cpp | 44 +++-----------------
2 files changed, 12 insertions(+), 41 deletions(-)
diff --git a/llvm/test/tools/llvm-rc/windres-preproc.test b/llvm/test/tools/llvm-rc/windres-preproc.test
index 74e888614aa2b9..13f82299a074bb 100644
--- a/llvm/test/tools/llvm-rc/windres-preproc.test
+++ b/llvm/test/tools/llvm-rc/windres-preproc.test
@@ -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"{{$}}
diff --git a/llvm/tools/llvm-rc/llvm-rc.cpp b/llvm/tools/llvm-rc/llvm-rc.cpp
index 27fb0309e0ee54..78ab96492acc75 100644
--- a/llvm/tools/llvm-rc/llvm-rc.cpp
+++ b/llvm/tools/llvm-rc/llvm-rc.cpp
@@ -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)) {
More information about the llvm-commits
mailing list