[PDB] Emit S_UDT records in LLD

[zturner]

Summary

This patch emits S_UDT records from LLD. These represent typedefs. The compiler emits these into the .debug$S section, and the linker must split these up between the PDB globals stream and PDB module stream, depending on whether it is a global typedef or a function local typedef.

Note there is a problem here, which is that we emit local typedefs using their fully qualified name, so we append the function name to the beginning. This is obviously wrong, and it is a simple fix, however it's orthogonal to this change so I'll submit that patch to the compiler in a followup.

[zturner]

Cleanup the test a little bit. Also added a CHECK-NOT: S_END line to make sure that the typedef we emit actually is inside the proper scope.

[zturner]

Update the diff for real.

[rnk]

lld/test/COFF/s_udt.test

1 ↗

(On Diff #176438)

Any particular reason for this to be an IR test that uses llc? LTO isn't involved, so you could either do a yaml input or an assembly file input. Which ever is more readable.

If you do want an IR test, can we name it .ll? We should be able to add .ll to the list of test case suffixes if it's not in there already.

llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp

52–59 ↗

(On Diff #176438)

I don't think we should assume xxHash64 doesn't have collisions. The space of 64-bit hashes is big, but it's not a content hash like SHA1, so it should be possible to construct some collisions. Someone has code to do it here: https://github.com/Cyan4973/xxHash/issues/54

If you don't want to add the collision resolution code because it's slow, at least try linking browser_tests with extra instrumentation code to detect collisions. If you find a collision, then we should keep the resolution code and add that as a test case. If you don't find a collision, then at least put a large comment here documenting our decision to sometimes drop typedefs with matching xxHash64 values.

Also, you might want to structure this code in such a way that it can be extended for S_CONSTANT, which also needs deduplication: https://bugs.llvm.org/show_bug.cgi?id=37933

[zturner]

llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp

52–59 ↗

(On Diff #176438)

The thing about collisions is that the consequences of a collision are that we end up with a few extra bytes in the PDB. So it seems unimportant to worry about a collision, even in the case that we do have collisions.

[aganea]

lld/test/COFF/s_udt.test

35 ↗

(On Diff #176442)

Could you possibly add here the source code that generated this test?

llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp

48 ↗

(On Diff #176442)

Could someone insert twice the same S_UDT record by calling GSIStreamBuilder::addGlobalSymbol(const UDTSym &Sym)? Replace Records.push_back by a call to addSymbol in that case.

[aganea]

llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp

52–59 ↗

(On Diff #176438)

Unless I'm missing something, @rnk is right: the PDB will not receive a colliding record. If you're inserting a S_UDT 'typedef char B' with hash 0x2, and a previous 'typedef int A' with same hash 0x2 was already there in UdtHashes, the new record 'B' will not make it to Records.push_back().

[zturner]

Yea he's right, I was just having a brain fail when I wrote that. I'm updating the code to correctly handle collisions. I don't think we need to do anything special for S_CONSTANT though, because the DenseSet<> is just holding hash values of generic CVSymbol records, which could be any kind of symbol, including S_CONSTANT. With a record-kind-agnostic hash set and the ability to handle collisions, we just need to change the if statement to a switch statement and we can handle all the types we want.

[zturner]

Changed the test to an llvm assembly test, and properly handle collisions. I actually added some instrumentation to detect if any collisions were actually occurring, and I did not find any with a PDB containing 450,000 S_UDT records. On the other hand, I did not measure a performance impact from adding this, so I'm adding it anyway. We can revisit later if it shows up on a profile.

Note that the test changed a little bit. For some reason, when I use an IR test and llc it generates different assembly than if I use an assembly test with llvm-mc. I don't know if this is normal, but throwing it out there just in case.

[aganea]

lld/test/COFF/s_udt.s

20 ↗

(On Diff #176680)

Just to be sure, 0x1003 is here because of the yet unlanded D55236? It should read 0x1006 once it lands? You would need to re-generate this test at that point?

llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp

64 ↗

(On Diff #176680)

Since you removed GSIStreamBuilder::addGlobalSymbol below, do we still need this function?

[zturner]

lld/test/COFF/s_udt.s

20 ↗

(On Diff #176680)

Yes, whoever lands second would need to update their patch.

llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp

64 ↗

(On Diff #176680)

I only removed the overload that takes a UDTSym (it was never called), but there are still other overloads. Once we extend the hash lookup to include types other than S_UDT, we can evaluate the other methods. I could also probably just change the push_back to an addSymbol since there's no reason not to. I'll do that offline before I submit.

[aganea]

Thank you!

lld/trunk/COFF/PDB.cpp

static void scopeStackClose(SmallVectorImpl<SymbolScope> &Stack,
   if (Stack.empty()) {
     warn("symbol scopes are not balanced in " + File->getName());
     return;
   }
   SymbolScope S = Stack.pop_back_val();
   S.OpeningRecord->PtrEnd = CurOffset;
 }
 
-static bool symbolGoesInModuleStream(const CVSymbol &Sym) {
+static bool symbolGoesInModuleStream(const CVSymbol &Sym, bool IsGlobalScope) {
   switch (Sym.kind()) {
   case SymbolKind::S_GDATA32:
   case SymbolKind::S_CONSTANT:
-  case SymbolKind::S_UDT:
   // We really should not be seeing S_PROCREF and S_LPROCREF in the first place
   // since they are synthesized by the linker in response to S_GPROC32 and
   // S_LPROC32, but if we do see them, don't put them in the module stream I
   // guess.
   case SymbolKind::S_PROCREF:
   case SymbolKind::S_LPROCREF:
     return false;
+  // S_UDT records go in the module stream if it is not a global S_UDT.
+  case SymbolKind::S_UDT:
+    return !IsGlobalScope;
   // S_GDATA32 does not go in the module stream, but S_LDATA32 does.
   case SymbolKind::S_LDATA32:
   default:
     return true;
   }
 }
 
-static bool symbolGoesInGlobalsStream(const CVSymbol &Sym) {
+static bool symbolGoesInGlobalsStream(const CVSymbol &Sym, bool IsGlobalScope) {
   switch (Sym.kind()) {
   case SymbolKind::S_CONSTANT:
   case SymbolKind::S_GDATA32:
   // S_LDATA32 goes in both the module stream and the globals stream.
   case SymbolKind::S_LDATA32:
   case SymbolKind::S_GPROC32:
   case SymbolKind::S_LPROC32:
   // We really should not be seeing S_PROCREF and S_LPROCREF in the first place
   // since they are synthesized by the linker in response to S_GPROC32 and
   // S_LPROC32, but if we do see them, copy them straight through.
   case SymbolKind::S_PROCREF:
   case SymbolKind::S_LPROCREF:
     return true;
-  // FIXME: For now, we drop all S_UDT symbols (i.e. they don't go in the
-  // globals stream or the modules stream).  These have special handling which
-  // needs more investigation before we can get right, but by putting them all
-  // into the globals stream WinDbg fails to display local variables of class
-  // types saying that it cannot find the type Foo *.  So as a stopgap just to
-  // keep things working, we drop them.
+  // S_UDT records go in the globals stream if it is a global S_UDT.
   case SymbolKind::S_UDT:
+    return IsGlobalScope;
   default:
     return false;
   }
 }
 
 static void addGlobalSymbol(pdb::GSIStreamBuilder &Builder, uint16_t ModIndex,
                             unsigned SymOffset, const CVSymbol &Sym) {
   switch (Sym.kind()) {
cantFail(forEachCodeViewRecord<CVSymbol>(
           scopeStackOpen(Scopes, CurSymOffset, Sym);
         else if (symbolEndsScope(Sym.kind()))
           scopeStackClose(Scopes, CurSymOffset, File);
 
         // Add the symbol to the globals stream if necessary.  Do this before
         // adding the symbol to the module since we may need to get the next
         // symbol offset, and writing to the module's symbol stream will update
         // that offset.
-        if (symbolGoesInGlobalsStream(Sym))
+        if (symbolGoesInGlobalsStream(Sym, Scopes.empty()))
           addGlobalSymbol(Builder.getGsiBuilder(),
                           File->ModuleDBI->getModuleIndex(), CurSymOffset, Sym);
 
-        if (symbolGoesInModuleStream(Sym)) {
+        if (symbolGoesInModuleStream(Sym, Scopes.empty())) {
           // Add symbols to the module in bulk. If this symbol is contiguous
           // with the previous run of symbols to add, combine the ranges. If
           // not, close the previous range of symbols and start a new one.
           if (Sym.data().data() == BulkSymbols.end()) {
             BulkSymbols = makeArrayRef(BulkSymbols.data(),
                                        BulkSymbols.size() + Sym.length());
           } else {
             File->ModuleDBI->addSymbolsInBulk(BulkSymbols);

lld/trunk/test/COFF/pdb-globals.test

 #    S_LPROCREF are added to the globals stream that refer to the module
 #    stream.
 #  * S_LDATA32 is copied byte for byte into both streams.
 
 
 CHECK-LABEL:                        Global Symbols
 CHECK-NEXT:  ============================================================
 CHECK-NEXT:   Records
+CHECK-NEXT:      340 | S_UDT [size = 20] `HelloPoint`
+CHECK-NEXT:            original type = 0x1007
 CHECK-NEXT:      208 | S_LPROCREF [size = 24] `LocalFunc`
 CHECK-NEXT:            module = 1, sum name = 0, offset = 292
 CHECK-NEXT:      160 | S_PROCREF [size = 28] `GlobalFunc`
 CHECK-NEXT:            module = 1, sum name = 0, offset = 52
 CHECK-NEXT:      188 | S_PROCREF [size = 20] `main`
 CHECK-NEXT:            module = 1, sum name = 0, offset = 108
 CHECK-NEXT:      232 | S_GDATA32 [size = 28] `__purecall`
 CHECK-NEXT:            type = 0x0403 (void*), addr = 0003:0004
 CHECK-NEXT:      260 | S_GDATA32 [size = 24] `GlobalVar`
 CHECK-NEXT:            type = 0x100B (const int*), addr = 0003:0000
+CHECK-NEXT:      312 | S_UDT [size = 28] `HelloPointTypedef`
+CHECK-NEXT:            original type = 0x1005
 CHECK-NEXT:      284 | S_LDATA32 [size = 28] `ConstantVar`
 CHECK-NEXT:            type = 0x100A (const int), addr = 0002:0000
-CHECK-NEXT:      312 | S_PROCREF [size = 40] `HelloPoint::HelloPoint`
+CHECK-NEXT:      360 | S_PROCREF [size = 40] `HelloPoint::HelloPoint`
 CHECK-NEXT:            module = 1, sum name = 0, offset = 376
 
 CHECK-LABEL:                           Symbols
 CHECK-NEXT:  ============================================================
 CHECK-NEXT:    Mod 0000
 CHECK-NOT:              | S_GDATA32
 CHECK-NOT:              | S_UDT
 CHECK:               52 | S_GPROC32 [size = 52] `GlobalFunc`
 CHECK:              108 | S_GPROC32 [size = 44] `main`
 CHECK:              292 | S_LPROC32 [size = 52] `LocalFunc`
 CHECK:              348 | S_LDATA32 [size = 28] `ConstantVar`
 CHECK:              376 | S_GPROC32 [size = 64] `HelloPoint::HelloPoint`

lld/trunk/test/COFF/pdb-symbol-types.yaml

 
 # Note that the type of 'global' goes from 0x1005 in the object file to 0x1004
 # in the PDB because the LF_FUNC_ID is moved to the id stream.
 
 # CHECK-LABEL:                  Global Symbols
 # CHECK-NEXT:  ============================================================
 # CHECK-NEXT:   Records
 # CHECK-NEXT:       48 | S_PROCREF [size = 20] `main`
-# CHECK-NEXT:             module = 1, sum name = 0, offset = 116
+# CHECK-NEXT:            module = 1, sum name = 0, offset = 116
+# CHECK-NEXT:       96 | S_UDT [size = 16] `UDT_Foo`
+# CHECK-NEXT:            original type = 0x1004
+# CHECK-NEXT:      112 | S_UDT [size = 12] `Foo`
+# CHECK-NEXT:            original type = 0x1004
 # CHECK-NEXT:       68 | S_GDATA32 [size = 28] `global_foo`
-# CHECK-NEXT:             type = 0x1004 (Foo), addr = 0003:0000
+# CHECK-NEXT:            type = 0x1004 (Foo), addr = 0003:0000
 
 # CHECK:                           Symbols
 # CHECK: ============================================================
 # CHECK-LABEL:   Mod 0000 | `{{.*}}pdb-symbol-types.yaml.tmp.obj`:
 # CHECK:     4 | S_OBJNAME [size = 52] sig=0, `C:\src\llvm-project\build\symbol-types.obj`
 # CHECK:    56 | S_COMPILE3 [size = 60]
 # CHECK:         machine = intel x86-x64, Ver = Microsoft (R) Optimizing Compiler, language = c
 # CHECK:         frontend = 19.0.24215.1, backend = 19.0.24215.1

lld/trunk/test/COFF/pdb-type-server-simple.test

 CHECK:   [[A_BUILD:[^ ]*]] | LF_BUILDINFO [size = 28]
 CHECK:            {{.*}}: `a.c`
 CHECK:   [[B_BUILD:[^ ]*]] | LF_BUILDINFO [size = 28]
 CHECK:            {{.*}}: `b.c`
 
 CHECK-LABEL:                       Global Symbols
 CHECK:       ============================================================
 CHECK-NEXT:    Records
-CHECK-NEXT:        36 | S_PROCREF [size = 20] `main`
-CHECK-NEXT:             module = 1, sum name = 0, offset = 104
-CHECK-NEXT:        56 | S_PROCREF [size = 16] `g`
-CHECK-NEXT:             module = 2, sum name = 0, offset = 104
+CHECK-NEXT:       36 | S_PROCREF [size = 20] `main`
+CHECK-NEXT:            module = 1, sum name = 0, offset = 104
+CHECK-NEXT:       68 | S_PROCREF [size = 16] `g`
+CHECK-NEXT:            module = 2, sum name = 0, offset = 104
+CHECK-NEXT:       56 | S_UDT [size = 12] `Foo`
+CHECK-NEXT:            original type = 0x1006
 
 CHECK-LABEL:                           Symbols
 CHECK: ============================================================
 CHECK-LABEL:   Mod 0000 | `{{.*}}a.obj`:
 CHECK:        4 | S_OBJNAME [size = 40] sig=0, `C:\src\llvm-project\build\a.obj`
 CHECK:      104 | S_GPROC32 [size = 44] `main`
 CHECK:            parent = 0, end = 196, addr = 0001:0000, code size = 27
 CHECK:            type = {{.*}}, debug start = 4, debug end = 22, flags = none

lld/trunk/test/COFF/s_udt.s

+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-windows-msvc < %s > %t.obj
+# RUN: lld-link /DEBUG:FULL /nodefaultlib /entry:main %t.obj /PDB:%t.pdb /OUT:%t.exe
+# RUN: llvm-pdbutil dump -types -globals -symbols -modi=0 %t.pdb | FileCheck %s
+
+# CHECK:                               Types (TPI Stream)
+# CHECK-NEXT: ============================================================
+# CHECK:      0x1003 | LF_STRUCTURE [size = 44] `Struct`
+# CHECK-NEXT:          unique name: `.?AUStruct@@`
+# CHECK-NEXT:          vtable: <no type>, base list: <no type>, field list: <no type>
+# CHECK-NEXT:          options: forward ref (-> 0x1006) | has unique name, sizeof 0
+# CHECK-NEXT: 0x1004 | LF_POINTER [size = 12]
+# CHECK-NEXT:          referent = 0x1003, mode = pointer, opts = None, kind = ptr64
+# CHECK:      0x1006 | LF_STRUCTURE [size = 44] `Struct`
+# CHECK-NEXT:          unique name: `.?AUStruct@@`
+# CHECK-NEXT:          vtable: <no type>, base list: <no type>, field list: 0x1005
+# CHECK-NEXT:          options: has unique name, sizeof 4
+# CHECK:                               Global Symbols
+# CHECK-NEXT: ============================================================
+# CHECK:      {{.*}} | S_UDT [size = 24] `StructTypedef`
+# CHECK:               original type = 0x1003
+# CHECK:      {{.*}} | S_UDT [size = 16] `Struct`
+# CHECK:               original type = 0x1006
+# CHECK:      {{.*}} | S_UDT [size = 20] `IntTypedef`
+# CHECK:               original type = 0x0074 (int)
+# CHECK:                               Symbols
+# CHECK-NEXT: ============================================================
+# CHECK:      {{.*}} | S_GPROC32 [size = 44] `main`
+# CHECK-NEXT:          parent = 0, end = 252, addr = 0001:0000, code size = 52
+# CHECK-NEXT:          type = `0x1002 (int (int, char**))`, debug start = 0, debug end = 0, flags = none
+# CHECK-NOT:  {{.*}} | S_END
+# CHECK:      {{.*}} | S_UDT [size = 28] `main::LocalTypedef`
+# CHECK-NEXT:          original type = 0x1004
+# CHECK:      {{.*}} | S_END [size = 4]
+
+# source code to re-generate:
+# clang-cl /Z7 /GS- /GR- /c foo.cpp
+#
+# struct Struct {
+#   int x;
+# };
+# 
+# using IntTypedef = int;
+# using StructTypedef = Struct;
+# Struct S;
+# StructTypedef SS;
+# IntTypedef I;
+# 
+# int main(int argc, char **argv) {
+#   using LocalTypedef = Struct*;
+#   LocalTypedef SPtr;
+#   return I + S.x + SS.x + SPtr->x;
+# }
+
+  .text
+  .def   @feat.00;
+  .scl  3;
+  .type 0;
+  .endef
+  .globl  @feat.00
+.set @feat.00, 0
+  .intel_syntax noprefix
+  .def   main;
+  .scl  2;
+  .type 32;
+  .endef
+  .globl  main                    # -- Begin function main
+  .p2align  4, 0x90
+main:                                   # @main
+.Lfunc_begin0:
+  .cv_func_id 0
+  .cv_file  1 "D:\\src\\llvmbuild\\cl\\Debug\\x64\\foo.cpp" "2B62298EE3EEF94E1D81FDFE18BD46A6" 1
+  .cv_loc 0 1 12 0                # foo.cpp:12:0
+.seh_proc main
+# %bb.0:                                # %entry
+  sub rsp, 32
+  .seh_stackalloc 32
+  .seh_endprologue
+  mov dword ptr [rsp + 28], 0
+  mov qword ptr [rsp + 16], rdx
+  mov dword ptr [rsp + 12], ecx
+.Ltmp0:
+  .cv_loc 0 1 15 0                # foo.cpp:15:0
+  mov  ecx, dword ptr [rip + "?I@@3HA"]
+  add  ecx, dword ptr [rip + "?S@@3UStruct@@A"]
+  add  ecx, dword ptr [rip + "?SS@@3UStruct@@A"]
+  mov rdx, qword ptr [rsp]
+  add ecx, dword ptr [rdx]
+  mov eax, ecx
+  add rsp, 32
+  ret
+.Ltmp1:
+.Lfunc_end0:
+  .seh_handlerdata
+  .text
+  .seh_endproc
+                                        # -- End function
+  .bss
+  .globl  "?S@@3UStruct@@A"       # @"?S@@3UStruct@@A"
+  .p2align  2
+"?S@@3UStruct@@A":
+  .zero 4
+
+  .globl  "?SS@@3UStruct@@A"      # @"?SS@@3UStruct@@A"
+  .p2align  2
+"?SS@@3UStruct@@A":
+  .zero 4
+
+  .globl  "?I@@3HA"               # @"?I@@3HA"
+  .p2align  2
+"?I@@3HA":
+  .long 0                       # 0x0
+
+  .section  .drectve,"yn"
+  .ascii  " /DEFAULTLIB:libcmt.lib"
+  .ascii  " /DEFAULTLIB:oldnames.lib"
+  .section  .debug$S,"dr"
+  .p2align  2
+  .long 4                       # Debug section magic
+  .long 241
+  .long .Ltmp3-.Ltmp2           # Subsection size
+.Ltmp2:
+  .short  .Ltmp5-.Ltmp4           # Record length
+.Ltmp4:
+  .short  4412                    # Record kind: S_COMPILE3
+  .long 1                       # Flags and language
+  .short  208                     # CPUType
+  .short  8                       # Frontend version
+  .short  0
+  .short  0
+  .short  0
+  .short  8000                    # Backend version
+  .short  0
+  .short  0
+  .short  0
+  .asciz  "clang version 8.0.0 "  # Null-terminated compiler version string
+.Ltmp5:
+.Ltmp3:
+  .p2align  2
+  .long 241                     # Symbol subsection for main
+  .long .Ltmp7-.Ltmp6           # Subsection size
+.Ltmp6:
+  .short  .Ltmp9-.Ltmp8           # Record length
+.Ltmp8:
+  .short  4423                    # Record kind: S_GPROC32_ID
+  .long 0                       # PtrParent
+  .long 0                       # PtrEnd
+  .long 0                       # PtrNext
+  .long .Lfunc_end0-main        # Code size
+  .long 0                       # Offset after prologue
+  .long 0                       # Offset before epilogue
+  .long 4099                    # Function type index
+  .secrel32 main            # Function section relative address
+  .secidx main                    # Function section index
+  .byte 0                       # Flags
+  .asciz  "main"                  # Function name
+.Ltmp9:
+  .short  .Ltmp11-.Ltmp10         # Record length
+.Ltmp10:
+  .short  4114                    # Record kind: S_FRAMEPROC
+  .long 32                      # FrameSize
+  .long 0                       # Padding
+  .long 0                       # Offset of padding
+  .long 0                       # Bytes of callee saved registers
+  .long 0                       # Exception handler offset
+  .short  0                       # Exception handler section
+  .long 81920                   # Flags (defines frame register)
+.Ltmp11:
+  .short  .Ltmp13-.Ltmp12         # Record length
+.Ltmp12:
+  .short  4414                    # Record kind: S_LOCAL
+  .long 116                     # TypeIndex
+  .short  1                       # Flags
+  .asciz  "argc"
+.Ltmp13:
+  .cv_def_range  .Ltmp0 .Ltmp1, "B\021\f\000\000\000"
+  .short  .Ltmp15-.Ltmp14         # Record length
+.Ltmp14:
+  .short  4414                    # Record kind: S_LOCAL
+  .long 4096                    # TypeIndex
+  .short  1                       # Flags
+  .asciz  "argv"
+.Ltmp15:
+  .cv_def_range  .Ltmp0 .Ltmp1, "B\021\020\000\000\000"
+  .short  .Ltmp17-.Ltmp16         # Record length
+.Ltmp16:
+  .short  4414                    # Record kind: S_LOCAL
+  .long 4101                    # TypeIndex
+  .short  0                       # Flags
+  .asciz  "SPtr"
+.Ltmp17:
+  .cv_def_range  .Ltmp0 .Ltmp1, "B\021\000\000\000\000"
+  .short  .Ltmp19-.Ltmp18         # Record length
+.Ltmp18:
+  .short  4360                    # Record kind: S_UDT
+  .long 4101                    # Type
+  .asciz  "main::LocalTypedef"
+.Ltmp19:
+  .short  2                       # Record length
+  .short  4431                    # Record kind: S_PROC_ID_END
+.Ltmp7:
+  .p2align  2
+  .cv_linetable 0, main, .Lfunc_end0
+  .long 241                     # Symbol subsection for globals
+  .long .Ltmp21-.Ltmp20         # Subsection size
+.Ltmp20:
+  .short  .Ltmp23-.Ltmp22         # Record length
+.Ltmp22:
+  .short  4365                    # Record kind: S_GDATA32
+  .long 4103                    # Type
+  .secrel32 "?S@@3UStruct@@A" # DataOffset
+  .secidx "?S@@3UStruct@@A"       # Segment
+  .asciz  "S"                     # Name
+.Ltmp23:
+  .short  .Ltmp25-.Ltmp24         # Record length
+.Ltmp24:
+  .short  4365                    # Record kind: S_GDATA32
+  .long 4100                    # Type
+  .secrel32 "?SS@@3UStruct@@A" # DataOffset
+  .secidx "?SS@@3UStruct@@A"      # Segment
+  .asciz  "SS"                    # Name
+.Ltmp25:
+  .short  .Ltmp27-.Ltmp26         # Record length
+.Ltmp26:
+  .short  4365                    # Record kind: S_GDATA32
+  .long 116                     # Type
+  .secrel32 "?I@@3HA"       # DataOffset
+  .secidx "?I@@3HA"               # Segment
+  .asciz  "I"                     # Name
+.Ltmp27:
+.Ltmp21:
+  .p2align  2
+  .long 241
+  .long .Ltmp29-.Ltmp28         # Subsection size
+.Ltmp28:
+  .short  .Ltmp31-.Ltmp30         # Record length
+.Ltmp30:
+  .short  4360                    # Record kind: S_UDT
+  .long 4103                    # Type
+  .asciz  "Struct"
+.Ltmp31:
+  .short  .Ltmp33-.Ltmp32         # Record length
+.Ltmp32:
+  .short  4360                    # Record kind: S_UDT
+  .long 4100                    # Type
+  .asciz  "StructTypedef"
+.Ltmp33:
+  .short  .Ltmp35-.Ltmp34         # Record length
+.Ltmp34:
+  .short  4360                    # Record kind: S_UDT
+  .long 116                     # Type
+  .asciz  "IntTypedef"
+.Ltmp35:
+.Ltmp29:
+  .p2align  2
+  .cv_filechecksums               # File index to string table offset subsection
+  .cv_stringtable                 # String table
+  .long 241
+  .long .Ltmp37-.Ltmp36         # Subsection size
+.Ltmp36:
+  .short  6                       # Record length
+  .short  4428                    # Record kind: S_BUILDINFO
+  .long 4108                    # LF_BUILDINFO index
+.Ltmp37:
+  .p2align  2
+  .section  .debug$T,"dr"
+  .p2align  2
+  .long 4                       # Debug section magic
+  # Pointer (0x1000) {
+  #   TypeLeafKind: LF_POINTER (0x1002)
+  #   PointeeType: char* (0x670)
+  #   PtrType: Near64 (0xC)
+  #   PtrMode: Pointer (0x0)
+  #   IsFlat: 0
+  #   IsConst: 0
+  #   IsVolatile: 0
+  #   IsUnaligned: 0
+  #   IsRestrict: 0
+  #   IsThisPtr&: 0
+  #   IsThisPtr&&: 0
+  #   SizeOf: 8
+  # }
+  .byte 0x0a, 0x00, 0x02, 0x10
+  .byte 0x70, 0x06, 0x00, 0x00
+  .byte 0x0c, 0x00, 0x01, 0x00
+  # ArgList (0x1001) {
+  #   TypeLeafKind: LF_ARGLIST (0x1201)
+  #   NumArgs: 2
+  #   Arguments [
+  #     ArgType: int (0x74)
+  #     ArgType: char** (0x1000)
+  #   ]
+  # }
+  .byte 0x0e, 0x00, 0x01, 0x12
+  .byte 0x02, 0x00, 0x00, 0x00
+  .byte 0x74, 0x00, 0x00, 0x00
+  .byte 0x00, 0x10, 0x00, 0x00
+  # Procedure (0x1002) {
+  #   TypeLeafKind: LF_PROCEDURE (0x1008)
+  #   ReturnType: int (0x74)
+  #   CallingConvention: NearC (0x0)
+  #   FunctionOptions [ (0x0)
+  #   ]
+  #   NumParameters: 2
+  #   ArgListType: (int, char**) (0x1001)
+  # }
+  .byte 0x0e, 0x00, 0x08, 0x10
+  .byte 0x74, 0x00, 0x00, 0x00
+  .byte 0x00, 0x00, 0x02, 0x00
+  .byte 0x01, 0x10, 0x00, 0x00
+  # FuncId (0x1003) {
+  #   TypeLeafKind: LF_FUNC_ID (0x1601)
+  #   ParentScope: 0x0
+  #   FunctionType: int (int, char**) (0x1002)
+  #   Name: main
+  # }
+  .byte 0x12, 0x00, 0x01, 0x16
+  .byte 0x00, 0x00, 0x00, 0x00
+  .byte 0x02, 0x10, 0x00, 0x00
+  .byte 0x6d, 0x61, 0x69, 0x6e
+  .byte 0x00, 0xf3, 0xf2, 0xf1
+  # Struct (0x1004) {
+  #   TypeLeafKind: LF_STRUCTURE (0x1505)
+  #   MemberCount: 0
+  #   Properties [ (0x280)
+  #     ForwardReference (0x80)
+  #     HasUniqueName (0x200)
+  #   ]
+  #   FieldList: 0x0
+  #   DerivedFrom: 0x0
+  #   VShape: 0x0
+  #   SizeOf: 0
+  #   Name: Struct
+  #   LinkageName: .?AUStruct@@
+  # }
+  .byte 0x2a, 0x00, 0x05, 0x15
+  .byte 0x00, 0x00, 0x80, 0x02
+  .byte 0x00, 0x00, 0x00, 0x00
+  .byte 0x00, 0x00, 0x00, 0x00
+  .byte 0x00, 0x00, 0x00, 0x00
+  .byte 0x00, 0x00, 0x53, 0x74
+  .byte 0x72, 0x75, 0x63, 0x74
+  .byte 0x00, 0x2e, 0x3f, 0x41
+  .byte 0x55, 0x53, 0x74, 0x72
+  .byte 0x75, 0x63, 0x74, 0x40
+  .byte 0x40, 0x00, 0xf2, 0xf1
+  # Pointer (0x1005) {
+  #   TypeLeafKind: LF_POINTER (0x1002)
+  #   PointeeType: Struct (0x1004)
+  #   PtrType: Near64 (0xC)
+  #   PtrMode: Pointer (0x0)
+  #   IsFlat: 0
+  #   IsConst: 0
+  #   IsVolatile: 0
+  #   IsUnaligned: 0
+  #   IsRestrict: 0
+  #   IsThisPtr&: 0
+  #   IsThisPtr&&: 0
+  #   SizeOf: 8
+  # }
+  .byte 0x0a, 0x00, 0x02, 0x10
+  .byte 0x04, 0x10, 0x00, 0x00
+  .byte 0x0c, 0x00, 0x01, 0x00
+  # FieldList (0x1006) {
+  #   TypeLeafKind: LF_FIELDLIST (0x1203)
+  #   DataMember {
+  #     TypeLeafKind: LF_MEMBER (0x150D)
+  #     AccessSpecifier: Public (0x3)
+  #     Type: int (0x74)
+  #     FieldOffset: 0x0
+  #     Name: x
+  #   }
+  # }
+  .byte 0x0e, 0x00, 0x03, 0x12
+  .byte 0x0d, 0x15, 0x03, 0x00
+  .byte 0x74, 0x00, 0x00, 0x00
+  .byte 0x00, 0x00, 0x78, 0x00
+  # Struct (0x1007) {
+  #   TypeLeafKind: LF_STRUCTURE (0x1505)
+  #   MemberCount: 1
+  #   Properties [ (0x200)
+  #     HasUniqueName (0x200)
+  #   ]
+  #   FieldList: <field list> (0x1006)
+  #   DerivedFrom: 0x0
+  #   VShape: 0x0
+  #   SizeOf: 4
+  #   Name: Struct
+  #   LinkageName: .?AUStruct@@
+  # }
+  .byte 0x2a, 0x00, 0x05, 0x15
+  .byte 0x01, 0x00, 0x00, 0x02
+  .byte 0x06, 0x10, 0x00, 0x00
+  .byte 0x00, 0x00, 0x00, 0x00
+  .byte 0x00, 0x00, 0x00, 0x00
+  .byte 0x04, 0x00, 0x53, 0x74
+  .byte 0x72, 0x75, 0x63, 0x74
+  .byte 0x00, 0x2e, 0x3f, 0x41
+  .byte 0x55, 0x53, 0x74, 0x72
+  .byte 0x75, 0x63, 0x74, 0x40
+  .byte 0x40, 0x00, 0xf2, 0xf1
+  # StringId (0x1008) {
+  #   TypeLeafKind: LF_STRING_ID (0x1605)
+  #   Id: 0x0
+  #   StringData: D:\src\llvmbuild\cl\Debug\x64\foo.cpp
+  # }
+  .byte 0x2e, 0x00, 0x05, 0x16
+  .byte 0x00, 0x00, 0x00, 0x00
+  .byte 0x44, 0x3a, 0x5c, 0x73
+  .byte 0x72, 0x63, 0x5c, 0x6c
+  .byte 0x6c, 0x76, 0x6d, 0x62
+  .byte 0x75, 0x69, 0x6c, 0x64
+  .byte 0x5c, 0x63, 0x6c, 0x5c
+  .byte 0x44, 0x65, 0x62, 0x75
+  .byte 0x67, 0x5c, 0x78, 0x36
+  .byte 0x34, 0x5c, 0x66, 0x6f
+  .byte 0x6f, 0x2e, 0x63, 0x70
+  .byte 0x70, 0x00, 0xf2, 0xf1
+  # UdtSourceLine (0x1009) {
+  #   TypeLeafKind: LF_UDT_SRC_LINE (0x1606)
+  #   UDT: Struct (0x1007)
+  #   SourceFile: D:\src\llvmbuild\cl\Debug\x64\foo.cpp (0x1008)
+  #   LineNumber: 1
+  # }
+  .byte 0x0e, 0x00, 0x06, 0x16
+  .byte 0x07, 0x10, 0x00, 0x00
+  .byte 0x08, 0x10, 0x00, 0x00
+  .byte 0x01, 0x00, 0x00, 0x00
+  # StringId (0x100A) {
+  #   TypeLeafKind: LF_STRING_ID (0x1605)
+  #   Id: 0x0
+  #   StringData: D:\\src\\llvmbuild\\cl\\Debug\\x64
+  # }
+  .byte 0x2a, 0x00, 0x05, 0x16
+  .byte 0x00, 0x00, 0x00, 0x00
+  .byte 0x44, 0x3a, 0x5c, 0x5c
+  .byte 0x73, 0x72, 0x63, 0x5c
+  .byte 0x5c, 0x6c, 0x6c, 0x76
+  .byte 0x6d, 0x62, 0x75, 0x69
+  .byte 0x6c, 0x64, 0x5c, 0x5c
+  .byte 0x63, 0x6c, 0x5c, 0x5c
+  .byte 0x44, 0x65, 0x62, 0x75
+  .byte 0x67, 0x5c, 0x5c, 0x78
+  .byte 0x36, 0x34, 0x00, 0xf1
+  # StringId (0x100B) {
+  #   TypeLeafKind: LF_STRING_ID (0x1605)
+  #   Id: 0x0
+  #   StringData: foo.cpp
+  # }
+  .byte 0x0e, 0x00, 0x05, 0x16
+  .byte 0x00, 0x00, 0x00, 0x00
+  .byte 0x66, 0x6f, 0x6f, 0x2e
+  .byte 0x63, 0x70, 0x70, 0x00
+  # BuildInfo (0x100C) {
+  #   TypeLeafKind: LF_BUILDINFO (0x1603)
+  #   NumArgs: 5
+  #   Arguments [
+  #     ArgType: D:\\src\\llvmbuild\\cl\\Debug\\x64 (0x100A)
+  #     ArgType: 0x0
+  #     ArgType: foo.cpp (0x100B)
+  #     ArgType: 0x0
+  #     ArgType: 0x0
+  #   ]
+  # }
+  .byte 0x1a, 0x00, 0x03, 0x16
+  .byte 0x05, 0x00, 0x0a, 0x10
+  .byte 0x00, 0x00, 0x00, 0x00
+  .byte 0x00, 0x00, 0x0b, 0x10
+  .byte 0x00, 0x00, 0x00, 0x00
+  .byte 0x00, 0x00, 0x00, 0x00
+  .byte 0x00, 0x00, 0xf2, 0xf1
+
+  .addrsig
+  .addrsig_sym "?S@@3UStruct@@A"
+  .addrsig_sym "?SS@@3UStruct@@A"
+  .addrsig_sym "?I@@3HA"

llvm/trunk/include/llvm/DebugInfo/PDB/Native/GSIStreamBuilder.h

public:
   uint32_t getGlobalsStreamIndex() const;
   uint32_t getRecordStreamIdx() const { return RecordStreamIdx; }
 
   void addPublicSymbol(const codeview::PublicSym32 &Pub);
 
   void addGlobalSymbol(const codeview::ProcRefSym &Sym);
   void addGlobalSymbol(const codeview::DataSym &Sym);
   void addGlobalSymbol(const codeview::ConstantSym &Sym);
-  void addGlobalSymbol(const codeview::UDTSym &Sym);
   void addGlobalSymbol(const codeview::CVSymbol &Sym);
 
 private:
   uint32_t calculatePublicsHashStreamSize() const;
   uint32_t calculateGlobalsHashStreamSize() const;
   Error commitSymbolRecordStream(WritableBinaryStreamRef Stream);
   Error commitPublicsHashStream(WritableBinaryStreamRef Stream);
   Error commitGlobalsHashStream(WritableBinaryStreamRef Stream);

llvm/trunk/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp

 //===- DbiStreamBuilder.cpp - PDB Dbi Stream Creation -----------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/DebugInfo/PDB/Native/GSIStreamBuilder.h"
 
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/DebugInfo/CodeView/RecordName.h"
 #include "llvm/DebugInfo/CodeView/SymbolDeserializer.h"
 #include "llvm/DebugInfo/CodeView/SymbolRecord.h"
 #include "llvm/DebugInfo/CodeView/SymbolSerializer.h"
 #include "llvm/DebugInfo/MSF/MSFBuilder.h"
 #include "llvm/DebugInfo/MSF/MSFCommon.h"
 #include "llvm/DebugInfo/MSF/MappedBlockStream.h"
 #include "llvm/DebugInfo/PDB/Native/GlobalsStream.h"
 #include "llvm/DebugInfo/PDB/Native/Hash.h"
 #include "llvm/Support/BinaryItemStream.h"
 #include "llvm/Support/BinaryStreamWriter.h"
+#include "llvm/Support/xxhash.h"
 #include <algorithm>
 #include <vector>
 
 using namespace llvm;
 using namespace llvm::msf;
 using namespace llvm::pdb;
 using namespace llvm::codeview;
 
 struct llvm::pdb::GSIHashStreamBuilder {
+  struct UdtDenseMapInfo {
+    static inline CVSymbol getEmptyKey() {
+      static CVSymbol Empty;
+      return Empty;
+    }
+    static inline CVSymbol getTombstoneKey() {
+      static CVSymbol Tombstone(static_cast<SymbolKind>(-1),
+                                ArrayRef<uint8_t>());
+      return Tombstone;
+    }
+    static unsigned getHashValue(const CVSymbol &Val) {
+      return xxHash64(Val.RecordData);
+    }
+    static bool isEqual(const CVSymbol &LHS, const CVSymbol &RHS) {
+      return LHS.RecordData == RHS.RecordData;
+    }
+  };
+
   std::vector<CVSymbol> Records;
   uint32_t StreamIndex;
+  llvm::DenseSet<CVSymbol, UdtDenseMapInfo> UdtHashes;
   std::vector<PSHashRecord> HashRecords;
   std::array<support::ulittle32_t, (IPHR_HASH + 32) / 32> HashBitmap;
   std::vector<support::ulittle32_t> HashBuckets;
 
   uint32_t calculateSerializedLength() const;
   uint32_t calculateRecordByteSize() const;
   Error commit(BinaryStreamWriter &Writer);
   void finalizeBuckets(uint32_t RecordZeroOffset);
 
   template <typename T> void addSymbol(const T &Symbol, MSFBuilder &Msf) {
     T Copy(Symbol);
-    Records.push_back(SymbolSerializer::writeOneSymbol(Copy, Msf.getAllocator(),
-                                                       CodeViewContainer::Pdb));
+    addSymbol(SymbolSerializer::writeOneSymbol(Copy, Msf.getAllocator(),
   }
-  void addSymbol(const CVSymbol &Symbol) { Records.push_back(Symbol); }
+  void addSymbol(const CVSymbol &Symbol) {
+    if (Symbol.kind() == S_UDT) {
+      auto Iter = UdtHashes.insert(Symbol);
+      if (!Iter.second)
+        return;
+    }
+
+    Records.push_back(Symbol);
+  }
 };
 
 uint32_t GSIHashStreamBuilder::calculateSerializedLength() const {
   uint32_t Size = sizeof(GSIHashHeader);
   Size += HashRecords.size() * sizeof(PSHashRecord);
   Size += HashBitmap.size() * sizeof(uint32_t);
   Size += HashBuckets.size() * sizeof(uint32_t);
   return Size;

 void GSIStreamBuilder::addGlobalSymbol(const DataSym &Sym) {
   GSH->addSymbol(Sym, Msf);
 }
 
 void GSIStreamBuilder::addGlobalSymbol(const ConstantSym &Sym) {
   GSH->addSymbol(Sym, Msf);
 }
 
-void GSIStreamBuilder::addGlobalSymbol(const UDTSym &Sym) {
-  GSH->addSymbol(Sym, Msf);
-}
-
 void GSIStreamBuilder::addGlobalSymbol(const codeview::CVSymbol &Sym) {
   GSH->addSymbol(Sym);
 }
 
 static Error writeRecords(BinaryStreamWriter &Writer,
                           ArrayRef<CVSymbol> Records) {
   BinaryItemStream<CVSymbol> ItemStream(support::endianness::little);
   ItemStream.setItems(Records);