[llvm-rc] Add rudimentary support for codepages

[mstorsjo]

Summary

Only support UTF-8 (since LLVM contains UTF-8 parsing support already, and the code even does that already) and Windows-1252 (where most code points has the same value in unicode). Keep the existing default as only allowing ASCII input.

Using the option type JoinedOrSeparate, since the real rc.exe handles options in this form, even if llvm-rc uses Separate for other similar existing options.

Rename the struct SearchParams to WriterParams since it's now used for more than just include paths.

Add a missing getResourceTypeName method to the BundleResource class, to fix error printing from within STRINGTABLE resources (used in tests).

[amccarth]

tools/llvm-rc/ResourceFileWriter.cpp

156

s/all/most/

208

s/all/most/

tools/llvm-rc/ResourceFileWriter.h

29

Are you concerned about the name clash with the macro defined in the Windows headers? It seems like code that wants to include this header better make sure that it hasn't already included Windows headers (or that it has undefined the macros).

33

That's not _strictly_ true. Windows-1252 maps many characters in the range 0x80-0x9F that do not correspond to the "upper" control characters Unicode has at the corresponding code points. For example, Windows-1252 0x83 corresponds to Unicode U+0192 (LATIN SMALL LETTER F WITH HOOK).

It's probably true enough for practical purposes, but I'd be disappointed if this comment leads readers to not appreciate the differences.

Also, s/8 bit values/8-bit values/.

[mstorsjo]

tools/llvm-rc/ResourceFileWriter.h

29

Oh, indeed, I hadn't thought about that. I'll test how it behaves if that header is included.

33

Oh, thanks for pointing this out!

Is there any other codepage I should rather pick for the same purpose - trivial implementation in converting to unicode without external dependencies? Win-28591 aka latin1?

[amccarth]

tools/llvm-rc/ResourceFileWriter.h

33

Yes, the Latin-1 page would work as you intended and require no conversion. That said, it's probably pretty uncommon in practice.

[mstorsjo]

Added a small mapping table to convert the full range of Win-1252 to unicode, renamed the enum constants to avoid potential clashes with windows headers.

[amccarth]

Looks good to me now, but I'm not an official reviewer.

It would be nice if the CP-1252 test exercised one of the unusual characters that you had to build the map for. Since the sources for the tests are stored as binary files, I'm having a hard time peeking into it from Phabricator, so maybe you did, but it's not obvious.

[mstorsjo]

In D46238#1083353, @amccarth wrote:

Looks good to me now, but I'm not an official reviewer.

It would be nice if the CP-1252 test exercised one of the unusual characters that you had to build the map for. Since the sources for the tests are stored as binary files, I'm having a hard time peeking into it from Phabricator, so maybe you did, but it's not obvious.

Yes, I used one of those chars. The test rc file for cp1252 is the following:

1 "åäö © ƒ \xe5\xe4\366 \251 \x83"
2 L"åäö © ƒ \xe5\xe4\366 \251 \x0192"

Both produce the same UTF16 string; the first half of the strings is a bunch of literal >= 0x80 values, the other half is producing the same using escape codes. The escape codes in the narrow string are interpreted according to the code page, while escape codes in the wide string map directly to their unicode values. The output of this is four times "E5 00 E4 00 F6 00 20 00 A9 00 20 00 92 01".

[amccarth]

LGTM.

test/tools/llvm-rc/codepage.test

+; RUN: llvm-rc /C 65001 /FO %t.utf8.res %p/Inputs/utf8.rc
+; RUN: llvm-readobj %t.utf8.res | FileCheck %s --check-prefix=UTF8
+
+; UTF8:      Resource type (int): 6
+; UTF8-NEXT: Resource name (int): 1
+; UTF8-NEXT: Data version: 0
+; UTF8-NEXT: Memory flags: 0x1030
+; UTF8-NEXT: Language ID: 1033
+; UTF8-NEXT: Version (major): 0
+; UTF8-NEXT: Version (minor): 0
+; UTF8-NEXT: Characteristics: 0
+; UTF8-NEXT: Data size: 68
+; UTF8-NEXT: Data: (
+; UTF8-NEXT:   0000: 00000700 E500E400 F6002000 A9002000  |.......... ... .|
+; UTF8-NEXT:   0010: 61000B00 E500E400 F6002000 A9002000  |a......... ... .|
+; UTF8-NEXT:   0020: E500E400 F6002000 A9000000 00000000  |...... .........|
+; UTF8-NEXT:   0030: 00000000 00000000 00000000 00000000  |................|
+; UTF8-NEXT:   0040: 00000000                             |....|
+; UTF8-NEXT: )
+
+; RUN: not llvm-rc /C 65001 /FO %t.utf8-escape-narrow.res %p/Inputs/utf8-escape-narrow.rc 2>&1 | FileCheck %s --check-prefix UTF8_ESCAPE
+; UTF8_ESCAPE: llvm-rc: Error in STRINGTABLE statement (ID 1):
+; UTF8_ESCAPE-NEXT: Unable to interpret single byte (195) as UTF-8
+
+; RUN: llvm-rc /C 1252 /FO %t.cp1252.res %p/Inputs/cp1252.rc
+; RUN: llvm-readobj %t.cp1252.res | FileCheck %s --check-prefix=CP1252
+
+; CP1252:      Resource type (int): 6
+; CP1252-NEXT: Resource name (int): 1
+; CP1252-NEXT: Data version: 0
+; CP1252-NEXT: Memory flags: 0x1030
+; CP1252-NEXT: Language ID: 1033
+; CP1252-NEXT: Version (major): 0
+; CP1252-NEXT: Version (minor): 0
+; CP1252-NEXT: Characteristics: 0
+; CP1252-NEXT: Data size: 92
+; CP1252-NEXT: Data: (
+; CP1252-NEXT:   0000: 00000F00 E500E400 F6002000 A9002000  |.......... ... .|
+; CP1252-NEXT:   0010: 92012000 E500E400 F6002000 A9002000  |.. ....... ... .|
+; CP1252-NEXT:   0020: 92010F00 E500E400 F6002000 A9002000  |.......... ... .|
+; CP1252-NEXT:   0030: 92012000 E500E400 F6002000 A9002000  |.. ....... ... .|
+; CP1252-NEXT:   0040: 92010000 00000000 00000000 00000000  |................|
+; CP1252-NEXT:   0050: 00000000 00000000 00000000           |............|
+; CP1252-NEXT: )

test/tools/llvm-rc/helpmsg.test

 ; RUN: llvm-rc /h > %t1
 ; RUN: llvm-rc '/?' > %t2
 ; RUN: diff %t1 %t2
 ; RUN: FileCheck -input-file=%t1 %s
 
 ; CHECK:  OVERVIEW: Resource Converter
 ; CHECK-DAG:  USAGE: rc [options] <inputs>
 ; CHECK-DAG:  OPTIONS:
 ; CHECK-NEXT:    /?          Display this help and exit.
+; CHECK-NEXT:    /C <value>  Set the codepage used for input strings.
 ; CHECK-NEXT:    /dry-run    Don't compile the input; only try to parse it.
 ; CHECK-NEXT:    /D <value>  Define a symbol for the C preprocessor.
 ; CHECK-NEXT:    /FO <value> Change the output file location.
 ; CHECK-NEXT:    /H          Display this help and exit.
 ; CHECK-NEXT:    /I <value>  Add an include path.
 ; CHECK-NEXT:    /LN <value> Set the default language name.
 ; CHECK-NEXT:    /L <value>  Set the default language identifier.
 ; CHECK-NEXT:    /N          Null-terminate all strings in the string table.
 ; CHECK-NEXT:    /U <value>  Undefine a symbol for the C preprocessor.
 ; CHECK-NEXT:    /V          Be verbose.
 ; CHECK-NEXT:    /X          Ignore 'include' variable.
 ; CHECK-NEXT:    /Y          Suppress warnings on duplicate resource IDs.

tools/llvm-rc/Opts.td

 def HELP : Flag<[ "/", "-" ], "?">, HelpText<"Display this help and exit.">;
 def H : Flag<[ "/", "-" ], "H">,
         Alias<HELP>,
         HelpText<"Display this help and exit.">;
 
 def DRY_RUN : Flag<[ "/", "-" ], "dry-run">,
               HelpText<"Don't compile the input; only try to parse it.">;
 
+def CODEPAGE : JoinedOrSeparate<[ "/", "-" ], "C">,
+               HelpText<"Set the codepage used for input strings.">;
+
 // Unused switches (at least for now). These will stay unimplemented
 // in an early stage of development and can be ignored. However, we need to
 // parse them in order to preserve the compatibility with the original tool.
 
 def NOLOGO : Flag<[ "/", "-" ], "NOLOGO">;
 def R : Flag<[ "/", "-" ], "R">;
 def SL : Flag<[ "/", "-" ], "SL">;
 
 // (Codepages support.)
-def C : Flag<[ "/", "-" ], "C">;
 def W : Flag<[ "/", "-" ], "W">;
 
 // (Support of MUI and similar.)
 def FM : Separate<[ "/", "-" ], "FM">;
 def Q : Separate<[ "/", "-" ], "Q">;
 def G : Flag<[ "/", "-" ], "G">;
 def GN : Flag<[ "/", "-" ], "GN">;
 def G1 : Flag<[ "/", "-" ], "G1">;
 def G2 : Flag<[ "/", "-" ], "G2">;

tools/llvm-rc/ResourceFileWriter.h

 #include "llvm/Support/Endian.h"
 
 namespace llvm {
 
 class MemoryBuffer;
 
 namespace rc {
 
-struct SearchParams {
+enum CodePage {
+  CpAcp = 0,        // The current used codepage. Since there's no such
+                    // notion in LLVM what codepage it actually means,
+                    // this only allows ASCII.
+  CpWin1252 = 1252, // A codepage where most 8 bit values correspond to
+                    // unicode code points with the same value.
+  CpUtf8 = 65001,   // UTF-8.
+};
+
+struct WriterParams {
   std::vector<std::string> Include;   // Additional folders to search for files.
   std::vector<std::string> NoInclude; // Folders to exclude from file search.
   StringRef InputFilePath;            // The full path of the input file.
+  int CodePage = CpAcp;               // The codepage for interpreting characters.
 };
 
 class ResourceFileWriter : public Visitor {
 public:
-  ResourceFileWriter(const SearchParams &Params,
+  ResourceFileWriter(const WriterParams &Params,
                      std::unique_ptr<raw_fd_ostream> Stream)
       : Params(Params), FS(std::move(Stream)), IconCursorID(1) {
     assert(FS && "Output stream needs to be provided to the serializator");
   }
 
   Error visitNullResource(const RCResource *) override;
   Error visitAcceleratorsResource(const RCResource *) override;
   Error visitCursorResource(const RCResource *) override;
private:
   // User defined resource
   Error writeUserDefinedBody(const RCResource *);
 
   // VersionInfoResource
   Error writeVersionInfoBody(const RCResource *);
   Error writeVersionInfoBlock(const VersionInfoBlock &);
   Error writeVersionInfoValue(const VersionInfoValue &);
 
-  const SearchParams &Params;
+  const WriterParams &Params;
 
   // Output stream handling.
   std::unique_ptr<raw_fd_ostream> FS;
 
   uint64_t tell() const { return FS->tell(); }
 
   uint64_t writeObject(const ArrayRef<uint8_t> Data);
 

tools/llvm-rc/ResourceFileWriter.cpp

if (IsLongString)
     Str = Str.drop_front();
 
   bool StripSuccess = Str.consume_front("\"") && Str.consume_back("\"");
   (void)StripSuccess;
   assert(StripSuccess && "Strings should be enclosed in quotes.");
   return true;
 }
 
+static UTF16 cp1252ToUnicode(unsigned char C) {
+  static const UTF16 Map80[] = {
+      0x20ac, 0x0081, 0x201a, 0x0192, 0x201e, 0x2026, 0x2020, 0x2021,
+      0x02c6, 0x2030, 0x0160, 0x2039, 0x0152, 0x008d, 0x017d, 0x008f,
+      0x0090, 0x2018, 0x2019, 0x201c, 0x201d, 0x2022, 0x2013, 0x2014,
+      0x02dc, 0x2122, 0x0161, 0x203a, 0x0153, 0x009d, 0x017e, 0x0178,
+  };
+  if (C >= 0x80 && C <= 0x9F)
+    return Map80[C - 0x80];
+  return C;
+}
+
 // Describes a way to handle '\0' characters when processing the string.
 // rc.exe tool sometimes behaves in a weird way in postprocessing.
 // If the string to be output is equivalent to a C-string (e.g. in MENU
 // titles), string is (predictably) truncated after first 0-byte.
 // When outputting a string table, the behavior is equivalent to appending
 // '\0\0' at the end of the string, and then stripping the string
 // before the first '\0\0' occurrence.
 // Finally, when handling strings in user-defined resources, 0-bytes
 // aren't stripped, nor do they terminate the string.
 
 enum class NullHandlingMethod {
   UserResource,   // Don't terminate string on '\0'.
   CutAtNull,      // Terminate string on '\0'.
   CutAtDoubleNull // Terminate string on '\0\0'; strip final '\0'.
 };
 
 // Parses an identifier or string and returns a processed version of it:
 //   * String the string boundary quotes.
 //   * Squash "" to a single ".
 //   * Replace the escape sequences with their processed version.
 // For identifiers, this is no-op.
 static Error processString(StringRef Str, NullHandlingMethod NullHandler,
-                           bool &IsLongString, SmallVectorImpl<UTF16> &Result) {
+                           bool &IsLongString, SmallVectorImpl<UTF16> &Result,
+                           int CodePage) {
   bool IsString = stripQuotes(Str, IsLongString);
   SmallVector<UTF16, 128> Chars;
+
+  // Convert the input bytes according to the chosen codepage.
+  if (CodePage == CpUtf8) {
     convertUTF8ToUTF16String(Str, Chars);
+  } else if (CodePage == CpWin1252) {
+    for (char C : Str)
+      Chars.push_back(cp1252ToUnicode((unsigned char)C));
+  } else {
+    // For other, unknown codepages, only allow plain ASCII input.
+    for (char C : Str) {
+      if ((unsigned char)C > 0x7F)
+        return createError("Non-ASCII 8-bit codepoint (" + Twine(C) +
+                           ") can't be interpreted in the current codepage");
+      Chars.push_back((unsigned char)C);
+    }
+  }
 
   if (!IsString) {
     // It's an identifier if it's not a string. Make all characters uppercase.
     for (UTF16 &Ch : Chars) {
       assert(Ch <= 0x7F && "We didn't allow identifiers to be non-ASCII");
       Ch = toupper(Ch);
     }
     Result.swap(Chars);
     return Error::success();
   }
   Result.reserve(Chars.size());
   size_t Pos = 0;
 
   auto AddRes = [&Result, NullHandler, IsLongString](UTF16 Char) -> Error {
     if (!IsLongString) {
       if (NullHandler == NullHandlingMethod::UserResource) {
         // Narrow strings in user-defined resources are *not* output in
         // UTF-16 format.
         if (Char > 0xFF)
           return createError("Non-8-bit codepoint (" + Twine(Char) +
                              ") can't occur in a user-defined narrow string");
+      }
+    }
 
+    Result.push_back(Char);
+    return Error::success();
+  };
+  auto AddEscapedChar = [AddRes, IsLongString, CodePage](UTF16 Char) -> Error {
+    if (!IsLongString) {
+      // Escaped chars in narrow strings have to be interpreted according to
+      // the chosen code page.
+      if (Char > 0xFF)
+        return createError("Non-8-bit escaped char (" + Twine(Char) +
+                           ") can't occur in narrow string");
+      if (CodePage == CpUtf8) {
+        if (Char >= 0x80)
+          return createError("Unable to interpret single byte (" + Twine(Char) +
+                             ") as UTF-8");
+      } else if (CodePage == CpWin1252) {
+        Char = cp1252ToUnicode(Char);
       } else {
-        // In case of narrow non-user strings, Windows RC converts
-        // [0x80, 0xFF] chars according to the current codepage.
-        // There is no 'codepage' concept settled in every supported platform,
-        // so we should reject such inputs.
-        if (Char > 0x7F && Char <= 0xFF)
+        // Unknown/unsupported codepage, only allow ASCII input.
+        if (Char > 0x7F)
           return createError("Non-ASCII 8-bit codepoint (" + Twine(Char) +
                              ") can't "
                              "occur in a non-Unicode string");
       }
     }
 
-    Result.push_back(Char);
-    return Error::success();
+    return AddRes(Char);
   };
 
   while (Pos < Chars.size()) {
     UTF16 CurChar = Chars[Pos];
     ++Pos;
 
     // Strip double "".
     if (CurChar == '"') {
if (CurChar == '\\') {
           if (isdigit(Digit))
             ReadInt |= Digit - '0';
           else
             ReadInt |= Digit - 'a' + 10;
 
           --RemainingChars;
         }
 
-        RETURN_IF_ERROR(AddRes(ReadInt));
+        RETURN_IF_ERROR(AddEscapedChar(ReadInt));
         continue;
       }
 
       if (TypeChar >= '0' && TypeChar < '8') {
         // Read an octal number. Note that we've already read the first digit.
         UTF16 ReadInt = TypeChar - '0';
         size_t RemainingChars = IsLongString ? 6 : 2;
 
         while (RemainingChars && Pos < Chars.size() && Chars[Pos] >= '0' &&
                Chars[Pos] < '8') {
           ReadInt <<= 3;
           ReadInt |= Chars[Pos] - '0';
           --RemainingChars;
           ++Pos;
         }
 
-        RETURN_IF_ERROR(AddRes(ReadInt));
+        RETURN_IF_ERROR(AddEscapedChar(ReadInt));
 
         continue;
       }
 
       switch (TypeChar) {
       case 'A':
       case 'a':
         // Windows '\a' translates into '\b' (Backspace).
uint64_t ResourceFileWriter::writeObject(const ArrayRef<uint8_t> Data) {
   FS->write((const char *)Data.begin(), Data.size());
   return Result;
 }
 
 Error ResourceFileWriter::writeCString(StringRef Str, bool WriteTerminator) {
   SmallVector<UTF16, 128> ProcessedString;
   bool IsLongString;
   RETURN_IF_ERROR(processString(Str, NullHandlingMethod::CutAtNull,
-                                IsLongString, ProcessedString));
+                                IsLongString, ProcessedString,
+                                Params.CodePage));
   for (auto Ch : ProcessedString)
     writeInt<uint16_t>(Ch);
   if (WriteTerminator)
     writeInt<uint16_t>(0);
   return Error::success();
 }
 
 Error ResourceFileWriter::writeIdentifier(const IntOrString &Ident) {
public:
 
   BundleResource(const BundleType &StrBundle) : Bundle(StrBundle) {}
   IntOrString getResourceType() const override { return 6; }
 
   ResourceKind getKind() const override { return RkStringTableBundle; }
   static bool classof(const RCResource *Res) {
     return Res->getKind() == RkStringTableBundle;
   }
+  Twine getResourceTypeName() const override { return "STRINGTABLE"; }
 };
 
 Error ResourceFileWriter::visitStringTableBundle(const RCResource *Res) {
   return writeResource(Res, &ResourceFileWriter::writeStringTableBundleBody);
 }
 
 Error ResourceFileWriter::insertStringIntoBundle(
     StringTableInfo::Bundle &Bundle, uint16_t StringID, StringRef String) {
Error ResourceFileWriter::writeStringTableBundleBody(const RCResource *Base) {
   for (size_t ID = 0; ID < Res->Bundle.Data.size(); ++ID) {
     // The string format is a tiny bit different here. We
     // first output the size of the string, and then the string itself
     // (which is not null-terminated).
     bool IsLongString;
     SmallVector<UTF16, 128> Data;
     RETURN_IF_ERROR(processString(Res->Bundle.Data[ID].getValueOr(StringRef()),
                                   NullHandlingMethod::CutAtDoubleNull,
-                                  IsLongString, Data));
+                                  IsLongString, Data, Params.CodePage));
     if (AppendNull && Res->Bundle.Data[ID])
       Data.push_back('\0');
     RETURN_IF_ERROR(
         checkNumberFits<uint16_t>(Data.size(), "STRINGTABLE string size"));
     writeInt<uint16_t>(Data.size());
     for (auto Char : Data)
       writeInt(Char);
   }
if (Elem.isInt()) {
       RETURN_IF_ERROR(
           checkRCInt(Elem.getInt(), "Number in user-defined resource"));
       writeRCInt(Elem.getInt());
       continue;
     }
 
     SmallVector<UTF16, 128> ProcessedString;
     bool IsLongString;
-    RETURN_IF_ERROR(processString(Elem.getString(),
-                                  NullHandlingMethod::UserResource,
-                                  IsLongString, ProcessedString));
+    RETURN_IF_ERROR(
+        processString(Elem.getString(), NullHandlingMethod::UserResource,
+                      IsLongString, ProcessedString, Params.CodePage));
 
     for (auto Ch : ProcessedString) {
       if (IsLongString) {
         writeInt(Ch);
         continue;
       }
 
       RETURN_IF_ERROR(checkNumberFits<uint8_t>(

tools/llvm-rc/llvm-rc.cpp

for (const RCToken &Token : Tokens) {
              << Token.value();
       if (Token.kind() == RCToken::Kind::Int)
         outs() << "; int value = " << Token.intValue();
 
       outs() << "\n";
     }
   }
 
-  SearchParams Params;
+  WriterParams Params;
   SmallString<128> InputFile(InArgsInfo[0]);
   llvm::sys::fs::make_absolute(InputFile);
   Params.InputFilePath = InputFile;
   Params.Include = InputArgs.getAllArgValues(OPT_INCLUDE);
   Params.NoInclude = InputArgs.getAllArgValues(OPT_NOINCLUDE);
 
+  if (InputArgs.hasArg(OPT_CODEPAGE)) {
+    if (InputArgs.getLastArgValue(OPT_CODEPAGE)
+            .getAsInteger(10, Params.CodePage))
+      fatalError("Invalid code page: " +
+                 InputArgs.getLastArgValue(OPT_CODEPAGE));
+    switch (Params.CodePage) {
+    case CpAcp:
+    case CpWin1252:
+    case CpUtf8:
+      break;
+    default:
+      fatalError(
+          "Unsupported code page, only 0, 1252 and 65001 are supported!");
+    }
+  }
+
   std::unique_ptr<ResourceFileWriter> Visitor;
   bool IsDryRun = InputArgs.hasArg(OPT_DRY_RUN);
 
   if (!IsDryRun) {
     auto OutArgsInfo = InputArgs.getAllArgValues(OPT_FILEOUT);
     if (OutArgsInfo.empty()) {
       SmallString<128> OutputFile = InputFile;
       llvm::sys::path::replace_extension(OutputFile, "res");