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)); _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
