[ms] Fix mangling of string literals used to initialize arrays larger or smaller than the literal

[hans]

Summary

A Chromium developer reported a bug which turned out to be a mangling collision between these two literals:

char s[] = "foo";
char t[32] = "foo";

They may look the same, but for the initialization of t we will (under some circumstances) use a literal that's extended with zeros, and both the length and those zeros should be accounted for by the mangling.

This actually makes the mangling code simpler: where it previously had special logic for null terminators, which are not part of the StringLiteral, that is now covered by the general algorithm.

[hans]

I couldn't get MSVC to create a symbol with padding at the end; they seem to always insert the padding with a separate loop during initialization, but this does show the truncated case: https://godbolt.org/g/B8ktA3

[thakis]

Nice!

lib/AST/MicrosoftMangle.cpp

3198

nit: Also do unsigned StingByteLength = StringLength * SL->getCharByteWidth() here and use it the 3 times you have that computation below

3211

You're capturing StringLength here, but then you don't use it in the lambda body.

3220

likewise

test/CodeGen/mangle-ms-string-literals.c

8

Nice, we used to mismangle this. MSVC matches the string here: https://godbolt.org/g/dbm6jT (old clang: https://godbolt.org/g/Yy7iCK)

[hans]

lib/AST/MicrosoftMangle.cpp

3198

Good idea, thanks!

3211

Oops, a leftover from an older version of the patch. Removed.

lib/AST/MicrosoftMangle.cpp

void MicrosoftMangleContextImpl::mangleStringLiteral(const StringLiteral *SL,
                                                      raw_ostream &Out) {
   // <char-type> ::= 0   # char, char16_t, char32_t
   //                     # (little endian char data in mangling)
   //             ::= 1   # wchar_t (big endian char data in mangling)
   //
   // <literal-length> ::= <non-negative integer>  # the length of the literal
   //
   // <encoded-crc>    ::= <hex digit>+ @          # crc of the literal including
-  //                                              # null-terminator
+  //                                              # trailing null bytes
   //
   // <encoded-string> ::= <simple character>           # uninteresting character
   //                  ::= '?$' <hex digit> <hex digit> # these two nibbles
   //                                                   # encode the byte for the
   //                                                   # character
   //                  ::= '?' [a-z]                    # \xe1 - \xfa
   //                  ::= '?' [A-Z]                    # \xc1 - \xda
   //                  ::= '?' [0-9]                    # [,/\:. \n\t'-]
   //
   // <literal> ::= '??_C@_' <char-type> <literal-length> <encoded-crc>
   //               <encoded-string> '@'
   MicrosoftCXXNameMangler Mangler(*this, Out);
   Mangler.getStream() << "??_C@_";
 
+  // The actual string length might be different from that of the string literal
+  // in cases like:
+  // char foo[3] = "foobar";
+  // char bar[42] = "foobar";
+  // Where it is truncated or zero-padded to fit the array. This is the length
+  // used for mangling, and any trailing null-bytes also need to be mangled.
+  unsigned StringLength = getASTContext()
+                              .getAsConstantArrayType(SL->getType())
+                              ->getSize()
+                              .getZExtValue();
+  unsigned StringByteLength = StringLength * SL->getCharByteWidth();
+
   // <char-type>: The "kind" of string literal is encoded into the mangled name.
   if (SL->isWide())
     Mangler.getStream() << '1';
   else
     Mangler.getStream() << '0';
 
   // <literal-length>: The next part of the mangled name consists of the length
-  // of the string.
-  // The StringLiteral does not consider the NUL terminator byte(s) but the
-  // mangling does.
-  // N.B. The length is in terms of bytes, not characters.
-  Mangler.mangleNumber(SL->getByteLength() + SL->getCharByteWidth());
+  // of the string in bytes.
+  Mangler.mangleNumber(StringByteLength);
 
   auto GetLittleEndianByte = [&SL](unsigned Index) {
     unsigned CharByteWidth = SL->getCharByteWidth();
+    if (Index / CharByteWidth >= SL->getLength())
+      return static_cast<char>(0);
     uint32_t CodeUnit = SL->getCodeUnit(Index / CharByteWidth);
     unsigned OffsetInCodeUnit = Index % CharByteWidth;
     return static_cast<char>((CodeUnit >> (8 * OffsetInCodeUnit)) & 0xff);
   };
 
   auto GetBigEndianByte = [&SL](unsigned Index) {
     unsigned CharByteWidth = SL->getCharByteWidth();
+    if (Index / CharByteWidth >= SL->getLength())
+      return static_cast<char>(0);
     uint32_t CodeUnit = SL->getCodeUnit(Index / CharByteWidth);
     unsigned OffsetInCodeUnit = (CharByteWidth - 1) - (Index % CharByteWidth);
     return static_cast<char>((CodeUnit >> (8 * OffsetInCodeUnit)) & 0xff);
   };
 
   // CRC all the bytes of the StringLiteral.
   llvm::JamCRC JC;
-  for (unsigned I = 0, E = SL->getByteLength(); I != E; ++I)
+  for (unsigned I = 0, E = StringByteLength; I != E; ++I)
     JC.update(GetLittleEndianByte(I));
 
-  // The NUL terminator byte(s) were not present earlier,
-  // we need to manually process those bytes into the CRC.
-  for (unsigned NullTerminator = 0; NullTerminator < SL->getCharByteWidth();
-       ++NullTerminator)
-    JC.update('\x00');
-
   // <encoded-crc>: The CRC is encoded utilizing the standard number mangling
   // scheme.
   Mangler.mangleNumber(JC.getCRC());
 
   // <encoded-string>: The mangled name also contains the first 32 bytes
   // (including null-terminator bytes) of the encoded StringLiteral.
   // Each character is encoded by splitting them into bytes and then encoding
   // the constituent bytes.
if (isIdentifierBody(Byte, /*AllowDollar=*/true)) {
         Mangler.getStream() << static_cast<char>('A' + ((Byte >> 4) & 0xf));
         Mangler.getStream() << static_cast<char>('A' + (Byte & 0xf));
       }
     }
   };
 
   // Enforce our 32 bytes max, except wchar_t which gets 32 chars instead.
   unsigned MaxBytesToMangle = SL->isWide() ? 64U : 32U;
-  unsigned NumBytesToMangle = std::min(MaxBytesToMangle, SL->getByteLength());
-  for (unsigned I = 0; I != NumBytesToMangle; ++I)
+  unsigned NumBytesToMangle = std::min(MaxBytesToMangle, StringByteLength);
+  for (unsigned I = 0; I != NumBytesToMangle; ++I) {
     if (SL->isWide())
       MangleByte(GetBigEndianByte(I));
     else
       MangleByte(GetLittleEndianByte(I));
-
-  // Encode the NUL terminator if there is room.
-  if (NumBytesToMangle < MaxBytesToMangle)
-    for (unsigned NullTerminator = 0; NullTerminator < SL->getCharByteWidth();
-         ++NullTerminator)
-      MangleByte(0);
+  }
 
   Mangler.getStream() << '@';
 }
 
 MicrosoftMangleContext *
 MicrosoftMangleContext::create(ASTContext &Context, DiagnosticsEngine &Diags) {
   return new MicrosoftMangleContextImpl(Context, Diags);
 }

test/CodeGen/mangle-ms-string-literals.c

+// RUN: %clang_cc1 -x c -emit-llvm %s -o - -triple=i386-pc-win32 | FileCheck %s
+// RUN: %clang_cc1 -x c -emit-llvm %s -o - -triple=x86_64-pc-win32 | FileCheck %s
+
+void crbug857442(int x) {
+  // Make sure to handle truncated or padded literals. The truncation is only valid in C.
+  struct {int x; char s[2]; } truncatedAscii = {x, "hello"};
+  // CHECK: "??_C@_01CONKJJHI@he@"
+  struct {int x; char s[16]; } paddedAscii = {x, "hello"};
+  // CHECK: "??_C@_0BA@EAAINDNC@hello?$AA?$AA?$AA?$AA?$AA?$AA?$AA?$AA?$AA?$AA?$AA@"
+}