[PDB][NativeSession] Clean up some things in NativeSession.

[akhuang]

Summary

-Use the actual sect/offset to keep track of symbols in the cache so they don't get created multiple times with different addresses.
-Remove getSymTag from PDBFunctionSymbol/PDBPublicSymbol because it's already implemented in the base class
-Merge the symbolizer test files for DIA and native, since the tests are the same.
-Implement getCompilandId for NativeLineNumber

[akhuang]

more cleanup

[hans]

I don't know this code, but on the surface it seems reasonable to me. Probably good if someone else takes a look too though.

[amccarth]

LGTM.

The naming improvements really help. And it's nice to see the symbol tag set correctly (rather than generically only to be overridden). Nice work.

llvm/include/llvm/DebugInfo/PDB/Native/NativeFunctionSymbol.h

public:
   ~NativeFunctionSymbol() override;
 
   void dump(raw_ostream &OS, int Indent, PdbSymbolIdField ShowIdFields,
             PdbSymbolIdField RecurseIdFields) const override;
 
   uint32_t getAddressOffset() const override;
   uint32_t getAddressSection() const override;
   std::string getName() const override;
-  PDB_SymType getSymTag() const override;
   uint64_t getLength() const override;
   uint32_t getRelativeVirtualAddress() const override;
   uint64_t getVirtualAddress() const override;
 
 protected:
   const codeview::ProcSym Sym;
 };
 
 } // namespace pdb
 } // namespace llvm
 
 #endif // LLVM_DEBUGINFO_PDB_NATIVE_NATIVEFUNCTIONSYMBOL_H

llvm/include/llvm/DebugInfo/PDB/Native/NativeLineNumber.h

 namespace llvm {
 namespace pdb {
 class NativeLineNumber : public IPDBLineNumber {
 public:
   explicit NativeLineNumber(const NativeSession &Session,
                             const codeview::LineInfo Line,
                             uint32_t ColumnNumber, uint32_t Length,
                             uint32_t Section, uint32_t Offset,
-                            uint32_t SrcFileId);
+                            uint32_t SrcFileId, uint32_t CompilandId);
 
   uint32_t getLineNumber() const override;
   uint32_t getLineNumberEnd() const override;
   uint32_t getColumnNumber() const override;
   uint32_t getColumnNumberEnd() const override;
   uint32_t getAddressSection() const override;
   uint32_t getAddressOffset() const override;
   uint32_t getRelativeVirtualAddress() const override;
   uint64_t getVirtualAddress() const override;
   uint32_t getLength() const override;
   uint32_t getSourceFileId() const override;
   uint32_t getCompilandId() const override;
   bool isStatement() const override;
 
 private:
   const NativeSession &Session;
   const codeview::LineInfo Line;
   uint32_t ColumnNumber;
   uint32_t Section;
   uint32_t Offset;
   uint32_t Length;
   uint32_t SrcFileId;
+  uint32_t CompilandId;
 };
 } // namespace pdb
 } // namespace llvm
 #endif

llvm/include/llvm/DebugInfo/PDB/Native/NativePublicSymbol.h

public:
   ~NativePublicSymbol() override;
 
   void dump(raw_ostream &OS, int Indent, PdbSymbolIdField ShowIdFields,
             PdbSymbolIdField RecurseIdFields) const override;
 
   uint32_t getAddressOffset() const override;
   uint32_t getAddressSection() const override;
   std::string getName() const override;
-  PDB_SymType getSymTag() const override;
   uint32_t getRelativeVirtualAddress() const override;
   uint64_t getVirtualAddress() const override;
 
 protected:
   const codeview::PublicSym32 Sym;
 };
 
 } // namespace pdb
 } // namespace llvm
 
 #endif // LLVM_DEBUGINFO_PDB_NATIVE_NATIVEPUBLICSYMBOL_H

llvm/include/llvm/DebugInfo/PDB/Native/SymbolCache.h

class SymbolCache {
   /// List of source files, indexed by unique source file index.
   mutable std::vector<std::unique_ptr<NativeSourceFile>> SourceFiles;
   mutable DenseMap<uint32_t, SymIndexId> FileNameOffsetToId;
 
   /// Map from global symbol offset to SymIndexId.
   DenseMap<uint32_t, SymIndexId> GlobalOffsetToSymbolId;
 
   /// Map from segment and code offset to SymIndexId.
-  DenseMap<std::pair<uint32_t, uint32_t>, SymIndexId> AddressToFunctionSymId;
+  DenseMap<std::pair<uint32_t, uint32_t>, SymIndexId> AddressToSymbolId;
   DenseMap<std::pair<uint32_t, uint32_t>, SymIndexId> AddressToPublicSymId;
 
   /// Map from virtual address to module index.
   using IMap =
       IntervalMap<uint64_t, uint16_t, 8, IntervalMapHalfOpenInfo<uint64_t>>;
   IMap::Allocator IMapAllocator;
   IMap AddrToModuleIndex;
 

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

 
 using namespace llvm;
 using namespace llvm::codeview;
 using namespace llvm::pdb;
 
 NativeFunctionSymbol::NativeFunctionSymbol(NativeSession &Session,
                                            SymIndexId Id,
                                            const codeview::ProcSym &Sym)
-    : NativeRawSymbol(Session, PDB_SymType::Data, Id), Sym(Sym) {}
+    : NativeRawSymbol(Session, PDB_SymType::Function, Id), Sym(Sym) {}
 
 NativeFunctionSymbol::~NativeFunctionSymbol() {}
 
 void NativeFunctionSymbol::dump(raw_ostream &OS, int Indent,
                                 PdbSymbolIdField ShowIdFields,
                                 PdbSymbolIdField RecurseIdFields) const {
   NativeRawSymbol::dump(OS, Indent, ShowIdFields, RecurseIdFields);
   dumpSymbolField(OS, "name", getName(), Indent);
   dumpSymbolField(OS, "length", getLength(), Indent);
   dumpSymbolField(OS, "offset", getAddressOffset(), Indent);
   dumpSymbolField(OS, "section", getAddressSection(), Indent);
 }
 
 uint32_t NativeFunctionSymbol::getAddressOffset() const {
   return Sym.CodeOffset;
 }
 
 uint32_t NativeFunctionSymbol::getAddressSection() const { return Sym.Segment; }
 std::string NativeFunctionSymbol::getName() const {
   return std::string(Sym.Name);
 }
 
-PDB_SymType NativeFunctionSymbol::getSymTag() const {
-  return PDB_SymType::Function;
-}
-
 uint64_t NativeFunctionSymbol::getLength() const { return Sym.CodeSize; }
 
 uint32_t NativeFunctionSymbol::getRelativeVirtualAddress() const {
   return Session.getRVAFromSectOffset(Sym.Segment, Sym.CodeOffset);
 }
 
 uint64_t NativeFunctionSymbol::getVirtualAddress() const {
   return Session.getVAFromSectOffset(Sym.Segment, Sym.CodeOffset);
 }

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

 
 using namespace llvm;
 using namespace llvm::pdb;
 
 NativeLineNumber::NativeLineNumber(const NativeSession &Session,
                                    const codeview::LineInfo Line,
                                    uint32_t ColumnNumber, uint32_t Section,
                                    uint32_t Offset, uint32_t Length,
-                                   uint32_t SrcFileId)
+                                   uint32_t SrcFileId, uint32_t CompilandId)
     : Session(Session), Line(Line), ColumnNumber(ColumnNumber),
-      Section(Section), Offset(Offset), Length(Length), SrcFileId(SrcFileId) {}
+      Section(Section), Offset(Offset), Length(Length), SrcFileId(SrcFileId),
+      CompilandId(CompilandId) {}
 
 uint32_t NativeLineNumber::getLineNumber() const { return Line.getStartLine(); }
 
 uint32_t NativeLineNumber::getLineNumberEnd() const {
   return Line.getEndLine();
 }
 
 uint32_t NativeLineNumber::getColumnNumber() const { return ColumnNumber; }

 uint64_t NativeLineNumber::getVirtualAddress() const {
   return Session.getVAFromSectOffset(Section, Offset);
 }
 
 uint32_t NativeLineNumber::getLength() const { return Length; }
 
 uint32_t NativeLineNumber::getSourceFileId() const { return SrcFileId; }
 
-uint32_t NativeLineNumber::getCompilandId() const { return 0; }
+uint32_t NativeLineNumber::getCompilandId() const { return CompilandId; }
 
 bool NativeLineNumber::isStatement() const { return Line.isStatement(); }

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

 #include "llvm/DebugInfo/PDB/Native/NativeTypeEnum.h"
 
 using namespace llvm;
 using namespace llvm::codeview;
 using namespace llvm::pdb;
 
 NativePublicSymbol::NativePublicSymbol(NativeSession &Session, SymIndexId Id,
                                        const codeview::PublicSym32 &Sym)
-    : NativeRawSymbol(Session, PDB_SymType::Data, Id), Sym(Sym) {}
+    : NativeRawSymbol(Session, PDB_SymType::PublicSymbol, Id), Sym(Sym) {}
 
 NativePublicSymbol::~NativePublicSymbol() {}
 
 void NativePublicSymbol::dump(raw_ostream &OS, int Indent,
                               PdbSymbolIdField ShowIdFields,
                               PdbSymbolIdField RecurseIdFields) const {
   NativeRawSymbol::dump(OS, Indent, ShowIdFields, RecurseIdFields);
   dumpSymbolField(OS, "name", getName(), Indent);
   dumpSymbolField(OS, "offset", getAddressOffset(), Indent);
   dumpSymbolField(OS, "section", getAddressSection(), Indent);
 }
 
 uint32_t NativePublicSymbol::getAddressOffset() const { return Sym.Offset; }
 
 uint32_t NativePublicSymbol::getAddressSection() const { return Sym.Segment; }
 
 std::string NativePublicSymbol::getName() const {
   return std::string(Sym.Name);
 }
 
-PDB_SymType NativePublicSymbol::getSymTag() const {
-  return PDB_SymType::PublicSymbol;
-}
-
 uint32_t NativePublicSymbol::getRelativeVirtualAddress() const {
   return Session.getRVAFromSectOffset(Sym.Segment, Sym.Offset);
 }
 
 uint64_t NativePublicSymbol::getVirtualAddress() const {
   return Session.getVAFromSectOffset(Sym.Segment, Sym.Offset);
 }

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

 std::unique_ptr<IPDBEnumLineNumbers>
 NativeSession::findLineNumbersByAddress(uint64_t Address,
                                         uint32_t Length) const {
   return Cache.findLineNumbersByVA(Address, Length);
 }
 
 std::unique_ptr<IPDBEnumLineNumbers>
 NativeSession::findLineNumbersByRVA(uint32_t RVA, uint32_t Length) const {
-  return findLineNumbersByAddress(getLoadAddress() + RVA, Length);
+  return Cache.findLineNumbersByVA(getLoadAddress() + RVA, Length);
 }
 
 std::unique_ptr<IPDBEnumLineNumbers>
 NativeSession::findLineNumbersBySectOffset(uint32_t Section, uint32_t Offset,
                                            uint32_t Length) const {
   uint64_t VA = getVAFromSectOffset(Section, Offset);
-  return findLineNumbersByAddress(VA, Length);
+  return Cache.findLineNumbersByVA(VA, Length);
 }
 
 std::unique_ptr<IPDBEnumSourceFiles>
 NativeSession::findSourceFiles(const PDBSymbolCompiland *Compiland,
                                StringRef Pattern,
                                PDB_NameSearchFlags Flags) const {
   return nullptr;
 }

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

SymbolCache::findSymbolBySectOffset(uint32_t Sect, uint32_t Offset,
   if (AddrToModuleIndex.empty())
     parseSectionContribs();
 
   switch (Type) {
   case PDB_SymType::Function:
     return findFunctionSymbolBySectOffset(Sect, Offset);
   case PDB_SymType::PublicSymbol:
     return findPublicSymbolBySectOffset(Sect, Offset);
+  case PDB_SymType::Compiland: {
+    Optional<uint16_t> Modi =
+        getModuleIndexForAddr(Session.getVAFromSectOffset(Sect, Offset));
+    if (!Modi)
+      return nullptr;
+    return getOrCreateCompiland(*Modi);
+  }
   case PDB_SymType::None: {
-    // FIXME: Implement for PDB_SymType::Data.
+    // FIXME: Implement for PDB_SymType::Data. The symbolizer calls this but
+    // only uses it to find the symbol length.
     if (auto Sym = findFunctionSymbolBySectOffset(Sect, Offset))
       return Sym;
     return nullptr;
   }
   default:
     return nullptr;
   }
 }
 
 std::unique_ptr<PDBSymbol>
 SymbolCache::findFunctionSymbolBySectOffset(uint32_t Sect, uint32_t Offset) {
-  auto Iter = AddressToFunctionSymId.find({Sect, Offset});
-  if (Iter != AddressToFunctionSymId.end())
+  auto Iter = AddressToSymbolId.find({Sect, Offset});
+  if (Iter != AddressToSymbolId.end())
     return getSymbolById(Iter->second);
 
   if (!Dbi)
     return nullptr;
 
   auto Modi = getModuleIndexForAddr(Session.getVAFromSectOffset(Sect, Offset));
   if (!Modi)
     return nullptr;
 
   auto ExpectedModS = getModuleDebugStream(*Modi);
   if (!ExpectedModS) {
     consumeError(ExpectedModS.takeError());
     return nullptr;
   }
   CVSymbolArray Syms = ExpectedModS->getSymbolArray();
 
   // Search for the symbol in this module.
   for (auto I = Syms.begin(), E = Syms.end(); I != E; ++I) {
     if (I->kind() != S_LPROC32 && I->kind() != S_GPROC32)
       continue;
     auto PS = cantFail(SymbolDeserializer::deserializeAs<ProcSym>(*I));
     if (Sect == PS.Segment && Offset >= PS.CodeOffset &&
         Offset < PS.CodeOffset + PS.CodeSize) {
+      // Check if the symbol is already cached.
+      auto Found = AddressToSymbolId.find({PS.Segment, PS.CodeOffset});
+      if (Found != AddressToSymbolId.end())
+        return getSymbolById(Found->second);
+
+      // Otherwise, create a new symbol.
       SymIndexId Id = createSymbol<NativeFunctionSymbol>(PS);
-      AddressToFunctionSymId.insert({{Sect, Offset}, Id});
+      AddressToSymbolId.insert({{PS.Segment, PS.CodeOffset}, Id});
       return getSymbolById(Id);
     }
 
     // Jump to the end of this ProcSym.
     I = Syms.at(PS.End);
   }
   return nullptr;
 }
if (It == AddrMap.begin())
     return nullptr;
   --It;
 
   Expected<CVSymbol> Sym = readSymbolFromStream(SymStream, *It);
   if (!Sym) {
     consumeError(Sym.takeError());
     return nullptr;
   }
+
+  // Check if the symbol is already cached.
   auto PS = cantFail(SymbolDeserializer::deserializeAs<PublicSym32>(Sym.get()));
+  auto Found = AddressToPublicSymId.find({PS.Segment, PS.Offset});
+  if (Found != AddressToPublicSymId.end())
+    return getSymbolById(Found->second);
+
+  // Otherwise, create a new symbol.
   SymIndexId Id = createSymbol<NativePublicSymbol>(PS);
-  AddressToPublicSymId.insert({{Sect, Offset}, Id});
+  AddressToPublicSymId.insert({{PS.Segment, PS.Offset}, Id});
   return getSymbolById(Id);
 }
 
 std::vector<SymbolCache::LineTableEntry>
 SymbolCache::findLineTable(uint16_t Modi) const {
   // Check if this module has already been added.
   auto LineTableIter = LineTable.find(Modi);
   if (LineTableIter != LineTable.end())
while (Modi <= *EndModi) {
 
     uint32_t LineSect, LineOff;
     Session.addressForVA(LineIter->Addr, LineSect, LineOff);
     uint32_t LineLength = std::next(LineIter)->Addr - LineIter->Addr;
     auto ChecksumIter =
         ExpectedChecksums->getArray().at(LineIter->FileNameIndex);
     uint32_t SrcFileId = getOrCreateSourceFile(*ChecksumIter);
     NativeLineNumber LineNum(Session, LineIter->Line, LineIter->ColumnNumber,
-                             LineSect, LineOff, LineLength, SrcFileId);
+                             LineSect, LineOff, LineLength, SrcFileId, Modi);
     LineNumbers.push_back(LineNum);
     ++LineIter;
   }
   return std::make_unique<NativeEnumLineNumbers>(std::move(LineNumbers));
 }
 
 std::unique_ptr<PDBSymbolCompiland>
 SymbolCache::getOrCreateCompiland(uint32_t Index) {

llvm/test/tools/llvm-symbolizer/pdb/pdb-columns.test

 RUN: echo 0x140006BA0 > %t.input
 RUN: echo 0x140006C00 >> %t.input
 RUN: echo 0x140006BB0 >> %t.input
 RUN: echo 0x140006C10 >> %t.input
 RUN: echo 0x140006C20 >> %t.input
 RUN: echo 0x140006C30 >> %t.input
 RUN: echo 0x140006C40 >> %t.input
 RUN: echo 0x140006C70 >> %t.input
+RUN:    llvm-symbolizer -obj="%p/Inputs/test-columns.exe" < %t.input \
+RUN:    | FileCheck %s
 RUN:    llvm-symbolizer -obj="%p/Inputs/test-columns.exe" -use-native-pdb-reader < %t.input \
 RUN:    | FileCheck %s
 
 This tests that the symbolizer outputs column info when it is present in the pdb.
 
 CHECK: foo(void)
 CHECK-NEXT: test.cpp:11:1
 CHECK: {{^private_symbol$}}

llvm/test/tools/llvm-symbolizer/pdb/pdb-native.test

-RUN: echo 0x401380 > %t.input
-RUN: echo 0x401390 >> %t.input
-RUN: echo 0x4013A0 >> %t.input
-RUN: echo 0x4013C0 >> %t.input
-RUN: echo 0x4013D0 >> %t.input
-RUN: echo 0x4013E0 >> %t.input
-RUN: echo 0x4013F0 >> %t.input
-RUN: echo 0x401420 >> %t.input
-RUN: llvm-symbolizer -obj="%p/Inputs/test.exe" -use-native-pdb-reader < %t.input \
-RUN:    | FileCheck %s
-RUN: llvm-symbolizer -obj="%p/Inputs/test.exe" -demangle=false -use-native-pdb-reader < %t.input \
-RUN:    | FileCheck %s --check-prefix=CHECK-NO-DEMANGLE
-
-Subtract ImageBase from all the offsets and run the test again with
---relative-address.
-
-RUN: %python -c 'import sys;print("\n".join([hex(int(x, 16) - 0x400000) for x in sys.stdin]))' < %t.input \
-RUN:    | llvm-symbolizer -obj="%p/Inputs/test.exe" -use-native-pdb-reader -demangle=false --relative-address \
-RUN:    | FileCheck %s --check-prefix=CHECK-NO-DEMANGLE
-
-CHECK: foo(void)
-CHECK-NEXT: test.cpp:10
-CHECK: {{^private_symbol$}}
-CHECK-NEXT: test.cpp:13:0
-CHECK: {{^main}}
-CHECK-NEXT: test.cpp:16:0
-CHECK: {{^foo_cdecl$}}
-CHECK: {{^foo_stdcall$}}
-CHECK: {{^foo_fastcall$}}
-CHECK: {{^foo_vectorcall$}}
-CHECK: NS::Foo::bar(void)
-CHECK-NEXT: test.cpp:6:0
-
-CHECK-NO-DEMANGLE: ?foo@@YAXXZ
-CHECK-NO-DEMANGLE-NEXT: test.cpp:10
-CHECK-NO-DEMANGLE: private_symbol
-CHECK-NO-DEMANGLE-NEXT: test.cpp:13
-CHECK-NO-DEMANGLE: _main
-CHECK-NO-DEMANGLE-NEXT: test.cpp:16
-CHECK-NO-DEMANGLE: _foo_cdecl
-CHECK-NO-DEMANGLE: _foo_stdcall@0
-CHECK-NO-DEMANGLE: @foo_fastcall@0
-CHECK-NO-DEMANGLE: foo_vectorcall@@0
-CHECK-NO-DEMANGLE: ?bar@Foo@NS@@QAEXXZ
-CHECK-NO-DEMANGLE-NEXT: test.cpp:6

llvm/test/tools/llvm-symbolizer/pdb/pdb.test

 RUN: echo 0x401380 > %t.input
 RUN: echo 0x401390 >> %t.input
 RUN: echo 0x4013A0 >> %t.input
 RUN: echo 0x4013C0 >> %t.input
 RUN: echo 0x4013D0 >> %t.input
 RUN: echo 0x4013E0 >> %t.input
 RUN: echo 0x4013F0 >> %t.input
 RUN: echo 0x401420 >> %t.input
 RUN: llvm-symbolizer -obj="%p/Inputs/test.exe" < %t.input \
 RUN:    | FileCheck %s
 RUN: llvm-symbolizer -obj="%p/Inputs/test.exe" -demangle=false < %t.input \
 RUN:    | FileCheck %s --check-prefix=CHECK-NO-DEMANGLE
 
+Test with native pdb reader.
+RUN: llvm-symbolizer -use-native-pdb-reader -obj="%p/Inputs/test.exe" < %t.input \
+RUN:    | FileCheck %s
+RUN: llvm-symbolizer -use-native-pdb-reader -obj="%p/Inputs/test.exe" -demangle=false < %t.input \
+RUN:    | FileCheck %s --check-prefix=CHECK-NO-DEMANGLE
+
 Subtract ImageBase from all the offsets and run the test again with
 --relative-address.
 
 RUN: %python -c 'import sys;print("\n".join([hex(int(x, 16) - 0x400000) for x in sys.stdin]))' < %t.input \
 RUN:    | llvm-symbolizer -obj="%p/Inputs/test.exe" -demangle=false --relative-address \
 RUN:    | FileCheck %s --check-prefix=CHECK-NO-DEMANGLE
 
+RUN: %python -c 'import sys;print("\n".join([hex(int(x, 16) - 0x400000) for x in sys.stdin]))' < %t.input \
+RUN:    | llvm-symbolizer -use-native-pdb-reader -obj="%p/Inputs/test.exe" -demangle=false --relative-address \
+RUN:    | FileCheck %s --check-prefix=CHECK-NO-DEMANGLE
+
 CHECK: foo(void)
 CHECK-NEXT: test.cpp:10
 CHECK: {{^private_symbol$}}
 CHECK-NEXT: test.cpp:13:0
 CHECK: {{^main}}
 CHECK-NEXT: test.cpp:16:0
 CHECK: {{^foo_cdecl$}}
 CHECK: {{^foo_stdcall$}}