[llvm] [llvm-rc] Concatenate consecutive string tokens (PR #68685)

Martin Storsjö via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 12 05:04:46 PDT 2023


https://github.com/mstorsjo updated https://github.com/llvm/llvm-project/pull/68685

>From aca00726fed749f693e93bd95d6c08f46dc9f54a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?= <martin at martin.st>
Date: Fri, 31 Jul 2020 14:31:57 +0300
Subject: [PATCH] [llvm-rc] Concatenate consecutive string tokens in windres
 mode

A number of constructs allow more than one string literal
for what represents one single string - version info values,
string tables, user data resources, etc. This mostly works
like how a C compiler merges consecutive string literals,
like "foo" "bar" as producing the same as "foobar". This is
beneficial for producing strings with some elements provided
by the preprocessor.

MS rc.exe only supports this in a few fixed locations (most of
which are already supported), while GNU windres supports this
essentially anywhere in any string. See
b989fcbae6f179ad887d19ceef83ace1c00b87cc for one recent change
that extended support for this in one specific resource.

A reasonable use case for multiple concatenated string literals
that GNU windres accepts is `1 ICON DIR "/name.ico"`, where the
directory is provided via the preprocessor, expanding to another
string literal; this is https://github.com/llvm/llvm-project/issues/51286.

To support this, extend the tokenizer to concatenate consecutive
quoted string tokens, when invoked in windres mode.

For cases where the difference between narrow and wide strings
is significant (e.g. in userdata resources), GNU windres treats
`L"aaa" "bbb"` the same as `L"aaabbb"`, contrary to rc.exe
which treats it as the wide string `"aaa"` followed by the
narrow string `"bbb"`. Similarly, windres rejects the sequence
`"aaa" L"bbb"`.

However, in contexts where the end result actually is UTF16,
GNU windres does accept such mismatched string representations.
Therefore, it seems clear that GNU windres doesn't do the
merging on the string token level.

Many of the existing windres tests happen to use
tag-stringtable-basic.rc as test input; this file contains a
case of `"wor" L"ld"` which now is rejected by the tokenizer in
windres mode - therefore switch those tests, where the actual input
file doesn't matter, to a different file.
---
 llvm/test/tools/llvm-rc/Inputs/split-path.rc  |  1 +
 .../Inputs/tokens-windres-invalid-concat.rc   |  1 +
 .../tools/llvm-rc/Inputs/tokens-windres.rc    |  3 +
 llvm/test/tools/llvm-rc/split-path.test       |  6 ++
 llvm/test/tools/llvm-rc/windres-format.test   |  8 +--
 llvm/test/tools/llvm-rc/windres-prefix.test   |  2 +-
 llvm/test/tools/llvm-rc/windres-target.test   |  8 +--
 .../test/tools/llvm-rc/windres-tokenizer.test | 13 +++++
 llvm/tools/llvm-rc/ResourceScriptToken.cpp    | 55 ++++++++++++++++++-
 llvm/tools/llvm-rc/ResourceScriptToken.h      |  4 +-
 llvm/tools/llvm-rc/llvm-rc.cpp                |  6 +-
 11 files changed, 93 insertions(+), 14 deletions(-)
 create mode 100644 llvm/test/tools/llvm-rc/Inputs/split-path.rc
 create mode 100644 llvm/test/tools/llvm-rc/Inputs/tokens-windres-invalid-concat.rc
 create mode 100644 llvm/test/tools/llvm-rc/Inputs/tokens-windres.rc
 create mode 100644 llvm/test/tools/llvm-rc/split-path.test
 create mode 100644 llvm/test/tools/llvm-rc/windres-tokenizer.test

diff --git a/llvm/test/tools/llvm-rc/Inputs/split-path.rc b/llvm/test/tools/llvm-rc/Inputs/split-path.rc
new file mode 100644
index 000000000000000..4f60b6f78ae15c5
--- /dev/null
+++ b/llvm/test/tools/llvm-rc/Inputs/split-path.rc
@@ -0,0 +1 @@
+100 ICON "subdir" "/icon-new.ico"
diff --git a/llvm/test/tools/llvm-rc/Inputs/tokens-windres-invalid-concat.rc b/llvm/test/tools/llvm-rc/Inputs/tokens-windres-invalid-concat.rc
new file mode 100644
index 000000000000000..5051ffaed513f19
--- /dev/null
+++ b/llvm/test/tools/llvm-rc/Inputs/tokens-windres-invalid-concat.rc
@@ -0,0 +1 @@
+"narrow" L"wide"
diff --git a/llvm/test/tools/llvm-rc/Inputs/tokens-windres.rc b/llvm/test/tools/llvm-rc/Inputs/tokens-windres.rc
new file mode 100644
index 000000000000000..46c6312002a55ba
--- /dev/null
+++ b/llvm/test/tools/llvm-rc/Inputs/tokens-windres.rc
@@ -0,0 +1,3 @@
+L"wide" " also wide",
+"narrow" " also narrow",
+L"wide" L" more wide",
diff --git a/llvm/test/tools/llvm-rc/split-path.test b/llvm/test/tools/llvm-rc/split-path.test
new file mode 100644
index 000000000000000..b396d805df5c908
--- /dev/null
+++ b/llvm/test/tools/llvm-rc/split-path.test
@@ -0,0 +1,6 @@
+; RUN: rm -rf %t
+; RUN: mkdir %t
+; RUN: cd %t
+; RUN: mkdir subdir
+; RUN: cp %p/Inputs/icon-new.ico subdir
+; RUN: llvm-windres --no-preprocess %p/Inputs/split-path.rc %t/split-path.res
diff --git a/llvm/test/tools/llvm-rc/windres-format.test b/llvm/test/tools/llvm-rc/windres-format.test
index 81e6147d94b4890..b647be6da6a61e3 100644
--- a/llvm/test/tools/llvm-rc/windres-format.test
+++ b/llvm/test/tools/llvm-rc/windres-format.test
@@ -2,11 +2,11 @@
 ; from file suffixes.
 
 ; RUN: rm -f %t.res
-; RUN: llvm-windres --no-preprocess %p/Inputs/tag-stringtable-basic.rc %t.res
+; RUN: llvm-windres --no-preprocess %p/Inputs/tag-versioninfo.rc %t.res
 ; RUN: llvm-readobj %t.res | FileCheck %s --check-prefix=CHECK-RES
 
 ; RUN: rm -f %t.o
-; RUN: llvm-windres --no-preprocess -F pe-x86-64 %p/Inputs/tag-stringtable-basic.rc %t.o
+; RUN: llvm-windres --no-preprocess -F pe-x86-64 %p/Inputs/tag-versioninfo.rc %t.o
 ; RUN: llvm-readobj --coff-resources %t.o | FileCheck %s --check-prefix=CHECK-OBJ
 
 ; RUN: rm -f %t.obj
@@ -16,7 +16,7 @@
 ; Check that we can specify the input/output file types explicitly.
 ; Also check options for specifying the input/output file names.
 
-; RUN: cat %p/Inputs/tag-stringtable-basic.rc > %t-anonymous
+; RUN: cat %p/Inputs/tag-versioninfo.rc > %t-anonymous
 ; RUN: rm -f %t-anonymous2
 ; RUN: llvm-windres --no-preprocess -O res -J rc -o %t-anonymous2 -i %t-anonymous
 ; RUN: llvm-readobj %t-anonymous2 | FileCheck %s --check-prefix=CHECK-RES
@@ -25,7 +25,7 @@
 ; RUN: llvm-windres -F pe-x86-64 -J res -O coff -i%t-anonymous2 -o%t-anonymous3
 ; RUN: llvm-readobj --coff-resources %t-anonymous3 | FileCheck %s --check-prefix=CHECK-OBJ
 
-; CHECK-RES: Resource type (int): STRINGTABLE
+; CHECK-RES: Resource type (int):
 
 ; CHECK-OBJ: Format: COFF-x86-64
 ; CHECK-OBJ: Resources [
diff --git a/llvm/test/tools/llvm-rc/windres-prefix.test b/llvm/test/tools/llvm-rc/windres-prefix.test
index 9d24bf9ee7b2aca..f085693610fc110 100644
--- a/llvm/test/tools/llvm-rc/windres-prefix.test
+++ b/llvm/test/tools/llvm-rc/windres-prefix.test
@@ -10,7 +10,7 @@
 
 ; Check that the triple prefix also affects the output object file type.
 
-; RUN: %t/aarch64-w64-mingw32-windres --no-preprocess %p/Inputs/tag-stringtable-basic.rc %t.o
+; RUN: %t/aarch64-w64-mingw32-windres --no-preprocess %p/Inputs/tag-versioninfo.rc %t.o
 ; RUN: llvm-readobj --coff-resources %t.o | FileCheck %s --check-prefix=CHECK-OBJ
 
 ; CHECK-OBJ: Format: COFF-ARM64
diff --git a/llvm/test/tools/llvm-rc/windres-target.test b/llvm/test/tools/llvm-rc/windres-target.test
index a832c038efccda8..c72ad25fa15fadd 100644
--- a/llvm/test/tools/llvm-rc/windres-target.test
+++ b/llvm/test/tools/llvm-rc/windres-target.test
@@ -17,13 +17,13 @@
 
 ; Check the actual written object types.
 
-; RUN: llvm-windres --no-preprocess -F i686-w64-mingw32 %p/Inputs/tag-stringtable-basic.rc %t.o
+; RUN: llvm-windres --no-preprocess -F i686-w64-mingw32 %p/Inputs/tag-versioninfo.rc %t.o
 ; RUN: llvm-readobj --coff-resources %t.o | FileCheck %s --check-prefixes=CHECK-OBJ,CHECK-OBJ-I686
-; RUN: llvm-windres --no-preprocess -F x86_64-w64-mingw32 %p/Inputs/tag-stringtable-basic.rc %t.o
+; RUN: llvm-windres --no-preprocess -F x86_64-w64-mingw32 %p/Inputs/tag-versioninfo.rc %t.o
 ; RUN: llvm-readobj --coff-resources %t.o | FileCheck %s --check-prefixes=CHECK-OBJ,CHECK-OBJ-X86-64
-; RUN: llvm-windres --no-preprocess -F armv7-w64-mingw32 %p/Inputs/tag-stringtable-basic.rc %t.o
+; RUN: llvm-windres --no-preprocess -F armv7-w64-mingw32 %p/Inputs/tag-versioninfo.rc %t.o
 ; RUN: llvm-readobj --coff-resources %t.o | FileCheck %s --check-prefixes=CHECK-OBJ,CHECK-OBJ-ARMV7
-; RUN: llvm-windres --no-preprocess -F aarch64-w64-mingw32 %p/Inputs/tag-stringtable-basic.rc %t.o
+; RUN: llvm-windres --no-preprocess -F aarch64-w64-mingw32 %p/Inputs/tag-versioninfo.rc %t.o
 ; RUN: llvm-readobj --coff-resources %t.o | FileCheck %s --check-prefixes=CHECK-OBJ,CHECK-OBJ-AARCH64
 
 ; CHECK-OBJ-I686: Format: COFF-i386
diff --git a/llvm/test/tools/llvm-rc/windres-tokenizer.test b/llvm/test/tools/llvm-rc/windres-tokenizer.test
new file mode 100644
index 000000000000000..b7c88fa2a91799a
--- /dev/null
+++ b/llvm/test/tools/llvm-rc/windres-tokenizer.test
@@ -0,0 +1,13 @@
+; RUN: not llvm-windres --no-preprocess --verbose %p/Inputs/tokens-windres.rc %t.res | FileCheck %s
+; llvm-windres fails on this sample because it is an invalid resource file
+; script. We silence the error message and just analyze the output.
+
+; CHECK:  String: L"wide also wide"
+; CHECK-NEXT:  Comma: ,
+; CHECK-NEXT:  String: "narrow also narrow"
+; CHECK-NEXT:  Comma: ,
+; CHECK-NEXT:  String: L"wide more wide"
+
+; RUN: not llvm-windres --no-preprocess --verbose %p/Inputs/tokens-windres-invalid-concat.rc %t.res 2>&1 | FileCheck %s --check-prefix=CONCAT-ERROR
+
+; CONCAT-ERROR: Error parsing file: Cannot concatenate a wide string L"wide" after a narrow one "narrow"
diff --git a/llvm/tools/llvm-rc/ResourceScriptToken.cpp b/llvm/tools/llvm-rc/ResourceScriptToken.cpp
index a8f40abdf8a680c..f036d279a1a9a60 100644
--- a/llvm/tools/llvm-rc/ResourceScriptToken.cpp
+++ b/llvm/tools/llvm-rc/ResourceScriptToken.cpp
@@ -85,7 +85,9 @@ namespace {
 
 class Tokenizer {
 public:
-  Tokenizer(StringRef Input) : Data(Input), DataLength(Input.size()), Pos(0) {}
+  Tokenizer(StringRef Input, StringSaver &Saver, bool IsWindres)
+      : Data(Input), DataLength(Input.size()), Pos(0), Saver(Saver),
+        IsWindres(IsWindres) {}
 
   Expected<std::vector<RCToken>> run();
 
@@ -141,8 +143,14 @@ class Tokenizer {
   // an identifier describing a block start or end.
   void processIdentifier(RCToken &token) const;
 
+  Expected<RCToken> mergeStringTokens(const RCToken &FirstToken,
+                                      const RCToken &SecondToken);
+
   StringRef Data;
   size_t DataLength, Pos;
+
+  StringSaver &Saver;
+  bool IsWindres;
 };
 
 void Tokenizer::skipCurrentLine() {
@@ -188,6 +196,16 @@ Expected<std::vector<RCToken>> Tokenizer::run() {
         return getStringError("Integer invalid or too large: " +
                               Token.value().str());
       }
+    } else if (TokenKind == Kind::String && !Result.empty() && IsWindres) {
+      RCToken &Prev = Result.back();
+      if (Prev.kind() == Kind::String) {
+        Expected<RCToken> MergedToken = mergeStringTokens(Prev, Token);
+        if (!MergedToken)
+          return MergedToken.takeError();
+        // Remove the old token and replace the new one with the merged token.
+        Result.pop_back();
+        Token = *MergedToken;
+      }
     }
 
     Result.push_back(Token);
@@ -356,12 +374,43 @@ void Tokenizer::processIdentifier(RCToken &Token) const {
     Token = RCToken(Kind::BlockEnd, Name);
 }
 
+Expected<RCToken> Tokenizer::mergeStringTokens(const RCToken &FirstToken,
+                                               const RCToken &SecondToken) {
+  assert(FirstToken.kind() == Kind::String &&
+         SecondToken.kind() == Kind::String);
+  StringRef First = FirstToken.value();
+  StringRef Second = SecondToken.value();
+  bool IsFirstLong = First.starts_with_insensitive("L");
+  bool IsSecondLong = Second.starts_with_insensitive("L");
+  //  "aaa"  "bbb" =  "aaabbb"
+  // L"aaa" L"bbb" = L"aaabbb"
+  // L"aaa"  "bbb" = L"aaabbb"
+  //  "aaa" L"bbb" = <error>
+  if (IsSecondLong && !IsFirstLong)
+    return getStringError("Cannot concatenate a wide string " + Twine(Second) +
+                          " after a narrow one " + Twine(First));
+  if (IsFirstLong)
+    First = First.drop_front();
+  if (IsSecondLong)
+    Second = Second.drop_front();
+  bool FirstUnquoted = First.consume_front("\"") && First.consume_back("\"");
+  bool SecondUnquoted = Second.consume_front("\"") && Second.consume_back("\"");
+  assert(FirstUnquoted && SecondUnquoted);
+  (void)FirstUnquoted;
+  (void)SecondUnquoted;
+
+  StringRef MergedString =
+      Saver.save(Twine(IsFirstLong ? "L" : "") + "\"" + First + Second + "\"");
+  return RCToken(Kind::String, MergedString);
+}
+
 } // anonymous namespace
 
 namespace llvm {
 
-Expected<std::vector<RCToken>> tokenizeRC(StringRef Input) {
-  return Tokenizer(Input).run();
+Expected<std::vector<RCToken>> tokenizeRC(StringRef Input, StringSaver &Saver,
+                                          bool IsWindres) {
+  return Tokenizer(Input, Saver, IsWindres).run();
 }
 
 } // namespace llvm
diff --git a/llvm/tools/llvm-rc/ResourceScriptToken.h b/llvm/tools/llvm-rc/ResourceScriptToken.h
index cc8ca48b4907ea8..7144b4f406e835a 100644
--- a/llvm/tools/llvm-rc/ResourceScriptToken.h
+++ b/llvm/tools/llvm-rc/ResourceScriptToken.h
@@ -26,6 +26,7 @@
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/StringSaver.h"
 
 #include <cstdint>
 #include <map>
@@ -74,7 +75,8 @@ class RCToken {
 // Tokens returned by this function hold only references to the parts
 // of the Input. Memory buffer containing Input cannot be freed,
 // modified or reallocated.
-Expected<std::vector<RCToken>> tokenizeRC(StringRef Input);
+Expected<std::vector<RCToken>> tokenizeRC(StringRef Input, StringSaver &Saver,
+                                          bool IsWindres);
 
 } // namespace llvm
 
diff --git a/llvm/tools/llvm-rc/llvm-rc.cpp b/llvm/tools/llvm-rc/llvm-rc.cpp
index b955347f2a8646e..a8c1ac7e46224c9 100644
--- a/llvm/tools/llvm-rc/llvm-rc.cpp
+++ b/llvm/tools/llvm-rc/llvm-rc.cpp
@@ -635,7 +635,11 @@ void doRc(std::string Src, std::string Dest, RcOptions &Opts,
   StringRef Contents = FileContents->getBuffer();
 
   std::string FilteredContents = filterCppOutput(Contents);
-  std::vector<RCToken> Tokens = ExitOnErr(tokenizeRC(FilteredContents));
+
+  BumpPtrAllocator Alloc;
+  StringSaver Saver(Alloc);
+  std::vector<RCToken> Tokens =
+      ExitOnErr(tokenizeRC(FilteredContents, Saver, Opts.IsWindres));
 
   if (Opts.BeVerbose) {
     const Twine TokenNames[] = {



More information about the llvm-commits mailing list