Fix a few small issues in llvm-pdbutil

[lemo]

Summary

Running "llvm-pdbutil dump -all" on linux (using the native PDB reader), over a few PDBs pulled from the Microsoft public symbol store uncovered a few issues:

• stripped PDBs might not have the strings stream (/names)
• stripped PDBs might not have the "module info" stream

[zturner]

Out of curiosity, is this a PDB you generated yourself by compiling something and running pdbstrip on it?

test/tools/llvm-pdbdump/stripped.test

4–5

Can you print the rest of this section? For example, I think there's actually a field here that says it's stripped.

7–8

This section might be interesting to print too. If for no other reason than to server as a reference of what streams are "normal" to expect in a stripped PDB.

tools/llvm-pdbutil/InputFile.cpp

182–183

This assert doesn't seem necessary, as it's essentially checking for an out of memory condition.

[lemo]

Incorporating CR feedback

[lemo]

Thanks Zach for your assistance with this!

test/tools/llvm-pdbdump/stripped.test

7–8

I agree. There's a slight downside though - the test becomes very sensitive to the output, including formatting details.

[zturner]

test/tools/llvm-pdbdump/stripped.test

7–8

FileCheck is pretty helpful when it comes to this kind of thing. You can regex out anything by putting {{.*}} (so stream sizes, for example), and also you can leave off anything at the end of a line. So for example, for Stream 3 (just picking a random one), you could write your line as:

; CHECK:   Stream 3 ({{.*}} bytes): [DBI Stream]
; CHECK:   Stream 4

Note the absence of the NEXT (because we're skipping blocks, and we're regex'ing out the stream sizes. So all of the sensitive stuff goes away.

However.... I don't think we should check in this particular PDB (or any other PDB that we didn't generate ourselves). Microsoft has tools that will strip PDBs, although I forget what it's called off the top of my head. Can we generate a private PDB using cl.exe, strip it using their tool, verify that it has the same properties as this PDB, and then use that one instead?

[lemo]

Using a new, smaller test pdb (Stripped.pdb)

[zturner]

LGTM! Thanks

test/tools/llvm-pdbdump/checksum-string.test

+; REQUIRES: diasdk
 ; RUN: llvm-pdbutil pretty -lines %p/Inputs/PrettyFuncDumperTest.pdb > %t
 
 ; CHECK: ---COMPILANDS---
 ; CHECK: {{.*}}\PrettyFuncDumperTest.obj
 ; CHECK: {{.*}}\prettyfuncdumpertest.cpp (MD5: E36B273C4D7B3D70C996387C95A6C4F7)

test/tools/llvm-pdbdump/class-layout.test

+; REQUIRES: diasdk
+
 ; RUN: llvm-pdbutil pretty -all -class-recurse-depth=1 \
 ; RUN:   %p/Inputs/ClassLayoutTest.pdb > %t
 ; RUN: FileCheck -input-file=%t %s -check-prefix=GLOBALS_TEST
 ; RUN: FileCheck -input-file=%t %s -check-prefix=MEMBERS_TEST
 ; RUN: FileCheck -input-file=%t %s -check-prefix=BASE_CLASS_A
 ; RUN: FileCheck -input-file=%t %s -check-prefix=BASE_CLASS_B
 ; RUN: FileCheck -input-file=%t %s -check-prefix=BASE_CLASS_C
 ; RUN: FileCheck -input-file=%t %s -check-prefix=BASE_CLASS_D

test/tools/llvm-pdbdump/complex-padding-graphical.test

+; REQUIRES: diasdk
+
 ; RUN: llvm-pdbutil pretty -classes -class-definitions=layout \
 ; RUN:     -include-types=Test %p/Inputs/ComplexPaddingTest.pdb > %t
 
 ; RUN: FileCheck -input-file=%t %s -check-prefix=DIRECT_VB_ONLY
 ; RUN: FileCheck -input-file=%t %s -check-prefix=DIRECT_VB_AND_NON_VB
 ; RUN: FileCheck -input-file=%t %s -check-prefix=INDIRECT_VB
 ; RUN: FileCheck -input-file=%t %s -check-prefix=INDIRECT_AND_DIRECT_VB
 

test/tools/llvm-pdbdump/enum-layout.test

+; REQUIRES: diasdk
+
 ; RUN: llvm-pdbutil pretty -types %p/Inputs/ClassLayoutTest.pdb > %t
 ; RUN: FileCheck -input-file=%t %s -check-prefix=GLOBAL_ENUM
 ; RUN: FileCheck -input-file=%t %s -check-prefix=MEMBER_ENUM
 
 ; GLOBAL_ENUM: ---TYPES---
 ; GLOBAL_ENUM: Enums:
 ; GLOBAL_ENUM: enum GlobalsTest::Enum {
 ; GLOBAL_ENUM-NEXT: Val1 = 0

test/tools/llvm-pdbdump/explain-dbi-stream.test

+; REQUIRES: diasdk
+
 ; RUN: llvm-pdbutil explain \
 ; RUN: -offset=0xF000 \
 ; RUN: -offset=0xF004 \
 ; RUN: -offset=0xF008 \
 ; RUN: -offset=0xF00C \
 ; RUN: -offset=0xF00E \
 ; RUN: -offset=0xF010 \
 ; RUN: -offset=0xF012 \

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

+; REQUIRES: diasdk
+
 ; RUN: llvm-pdbutil explain \
 ; RUN: -offset=0x11000 \
 ; RUN: -offset=0x11004 \
 ; RUN: -offset=0x11008 \
 ; RUN: -offset=0x1100C \
 ; RUN: -offset=0x1101C \
 ; RUN: %p/Inputs/InjectedSource.pdb | FileCheck %s
 

test/tools/llvm-pdbdump/explain.test

+; REQUIRES: diasdk
+
 ; RUN: llvm-pdbutil explain -offset=0 %p/Inputs/InjectedSource.pdb \
 ; RUN:  | FileCheck --check-prefix=ZERO %s
 ; RUN: llvm-pdbutil explain -offset=40 %p/Inputs/InjectedSource.pdb \
 ; RUN:  | FileCheck --check-prefix=FORTY %s
 ; RUN: llvm-pdbutil explain -offset=60 %p/Inputs/InjectedSource.pdb \
 ; RUN:  | FileCheck --check-prefix=SIXTY %s
 
 ; RUN: llvm-pdbutil explain -offset=0x1000 %p/Inputs/InjectedSource.pdb \

test/tools/llvm-pdbdump/injected-sources.test

 ; The PDB committed to the repo does not seem to be recognized by older
 ; versions of DIA SDK, so we xfail the test temporarily until we can
 ; figure out how to get a PDB that makes all versions of MSVC happy.
+; REQUIRES: diasdk
+
 ; RUN: llvm-pdbutil pretty -injected-sources -injected-source-content \
 ; RUN:   %p/Inputs/InjectedSource.pdb | FileCheck %s
 ; RUN: llvm-pdbutil pretty -injected-sources -injected-source-content \
 ; RUN:   %p/Inputs/ClassLayoutTest.pdb | FileCheck --check-prefix=NEGATIVE %s
 
 ; CHECK:      ---INJECTED SOURCES---
 ; CHECK:      c.natvis (140 bytes): obj=<null>, vname=c.natvis, crc=334478030, compression=None
 ; CHECK-NEXT: <?xml version="1.0" encoding="utf-8"?>

test/tools/llvm-pdbdump/lit.local.cfg

-config.unsupported = not config.have_dia_sdk
+if config.have_dia_sdk:
+  config.available_features.add("diasdk")

test/tools/llvm-pdbdump/load-address.test

+; REQUIRES: diasdk
+
 ; RUN: llvm-pdbutil pretty -externals %p/Inputs/LoadAddressTest.pdb \
 ; RUN:    | FileCheck --check-prefix=RVA %s
 ; RUN: llvm-pdbutil pretty -externals -load-address=0x40000000 \
 ; RUN: %p/Inputs/LoadAddressTest.pdb | FileCheck --check-prefix=VA %s
 
 ; RVA: ---EXTERNALS---
 ; RVA: [0x00001010] _main
 
 ; VA: ---EXTERNALS---
 ; VA: [0x40001010] _main

test/tools/llvm-pdbdump/pretty-func-dumper.test

+; REQUIRES: diasdk
+
 ; RUN: llvm-pdbutil pretty -all -class-recurse-depth=1 \
 ; RUN:   %p/Inputs/PrettyFuncDumperTest.pdb > %t
 ; RUN: FileCheck -input-file=%t %s -check-prefix=GLOBALS_FUNC
 ; RUN: FileCheck -input-file=%t %s -check-prefix=GLOBALS_DATA
 ; RUN: FileCheck -input-file=%t %s -check-prefix=TYPEDEF
 
 ; GLOBALS_FUNC: ---GLOBALS---
 ; GLOBALS_FUNC-DAG: void __cdecl NS::Func(char a, int b, ...)

test/tools/llvm-pdbdump/regex-filter.test

+; REQUIRES: diasdk
+
 ; RUN: llvm-pdbutil pretty -module-syms -globals -types %p/Inputs/FilterTest.pdb \
 ; RUN:    | FileCheck --check-prefix=NO_FILTER %s
 
 ; RUN: llvm-pdbutil pretty -types -exclude-types="GlobalTypedef|NestedTypedef" \
 ; RUN:    %p/Inputs/FilterTest.pdb | FileCheck --check-prefix=EXCLUDE_TYPEDEFS %s
 ; RUN: llvm-pdbutil pretty -classes -enums %p/Inputs/FilterTest.pdb \
 ; RUN:    | FileCheck --check-prefix=EXCLUDE_TYPEDEFS %s
 

test/tools/llvm-pdbdump/simple-padding-graphical.test

+; REQUIRES: diasdk
+
 ; RUN: llvm-pdbutil pretty -classes -class-definitions=layout \
 ; RUN:     -include-types=SimplePad %p/Inputs/SimplePaddingTest.pdb > %t
 
 ; RUN: FileCheck -input-file=%t %s -check-prefix=NO_PADDING
 ; RUN: FileCheck -input-file=%t %s -check-prefix=UNION
 ; RUN: FileCheck -input-file=%t %s -check-prefix=NESTED_UNION
 ; RUN: FileCheck -input-file=%t %s -check-prefix=PAD_FROM_FIELDS1
 ; RUN: FileCheck -input-file=%t %s -check-prefix=PAD_FROM_FIELDS2

test/tools/llvm-pdbdump/stripped.test

+; RUN: llvm-pdbutil dump -all %p/Inputs/Stripped.pdb > %t
+; RUN: FileCheck -input-file=%t %s
+
+; CHECK: Summary                           
+; CHECK-NEXT: ============================================================
+
+; CHECK: Streams                           
+; CHECK-NEXT: ============================================================
+
+; CHECK: Module Stats                        
+; CHECK-NEXT: ============================================================
+
+; CHECK: S_UDT Record Stats                     
+; CHECK-NEXT: ============================================================
+
+; CHECK: String Table                        
+; CHECK-NEXT: ============================================================
+
+; CHECK: Modules                           
+; CHECK-NEXT: ============================================================
+
+; CHECK: Files                            
+; CHECK-NEXT: ============================================================
+
+; CHECK: Lines                            
+; CHECK-NEXT: ============================================================
+
+; CHECK: Inlinee Lines                        
+; CHECK-NEXT: ============================================================
+
+; CHECK: Cross Module Imports                    
+; CHECK-NEXT: ============================================================
+
+; CHECK: Cross Module Exports                    
+; CHECK-NEXT: ============================================================
+
+; CHECK: Old FPO Data                        
+; CHECK-NEXT: ============================================================
+
+; CHECK: New FPO Data                        
+; CHECK-NEXT: ============================================================
+
+; CHECK: Types (TPI Stream)                     
+; CHECK-NEXT: ============================================================
+
+; CHECK: Types (IPI Stream)                     
+; CHECK-NEXT: ============================================================
+
+; CHECK: Global Symbols                       
+; CHECK-NEXT: ============================================================
+
+; CHECK: Public Symbols                       
+; CHECK-NEXT: ============================================================
+
+; CHECK: Symbols                           
+; CHECK-NEXT: ============================================================
+
+; CHECK: Section Headers                       
+; CHECK-NEXT: ============================================================
+
+; CHECK: Original Section Headers                  
+; CHECK-NEXT: ============================================================
+
+; CHECK: Section Contributions                    
+; CHECK-NEXT: ============================================================
+
+; CHECK: Section Map                         
+; CHECK-NEXT: ============================================================
+
+

test/tools/llvm-pdbdump/symbol-filters.test

+; REQUIRES: diasdk
+
 ; RUN: llvm-pdbutil pretty -globals -module-syms -sym-types=data %p/Inputs/FilterTest.pdb \
 ; RUN:    | FileCheck --check-prefix=ONLY_DATA %s
 
 ; RUN: llvm-pdbutil pretty -globals -module-syms -sym-types=thunks %p/Inputs/FilterTest.pdb \
 ; RUN:    | FileCheck --check-prefix=ONLY_THUNKS %s
 
 ; RUN: llvm-pdbutil pretty -globals -module-syms -sym-types=funcs %p/Inputs/FilterTest.pdb \
 ; RUN:    | FileCheck --check-prefix=ONLY_FUNCS %s

test/tools/llvm-pdbdump/type-qualifiers.test

+; REQUIRES: diasdk
+
 ; RUN: llvm-pdbutil pretty -all -class-recurse-depth=1 \
 ; RUN:   %p/Inputs/TypeQualifiersTest.pdb > %t
 ; RUN: FileCheck -input-file=%t %s -check-prefix=GLOBALS_FUNC
 ; RUN: FileCheck -input-file=%t %s -check-prefix=GLOBALS_DATA
 ; RUN: FileCheck -input-file=%t %s -check-prefix=QUALS
 
 ; GLOBALS_FUNC: ---GLOBALS---
 ; GLOBALS_FUNC-DAG: int __cdecl f(const volatile int* __restrict arg_crv)

test/tools/llvm-pdbdump/usingnamespace.test

+; REQUIRES: diasdk
+
 ; RUN: llvm-pdbutil pretty -module-syms %p/Inputs/UsingNamespaceTest.pdb > %t
 ; RUN: FileCheck -input-file=%t %s
 
 ; CHECK: ---SYMBOLS---
 ; CHECK-NEXT: {{.*}}UsingNamespaceTest.obj
 ; CHECK-DAG: using namespace NS

tools/llvm-pdbutil/DumpOutputStyle.cpp

Error DumpOutputStyle::dumpFileSummary() {
   }
 
   return Error::success();
 }
 
 static StatCollection getSymbolStats(const SymbolGroup &SG,
                                      StatCollection &CumulativeStats) {
   StatCollection Stats;
-  if (SG.getFile().isPdb()) {
+  if (SG.getFile().isPdb() && SG.hasDebugStream()) {
     // For PDB files, all symbols are packed into one stream.
     for (const auto &S : SG.getPdbModuleStream().symbols(nullptr)) {
       Stats.update(S.kind(), S.length());
       CumulativeStats.update(S.kind(), S.length());
     }
     return Stats;
   }
 
if (DumpExtras) {
     P.NewLine();
     auto IndexOffsets = Stream.getTypeIndexOffsets();
     P.formatLine("Type Index Offsets:");
     for (const auto &IO : IndexOffsets) {
       AutoIndent Indent2(P);
       P.formatLine("TI: {0}, Offset: {1}", IO.Type, fmtle(IO.Offset));
     }
 
+    if (getPdb().hasPDBStringTable()) {
       P.NewLine();
       P.formatLine("Hash Adjusters:");
       auto &Adjusters = Stream.getHashAdjusters();
       auto &Strings = Err(getPdb().getStringTable());
       for (const auto &A : Adjusters) {
         AutoIndent Indent2(P);
         auto ExpectedStr = Strings.getStringForID(A.first);
         TypeIndex TI(A.second);
         if (ExpectedStr)
           P.formatLine("`{0}` -> {1}", *ExpectedStr, TI);
         else {
           P.formatLine("unknown str id ({0}) -> {1}", A.first, TI);
           consumeError(ExpectedStr.takeError());
         }
       }
     }
+  }
   return Error::success();
 }
 
 Error DumpOutputStyle::dumpModuleSymsForObj() {
   printHeader(P, "Symbols");
 
   AutoIndent Indent(P);
 

tools/llvm-pdbutil/InputFile.h

public:
   codeview::DebugSubsectionArray getDebugSubsections() const {
     return Subsections;
   }
   const ModuleDebugStreamRef &getPdbModuleStream() const;
 
   const InputFile &getFile() const { return *File; }
   InputFile &getFile() { return *File; }
 
+  bool hasDebugStream() const { return DebugStream != nullptr; }
+
 private:
   void initializeForPdb(uint32_t Modi);
   void updatePdbModi(uint32_t Modi);
   void updateDebugS(const codeview::DebugSubsectionArray &SS);
 
   void rebuildChecksumMap();
   InputFile *File = nullptr;
   StringRef Name;

tools/llvm-pdbutil/InputFile.cpp

switch (Kind) {
     RETURN_CASE(FileChecksumKind, None, "None");
     RETURN_CASE(FileChecksumKind, MD5, "MD5");
     RETURN_CASE(FileChecksumKind, SHA1, "SHA-1");
     RETURN_CASE(FileChecksumKind, SHA256, "SHA-256");
   }
   return formatUnknownEnum(Kind);
 }
 
-static const DebugStringTableSubsectionRef &extractStringTable(PDBFile &File) {
-  return cantFail(File.getStringTable()).getStringTable();
-}
-
 template <typename... Args>
 static void formatInternal(LinePrinter &Printer, bool Append, Args &&... args) {
   if (Append)
     Printer.format(std::forward<Args>(args)...);
   else
     Printer.formatLine(std::forward<Args>(args)...);
 }
 

 
 void SymbolGroup::updatePdbModi(uint32_t Modi) { initializeForPdb(Modi); }
 
 void SymbolGroup::initializeForPdb(uint32_t Modi) {
   assert(File && File->isPdb());
 
   // PDB always uses the same string table, but each module has its own
   // checksums.  So we only set the strings if they're not already set.
-  if (!SC.hasStrings())
-    SC.setStrings(extractStringTable(File->pdb()));
+  if (!SC.hasStrings()) {
+    auto StringTable = File->pdb().getStringTable();
+    if (StringTable)
+      SC.setStrings(StringTable->getStringTable());
+    else
+      consumeError(StringTable.takeError());
+  }
 
   SC.resetChecksums();
   auto MDS = getModuleDebugStream(File->pdb(), Name, Modi);
   if (!MDS) {
     consumeError(MDS.takeError());
     return;
   }
 
   DebugStream = std::make_shared<ModuleDebugStreamRef>(std::move(*MDS));
+  assert(DebugStream);
   Subsections = DebugStream->getSubsectionsArray();
   SC.initialize(Subsections);
   rebuildChecksumMap();
 }
 
 void SymbolGroup::rebuildChecksumMap() {
   if (!SC.hasChecksums())
     return;