[llvm-branch-commits] [llvm] 466677b - [llvm-windres] Implement the windres flag --use-temp-file
Tobias Hieta via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Sun Sep 3 23:30:34 PDT 2023
Author: Martin Storsjö
Date: 2023-09-04T08:26:18+02:00
New Revision: 466677b126855c79a05a1ebc111eea48053ad4ef
URL: https://github.com/llvm/llvm-project/commit/466677b126855c79a05a1ebc111eea48053ad4ef
DIFF: https://github.com/llvm/llvm-project/commit/466677b126855c79a05a1ebc111eea48053ad4ef.diff
LOG: [llvm-windres] Implement the windres flag --use-temp-file
Whether a temp file or a pipe is used for preprocessing is an
internal detail, this flag has a notable effect on the preprocessing
in GNU windres. Without this flag, GNU windres passes command
arguments as-is to popen(), which means they get evaluated by a
shell without being re-escaped for this case. To mimic this,
llvm-windres has manually tried to unescape arguments.
When GNU windres is given the --use-temp-file flag, it uses a
different API for invoking the preprocessor, and this API takes care
of preserving special characters in the command line arguments.
For users of GNU windres, this means that by using --use-temp-file,
they don't need to do the (quite terrible) double escaping of
quotes/spaces etc.
The xz project uses the --use-temp-file flag when invoking
GNU windres, see
https://github.com/tukaani-project/xz/commit/6b117d3b1fe91eb26d533ab16a2e552f84148d47.
However as llvm-windres didn't implement this flag and just
assumed the GNU windres popen() behaviour, they had to use a
different codepath for llvm-windres.
That separate codepath for llvm-windres broke later when llvm-windres
got slightly more accurate unescaping of lone quotes in
0f4c6b120f21d582ab7c5c4f2b2a475086c34938 /
https://reviews.llvm.org/D146848 (fixing a discrepancy to GNU
windres as found in https://github.com/llvm/llvm-project/issues/57334),
and this was reported in
https://github.com/mstorsjo/llvm-mingw/issues/363.
Not touching the implementation of the --preprocessor option
with respect to the --use-temp-file flag; that option is doubly
tricky as GNU windres changed its behaviour in a backwards incompatible
way recently (and llvm-windres currently matches the old behaviour).
(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;hp=3abbafc2aacc6706fea3e3e326e2f08d107c3672
for the behaviour change.)
Differential Revision: https://reviews.llvm.org/D159223
(cherry picked from commit 2bcc0fdc58a220cb9921b47ec8a32c85f2511a47)
Added:
Modified:
llvm/test/tools/llvm-rc/windres-preproc.test
llvm/tools/llvm-rc/WindresOpts.td
llvm/tools/llvm-rc/llvm-rc.cpp
Removed:
################################################################################
diff --git a/llvm/test/tools/llvm-rc/windres-preproc.test b/llvm/test/tools/llvm-rc/windres-preproc.test
index 888be03f7d9e4b7..e55195b3a4d2802 100644
--- a/llvm/test/tools/llvm-rc/windres-preproc.test
+++ b/llvm/test/tools/llvm-rc/windres-preproc.test
@@ -4,6 +4,7 @@
; REQUIRES: shell
; 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" "{{.*}}empty.rc" "-o" "{{.*}}preproc-{{.*}}.rc" "-I" "{{.*}}incdir1" "-I" "{{.*}}incdir2" "-D" "FOO1=\"foo bar\"" "-U" "FOO2" "-D" "FOO3" "-DFOO4=\"baz baz\"" "-D" "FOO5=bar"{{$}}
; 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"{{$}}
diff --git a/llvm/tools/llvm-rc/WindresOpts.td b/llvm/tools/llvm-rc/WindresOpts.td
index 3c75c85ece0f6e1..42a56dbfda4cd89 100644
--- a/llvm/tools/llvm-rc/WindresOpts.td
+++ b/llvm/tools/llvm-rc/WindresOpts.td
@@ -48,6 +48,10 @@ defm codepage : LongShort<"c", "codepage", "Default codepage to use">;
defm language : LongShort<"l", "language", "Default language to use (0x0-0xffff)">;
+def use_temp_file: Flag<["--"], "use-temp-file">,
+ HelpText<"Mimic GNU windres preprocessor option handling "
+ "(don't unescape preprocessor options)">;
+
defm verbose : F<"v", "verbose", "Enable verbose output">;
defm version : F<"V", "version", "Display version">;
@@ -57,6 +61,3 @@ defm help : F<"h", "help", "Display this message and exit">;
def _HASH_HASH_HASH : Flag<["-"], "###">;
def no_preprocess : Flag<["--"], "no-preprocess">;
-
-// Unimplemented options for compatibility
-def use_temp_file: Flag<["--"], "use-temp-file">;
diff --git a/llvm/tools/llvm-rc/llvm-rc.cpp b/llvm/tools/llvm-rc/llvm-rc.cpp
index 233b888546a81e5..0caa8117cb70ba3 100644
--- a/llvm/tools/llvm-rc/llvm-rc.cpp
+++ b/llvm/tools/llvm-rc/llvm-rc.cpp
@@ -473,7 +473,14 @@ RcOptions parseWindresOptions(ArrayRef<const char *> ArgsArr,
// done this double escaping) probably is confined to cases like these
// quoted string defines, and those happen to work the same across unix
// and windows.
- std::string Unescaped = unescape(Arg->getValue());
+ //
+ // If GNU windres is executed with --use-temp-file, it doesn't use
+ // popen() to invoke the preprocessor, but uses another function which
+ // actually preserves tricky characters better. To mimic this behaviour,
+ // don't unescape arguments here.
+ std::string Value = Arg->getValue();
+ if (!InputArgs.hasArg(WINDRES_use_temp_file))
+ Value = unescape(Value);
switch (Arg->getOption().getID()) {
case WINDRES_include_dir:
// Technically, these are handled the same way as e.g. defines, but
@@ -487,17 +494,19 @@ RcOptions parseWindresOptions(ArrayRef<const char *> ArgsArr,
break;
case WINDRES_define:
Opts.PreprocessArgs.push_back("-D");
- Opts.PreprocessArgs.push_back(Unescaped);
+ Opts.PreprocessArgs.push_back(Value);
break;
case WINDRES_undef:
Opts.PreprocessArgs.push_back("-U");
- Opts.PreprocessArgs.push_back(Unescaped);
+ Opts.PreprocessArgs.push_back(Value);
break;
case WINDRES_preprocessor_arg:
- Opts.PreprocessArgs.push_back(Unescaped);
+ Opts.PreprocessArgs.push_back(Value);
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));
More information about the llvm-branch-commits
mailing list