[lld] Fixed CodeView GuidAdapter::format to handle GUID bytes in the right order.

[aorlov]

Summary

This fixes https://bugs.llvm.org/show_bug.cgi?id=41712 bug.

[aganea]

llvm/lib/DebugInfo/CodeView/Formatters.cpp

28

I know the patch you're proposing is more optimal in terms of runtime performance, but it adds an extra layer of complexity for a reader who was to figure out why memory is read in that way. Since GUIDs are not that used in binary files, performance is maybe less of a concern. We could maybe rewrite the code to be more self-documenting, something along those lines?

void GuidAdapter::format(raw_ostream &Stream, StringRef Style) {
  assert(Item.size() == 16 && "Expected 16-byte GUID");
  struct _GUID {
    support::ulittle32_t Data1;
    support::ulittle16_t Data2;
    support::ulittle16_t Data3;
    support::ubig64_t Data4;
  };
  const _GUID *G = reinterpret_cast<const _GUID *>(Item.data());
  Stream << "{"
         << format_hex_no_prefix(G->Data1, sizeof(G->Data1), /*Upper=*/true)
         << '-'
         << format_hex_no_prefix(G->Data2, sizeof(G->Data2), /*Upper=*/true)
         << '-'
         << format_hex_no_prefix(G->Data3, sizeof(G->Data3), /*Upper=*/true)
         << '-'
         << format_hex_no_prefix(G->Data4, sizeof(G->Data4), /*Upper=*/true)
         << "}";
}

[rnk]

llvm/lib/DebugInfo/CodeView/Formatters.cpp

28

+1

[aganea]

Thanks for integrating the comments!

llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp

176

In order to avoid any magical offsets into the string, can you possibly keep a loop, but add a counter and successively write into Data1, Data2, Data3, etc.? Something like:

uint8_t *OutBuffer = S.Guid;
unsigned Index = 0;
uint64_t D41{}, D42{};
bool GotHex;
for (auto Iter = Scalar.begin(); Iter != Scalar.end(); ++Iter) {
  auto Next = std::find_if(Iter, Scalar.end(), std::isxdigit);
  if (Next == Scalar.end())
    break;
  StringRef Group(Iter, Next - Iter);
  if (Index == 0)
    GotHex = to_integer(Group, G.Data1, 16);
  else if (Index == 1)
    GotHex &= to_integer(Group, G.Data2, 16);
  else if (Index == 2)
    GotHex &= to_integer(Group, G.Data3, 16);
  else if (Index == 3)
    GotHex &= to_integer(Group, D41, 16);
  else if (Index == 4)
    GotHex &= to_integer(Group, D42, 16);
  ++Index;
}
G.Data4 = (D41 << 48) | D42;

[aorlov]

llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp

176

No magical offsets, no loops. But I kept the maximal validation.

[aganea]

LGTM. Thank you!

llvm/lib/DebugInfo/CodeView/Formatters.cpp

 GuidAdapter::GuidAdapter(StringRef Guid)
     : FormatAdapter(makeArrayRef(Guid.bytes_begin(), Guid.bytes_end())) {}
 
 GuidAdapter::GuidAdapter(ArrayRef<uint8_t> Guid)
     : FormatAdapter(std::move(Guid)) {}
 
 void GuidAdapter::format(raw_ostream &Stream, StringRef Style) {
   static const char *Lookup = "0123456789ABCDEF";
+  static const uint8_t Order[16] = {3, 2, 1,  0,  5,  4,  7,  6,
+                                    8, 9, 10, 11, 12, 13, 14, 15};
 
   assert(Item.size() == 16 && "Expected 16-byte GUID");
   Stream << "{";
   for (int i = 0; i < 16;) {
-    uint8_t Byte = Item[i];
+    uint8_t Byte = Item[Order[i]];
     uint8_t HighNibble = (Byte >> 4) & 0xF;
     uint8_t LowNibble = Byte & 0xF;
     Stream << Lookup[HighNibble] << Lookup[LowNibble];
     ++i;
     if (i >= 4 && i <= 10 && i % 2 == 0)
       Stream << "-";
   }
   Stream << "}";
 }
 
 raw_ostream &llvm::codeview::operator<<(raw_ostream &OS, const GUID &Guid) {
   codeview::detail::GuidAdapter A(Guid.Guid);
   A.format(OS, "");
   return OS;
 }

llvm/lib/ObjectYAML/CodeViewYAMLTypes.cpp

 } // end namespace CodeViewYAML
 } // end namespace llvm
 
 void ScalarTraits<GUID>::output(const GUID &G, void *, llvm::raw_ostream &OS) {
   OS << G;
 }
 
 StringRef ScalarTraits<GUID>::input(StringRef Scalar, void *Ctx, GUID &S) {
+  static const uint8_t Order[16] = {3, 2, 1,  0,  5,  4,  7,  6,
+                                    8, 9, 10, 11, 12, 13, 14, 15};
   if (Scalar.size() != 38)
     return "GUID strings are 38 characters long";
   if (Scalar[0] != '{' || Scalar[37] != '}')
     return "GUID is not enclosed in {}";
   if (Scalar[9] != '-' || Scalar[14] != '-' || Scalar[19] != '-' ||
       Scalar[24] != '-')
     return "GUID sections are not properly delineated with dashes";
 
-  uint8_t *OutBuffer = S.Guid;
+  uint8_t I = 0;
   for (auto Iter = Scalar.begin(); Iter != Scalar.end();) {
     if (*Iter == '-' || *Iter == '{' || *Iter == '}') {
       ++Iter;
       continue;
     }
     uint8_t Value = (llvm::hexDigitValue(*Iter++) << 4);
     Value |= llvm::hexDigitValue(*Iter++);
-    *OutBuffer++ = Value;
+    S.Guid[Order[I++]] = Value;
   }
 
   return "";
 }
 
 void ScalarTraits<TypeIndex>::output(const TypeIndex &S, void *,
                                      raw_ostream &OS) {
   OS << S.getIndex();
 }
 
 StringRef ScalarTraits<TypeIndex>::input(StringRef Scalar, void *Ctx,
                                          TypeIndex &S) {
   uint32_t I;
   StringRef Result = ScalarTraits<uint32_t>::input(Scalar, Ctx, I);
   S.setIndex(I);
   return Result;
 }

llvm/test/DebugInfo/PDB/DIA/pdbdump-flags.test

 ; RUN: llvm-pdbutil pretty %p/../Inputs/empty.pdb | FileCheck %s -check-prefix=NO_ARGS
 ; RUN: llvm-pdbutil pretty -types %p/../Inputs/empty.pdb | FileCheck %s -check-prefix=TYPES
 ; RUN: llvm-pdbutil pretty -compilands %p/../Inputs/empty.pdb | FileCheck %s -check-prefix=COMPILANDS
 ; RUN: llvm-pdbutil pretty -types -compilands %p/../Inputs/empty.pdb | FileCheck %s -check-prefix=MULTIPLE
 
 ; Check that neither symbols nor compilands are dumped when neither argument specified.
 ; NO_ARGS: empty.pdb
-; NO_ARGS: Guid: {0B355641-86A0-A249-896F-9988FAE52FF0}
+; NO_ARGS: Guid: {4156350B-A086-49A2-896F-9988FAE52FF0}
 ; NO_ARGS: Attributes: HasPrivateSymbols
 ; NO_ARGS-NOT: ---TYPES---
 ; NO_ARGS-NOT: ---COMPILANDS---
 ; NO_ARGS-NOT: ---GLOBALS---
 ; NO_ARGS-NOT: ---SYMBOLS---
 
 ; Check that only types are dumped when only -types is specified.
 ; TYPES: empty.pdb
-; TYPES: Guid: {0B355641-86A0-A249-896F-9988FAE52FF0}
+; TYPES: Guid: {4156350B-A086-49A2-896F-9988FAE52FF0}
 ; TYPES: Attributes: HasPrivateSymbols
 ; TYPES: ---TYPES---
 ; TYPES-NOT: ---COMPILANDS---
 ; TYPES-NOT: ---GLOBALS---
 ; TYPES-NOT: ---SYMBOLS---
 
 ; Check that only compilands are dumped when only -compilands is specified.
 ; COMPILANDS: empty.pdb
-; COMPILANDS: Guid: {0B355641-86A0-A249-896F-9988FAE52FF0}
+; COMPILANDS: Guid: {4156350B-A086-49A2-896F-9988FAE52FF0}
 ; COMPILANDS: Attributes: HasPrivateSymbols
 ; COMPILANDS: ---COMPILANDS---
 ; COMPILANDS-NOT: ---TYPES---
 ; COMPILANDS-NOT: ---GLOBALS---
 ; COMPILANDS-NOT: ---SYMBOLS---
 
 ; Check that types and compilands are dumped when both arguments are specified.
 ; MULTIPLE: empty.pdb
-; MULTIPLE: Guid: {0B355641-86A0-A249-896F-9988FAE52FF0}
+; MULTIPLE: Guid: {4156350B-A086-49A2-896F-9988FAE52FF0}
 ; MULTIPLE: Attributes: HasPrivateSymbols
 ; MULTIPLE: ---COMPILANDS---
 ; MULTIPLE: ---TYPES---
 ; MULTIPLE-NOT: ---GLOBALS---
 ; MULTIPLE-NOT: ---SYMBOLS---

llvm/test/DebugInfo/PDB/Native/pdb-native-summary.test

 ; Test that the native PDB reader gets the PDB summary correct.
 ; RUN: llvm-pdbutil pretty -native -color-output=false %p/../Inputs/empty.pdb \
 ; RUN:   | FileCheck -check-prefix=EMPTY %s
 
 ; Reference output was generated with the DIA reader to ensure that the
 ; `-native` option produces identical output.
 
 ; EMPTY:  Size: 102400 bytes
-; EMPTY:  Guid: {0B355641-86A0-A249-896F-9988FAE52FF0}
+; EMPTY:  Guid: {4156350B-A086-49A2-896F-9988FAE52FF0}
 ; EMPTY:  Age: 1
 ; EMPTY:  Attributes: HasPrivateSymbols

llvm/test/DebugInfo/PDB/pdbdump-headers.test

 ; RUN: llvm-pdbutil dump -all -dont-resolve-forward-refs %p/Inputs/empty.pdb \
 ; RUN:   | FileCheck -check-prefix=ALL %s
 ; RUN: llvm-pdbutil dump -summary -modules -files \
 ; RUN:              %p/Inputs/big-read.pdb | FileCheck -check-prefix=BIG %s
 ; RUN: not llvm-pdbutil dump -summary %p/Inputs/bad-block-size.pdb 2>&1 \
 ; RUN:   | FileCheck -check-prefix=BAD-BLOCK-SIZE %s
 
 ALL:                                Summary
 ALL-NEXT: ============================================================
 ALL-NEXT:   Block Size: 4096
 ALL-NEXT:   Number of blocks: 25
 ALL-NEXT:   Number of streams: 17
 ALL-NEXT:   Signature: 1424295906
 ALL-NEXT:   Age: 1
-ALL-NEXT:   GUID: {0B355641-86A0-A249-896F-9988FAE52FF0}
+ALL-NEXT:   GUID: {4156350B-A086-49A2-896F-9988FAE52FF0}
 ALL-NEXT:   Features: 0x1
 ALL-NEXT:   Has Debug Info: true
 ALL-NEXT:   Has Types: true
 ALL-NEXT:   Has IDs: true
 ALL-NEXT:   Has Globals: true
 ALL-NEXT:   Has Publics: true
 ALL-NEXT:   Is incrementally linked: true
 ALL-NEXT:   Has conflicting types: false

 
 BIG:                            Summary
 BIG-NEXT: ============================================================
 BIG-NEXT:   Block Size: 4096
 BIG-NEXT:   Number of blocks: 99
 BIG-NEXT:   Number of streams: 64
 BIG-NEXT:   Signature: 1461714535
 BIG-NEXT:   Age: 1
-BIG-NEXT:   GUID: {880ECC89-DF81-0B4F-839C-58CBD052E937}
+BIG-NEXT:   GUID: {89CC0E88-81DF-4F0B-839C-58CBD052E937}
 BIG-NEXT:   Features: 0x1
 BIG-NEXT:   Has Debug Info: true
 BIG-NEXT:   Has Types: true
 BIG-NEXT:   Has IDs: true
 BIG-NEXT:   Has Globals: true
 BIG-NEXT:   Has Publics: true
 BIG-NEXT:   Is incrementally linked: true
 BIG-NEXT:   Has conflicting types: false

llvm/test/DebugInfo/PDB/pdbdump-readwrite.test

 RUN: llvm-pdbutil pdb2yaml -modules -module-files -dbi-stream \
 RUN:   -pdb-stream -string-table -tpi-stream -stream-directory \
 RUN:   -stream-metadata %p/Inputs/empty.pdb > %t.1
 RUN: llvm-pdbutil yaml2pdb -pdb=%t.2 %t.1
 
 RUN: llvm-pdbutil dump -summary -string-table -types %p/Inputs/empty.pdb | FileCheck %s
 RUN: llvm-pdbutil dump -summary -string-table -types %t.2 | FileCheck %s
 
 
 CHECK:                                Summary
 CHECK-NEXT: ============================================================
 CHECK-NEXT:  Block Size: 4096
 CHECK-NEXT:  Number of blocks:
 CHECK-NEXT:  Number of streams:
 CHECK-NEXT:  Signature: 1424295906
 CHECK-NEXT:  Age: 1
-CHECK-NEXT:  GUID: {0B355641-86A0-A249-896F-9988FAE52FF0}
+CHECK-NEXT:  GUID: {4156350B-A086-49A2-896F-9988FAE52FF0}
 CHECK-NEXT:  Features: 0x1
 CHECK-NEXT:  Has Debug Info: true
 CHECK-NEXT:  Has Types: true
 CHECK-NEXT:  Has IDs: true
 CHECK-NEXT:  Has Globals:
 CHECK-NEXT:  Has Publics:
 CHECK-NEXT:  Is incrementally linked: true
 CHECK-NEXT:  Has conflicting types: false

llvm/test/DebugInfo/PDB/pdbdump-yaml.test

 ; YAML-NEXT:   - Stream: [ 21 ]
 ; YAML-NEXT:   - Stream: [ 22 ]
 ; YAML-NEXT: StringTable:
 ; YAML-NEXT:   - 'd:\src\llvm\test\debuginfo\pdb\inputs\predefined c++ attributes (compiler internal)'
 ; YAML-NEXT:   - 'd:\src\llvm\test\debuginfo\pdb\inputs\empty.cpp'
 ; YAML-NEXT:   - '$T0 $ebp = $eip $T0 4 + ^ = $ebp $T0 ^ = $esp $T0 8 + = '
 ; YAML-NEXT: PdbStream:
 ; YAML-NEXT:   Age:             1
-; YAML-NEXT:   Guid:            '{0B355641-86A0-A249-896F-9988FAE52FF0}'
+; YAML-NEXT:   Guid:            '{4156350B-A086-49A2-896F-9988FAE52FF0}'
 ; YAML-NEXT:   Signature:       1424295906
 ; YAML-NEXT:   Features:        [ VC110 ]
 ; YAML-NEXT:   Version:         VC70
 ; YAML-NEXT: ...
 
 ; NO-HEADERS: ---
 ; NO-HEADERS-NOT: MSF:
 ; NO-HEADERS-NOT:   SuperBlock:
 ; NO-HEADERS: ...
 No newline at end of file

llvm/test/ObjectYAML/CodeView/sections.yaml

 # RUN: yaml2obj %s -o %t.obj
 # RUN: obj2yaml %t.obj | FileCheck --check-prefix=CHECK %s
 # RUN: llvm-objdump --section-headers %t.obj | FileCheck --check-prefix=HEADERS %s
 
 --- !COFF
 header:
   Machine:         IMAGE_FILE_MACHINE_AMD64
   Characteristics: [  ]
 sections:
   - Name:            '.debug$T'
     Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
     Alignment:       1
     Types:
       - Kind:            LF_TYPESERVER2
         TypeServer2:
-          Guid:            '{01DF191B-22BF-6B42-96CE-5258B8329FE5}'
+          Guid:            '{1B19DF01-BF22-426B-96CE-5258B8329FE5}'
           Age:             24
           Name:            'C:\src\llvm-project\build\vc140.pdb'
   - Name:            '.debug$H'
     Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
     Alignment:       4
     GlobalHashes:
       Version:          0
       HashAlgorithm:    1

 # CHECK:   Characteristics: [  ]
 # CHECK: sections:
 # CHECK:   - Name:            '.debug$T'
 # CHECK:     Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
 # CHECK:     Alignment:       1
 # CHECK:     Types:
 # CHECK:       - Kind:            LF_TYPESERVER2
 # CHECK:         TypeServer2:
-# CHECK:           Guid:            '{01DF191B-22BF-6B42-96CE-5258B8329FE5}'
+# CHECK:           Guid:            '{1B19DF01-BF22-426B-96CE-5258B8329FE5}'
 # CHECK:           Age:             24
 # CHECK:           Name:            'C:\src\llvm-project\build\vc140.pdb'
 # CHECK:   - Name:            '.debug$H'
 # CHECK:     Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
 # CHECK:     Alignment:       4
 # CHECK:     GlobalHashes:
 # CHECK:       Version:          0
 # CHECK:       HashAlgorithm:    1

llvm/test/tools/llvm-pdbutil/explain-pdb-stream.test

 CHECK-NEXT:     The current value is 1.
 
 CHECK:      Block:Offset = 11:000C.
 CHECK-NEXT: Address is in block 17 (allocated).
 CHECK-NEXT:   Address is at offset 12/202 of Stream 1 (PDB Stream).
 CHECK-NEXT:   Within the PDB stream:
 CHECK-NEXT:     address is at offset 12/28 of the PDB Stream Header.
 CHECK-NEXT:     which contains the guid of the PDB.
-CHECK-NEXT:     The current value is {826BE46E-02ED-7043-9C27-20CCC07E92A7}.
+CHECK-NEXT:     The current value is {6EE46B82-ED02-4370-9C27-20CCC07E92A7}.
 
 CHECK:      Block:Offset = 11:001C.
 CHECK-NEXT: Address is in block 17 (allocated).
 CHECK-NEXT:   Address is at offset 28/202 of Stream 1 (PDB Stream).
 CHECK-NEXT:   Within the PDB stream:
 CHECK-NEXT:     address is at offset 0/166 of the Named Stream Map.

llvm/test/tools/llvm-pdbutil/stripped.test

 ; RUN: llvm-pdbutil dump -all %p/Inputs/Stripped.pdb > %t
 ; RUN: FileCheck -input-file=%t %s
 
 ; CHECK: Summary
 ; CHECK-NEXT: ============================================================
 ; CHECK-NEXT:  Block Size: 4096
 ; CHECK-NEXT:  Number of blocks: 17
 ; CHECK-NEXT:  Number of streams: 12
 ; CHECK-NEXT:  Signature: 1541179274
 ; CHECK-NEXT:  Age: 2
-; CHECK-NEXT:  GUID: {FF4F9B62-D99A-4647-97A7-22C702B1E053}
+; CHECK-NEXT:  GUID: {629B4FFF-9AD9-4746-97A7-22C702B1E053}
 ; CHECK-NEXT:  Features: 0x1
 ; CHECK-NEXT:  Has Debug Info: true
 ; CHECK-NEXT:  Has Types: true
 ; CHECK-NEXT:  Has IDs: true
 ; CHECK-NEXT:  Has Globals: true
 ; CHECK-NEXT:  Has Publics: true
 ; CHECK-NEXT:  Is incrementally linked: false
 ; CHECK-NEXT:  Has conflicting types: false