[lvm-windres] Try to match GNU windres regarding handling of unescaped quotes

[mstorsjo]

Summary

Some background context: GNU windres invokes the preprocessor in
a subprocess. Some windres options are passed through to the
preproocessor, e.g. -D options for predefining defines.
When GNU windres passes these options onwards, it takes the options
in exact the form they are received (in argv or similar) and
assembles them into a single preprocessor command string which gets
interpreted by a shell (IIRC via the popen() function, or similar).

When LLVM invokes subprocesses, it does so via APIs that take
properly split argument vectors, to avoid needing to worry about
shell quoting/escaping/unescaping. But in the case of LLVM windres,
we have to emulate the effect of the shell parsing done by popen().

Most of the relevant cases are already taken care of here, but this
patch fixes an uncommon case encountered in
https://github.com/llvm/llvm-project/issues/57334.
(This case is uncommon since it doesn't do what one would want to;
the quotes need to be escaped more to work as intended through
the popen() shell).

[mati865]

Oh, that's unfortunate.
Maybe llvm-windres should generate a warning in such case?

[mstorsjo]

In D146848#4221240, @mati865 wrote:

Oh, that's unfortunate.
Maybe llvm-windres should generate a warning in such case?

Hmm, maybe... (I'd need to think about it again to sort my thoughts out about it; I had written this patch 6 months ago but never took the time to finish it to have it reviewed.)

[mstorsjo]

In D146848#4221771, @mstorsjo wrote:

In D146848#4221240, @mati865 wrote:

Oh, that's unfortunate.
Maybe llvm-windres should generate a warning in such case?

Hmm, maybe... (I'd need to think about it again to sort my thoughts out about it; I had written this patch 6 months ago but never took the time to finish it to have it reviewed.)

I think I'll skip adding a warning in this case, as I think it feels somewhat out of place. Whenever debugging such uses, we should have mostly consistent behaviour between GNU windres and LLVM windres anyway. The user can see the actual preprocessing command that gets run by running windres with -v/--verbose too, which is helpful for figuring out the right amount of quoting/escaping.

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

-; RUN: llvm-windres -### --include-dir %p/incdir1 --include %p/incdir2 "-DFOO1=\\\"foo bar\\\"" -UFOO2 -D FOO3 --preprocessor-arg "-DFOO4=\\\"baz baz\\\"" %p/Inputs/empty.rc %t.res | 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\""{{$}}
+; 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
+; 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"{{$}}

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

std::string unescape(StringRef S) {
   Out.reserve(S.size());
   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;
+    } else if (S[I] == '"') {
+      // This eats an individual unescaped quote, like a shell would do.
+      continue;
     }
     Out.push_back(S[I]);
   }
   return Out;
 }
 
 std::vector<std::string> unescapeSplit(StringRef S) {
   std::vector<std::string> OutArgs;