[NativePDB] Read inc-link thunks from /* LINKER */ module and create symbols

[lanza]

Summary

The "/* LINKER */" module contains symbols for thunks used for incremental
linking. A callq to a symbol gets statically linked to a thunk that is just a
jmp to the actual target symbol. This information is contained in this "/*
LINKER */" module.

In order for ThreadPlans to handle stepping through these thunks there has to be
something pertaining to it returned from ResolveSymbolContext. So while looking
up symbols in the PDB by VA we can create a Symbol from the PDB Symbol type
which was previously ignored and then the ThreadPlan can properly handle this
step.

[lanza]

Fix a test

[aleksandr.urakov]

source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp

982–1006

I think that this stuff should be in AddSymbols function. After changing a symtab we should call CalculateSymbolSizes and Finalize on it (you can take a look on the implementation in the old PDB plugin).

992

May be it would be better to pass nullptr here to avoid possible name conflicts with e.g. functions with the same name?

1147

I'm pretty sure that using std::unique_ptr after releasing it is not a good idea. There will be a null pointer dereference here.

source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

209–218

Why is this necessary? The change looks unrelated to the old PDB plugin. I think that it would be better to make SymbolFilePDBTests work over the native PDB plugin instead.

[lanza]

source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp

992

Is nullptr valid? It attempts to use the string to print numerous labels and just yields a weirdly empty print out. I could possibly just call ResolveSymbolContext on the tramp.TargetOffset + tramp.TargetSection and use "ILT'" + function.GetName().

1147

Yup, absent minded copy+paste.

source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

209–218

This is technically wrong. It's not a "dummy" compilation unit, it's crucial for debugging incrementally linked code. SymbolFilePDB flat out fails when it hits incrementally linked code. If you're using clang+lld it's not a problem but msvc compiled code will fail. I figured I'd remove this chun of code here while removing the analogous stuff from the Native version.

lit/SymbolFile/NativePDB/local-variables.cpp

 // CHECK-NEXT: Process {{.*}} exited with status = 18 (0x00000012)
 
 // CHECK:      (lldb) target modules dump ast
 // CHECK-NEXT: Dumping clang ast for {{.*}} modules.
 // CHECK-NEXT: TranslationUnitDecl
 // CHECK-NEXT: |-FunctionDecl {{.*}} main 'int (int, char **)'
 // CHECK-NEXT: | |-ParmVarDecl {{.*}} argc 'int'
 // CHECK-NEXT: | `-ParmVarDecl {{.*}} argv 'char **'
+// CHECK-NEXT: |-FunctionDecl {{.*}} __scrt_common_main_seh 'int ()' static
 // CHECK-NEXT: `-FunctionDecl {{.*}} Function 'int (int, char)'
 // CHECK-NEXT:   |-ParmVarDecl {{.*}} Param1 'int'
 // CHECK-NEXT:   `-ParmVarDecl {{.*}} Param2 'char'

source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp

if (ts)
     ts->SetSymbolFile(this);
 
   m_ast = llvm::make_unique<PdbAstBuilder>(*m_obj_file, *m_index);
 }
 
 uint32_t SymbolFileNativePDB::GetNumCompileUnits() {
   const DbiModuleList &modules = m_index->dbi().modules();
   uint32_t count = modules.getModuleCount();
-  if (count == 0)
-    return count;
-
-  // The linker can inject an additional "dummy" compilation unit into the
-  // PDB. Ignore this special compile unit for our purposes, if it is there.
-  // It is always the last one.
-  DbiModuleDescriptor last = modules.getModuleDescriptor(count - 1);
-  if (last.getModuleName() == "* Linker *")
-    --count;
   return count;
 }
 
 Block &SymbolFileNativePDB::CreateBlock(PdbCompilandSymId block_id) {
   CompilandIndexItem *cii = m_index->compilands().GetCompiland(block_id.modi);
   CVSymbol sym = cii->m_debug_stream.readSymbolAtOffset(block_id.offset);
 
   if (sym.kind() == S_GPROC32 || sym.kind() == S_LPROC32) {
uint32_t SymbolFileNativePDB::ResolveSymbolContext(
     const Address &addr, SymbolContextItem resolve_scope, SymbolContext &sc) {
   uint32_t resolved_flags = 0;
   lldb::addr_t file_addr = addr.GetFileAddress();
 
   if (NeedsResolvedCompileUnit(resolve_scope)) {
     llvm::Optional<uint16_t> modi = m_index->GetModuleIndexForVa(file_addr);
     if (!modi)
       return 0;
-    CompilandIndexItem *cci = m_index->compilands().GetCompiland(*modi);
-    if (!cci)
-      return 0;
+    CompilandIndexItem cci = m_index->compilands().GetOrCreateCompiland(*modi);
 
-    sc.comp_unit = GetOrCreateCompileUnit(*cci).get();
+    sc.comp_unit = GetOrCreateCompileUnit(cci).get();
     resolved_flags |= eSymbolContextCompUnit;
   }
 
   if (resolve_scope & eSymbolContextFunction ||
       resolve_scope & eSymbolContextBlock) {
     lldbassert(sc.comp_unit);
     std::vector<SymbolAndUid> matches = m_index->FindSymbolsByVa(file_addr);
     // Search the matches in reverse.  This way if there are multiple matches
     // (for example we are 3 levels deep in a nested scope) it will find the
     // innermost one first.
     for (const auto &match : llvm::reverse(matches)) {
       if (match.uid.kind() != PdbSymUidKind::CompilandSym)
         continue;
 
       PdbCompilandSymId csid = match.uid.asCompilandSym();
       CVSymbol cvs = m_index->ReadSymbolRecord(csid);
       PDB_SymType type = CVSymToPDBSym(cvs.kind());
-      if (type != PDB_SymType::Function && type != PDB_SymType::Block)
+      if (type != PDB_SymType::Function && type != PDB_SymType::Block &&
+          type != PDB_SymType::Thunk)
         continue;
+
+      if (type == PDB_SymType::Thunk) {
+        auto symtab = m_obj_file->GetSymtab();
+        if (!symtab)
+          continue;
+
+        if (auto symbol = symtab->FindSymbolContainingFileAddress(file_addr)) {
+          sc.symbol = symbol;
+          resolved_flags |= eSymbolContextSymbol;
+        } else {
+          TrampolineSym tramp(static_cast<SymbolRecordKind>(cvs.kind()));
+          cantFail(
+              SymbolDeserializer::deserializeAs<TrampolineSym>(cvs, tramp));
+
+          auto section_list = m_obj_file->GetSectionList();
+          if (!section_list)
+            continue;
+          auto section = section_list->FindSectionByID(tramp.ThunkSection);
+
+          symtab->AddSymbol(Symbol(match.uid.toOpaqueId(), // symID
+                                   "IncrementalLinkThunk", // name
+                                   true,                   // name_is_mangled
+                                   eSymbolTypeTrampoline,
+                                   true,              // external
+                                   false,             // is_debug
+                                   true,              // is_trampoline
+                                   false,             // is_artificial
+                                   section,           // section_sp
+                                   tramp.ThunkOffset, // value
+                                   tramp.Size,        // size
+                                   tramp.Size != 0,   // size_is_valid
+                                   false, // contains_linker_annotations
+                                   0      // flags
+                                   ));
+        }
+      } else {
         if (type == PDB_SymType::Function) {
           sc.function = GetOrCreateFunction(csid, *sc.comp_unit).get();
           sc.block = sc.GetFunctionBlock();
         }
 
         if (type == PDB_SymType::Block) {
           sc.block = &GetOrCreateBlock(csid);
           sc.function = sc.block->CalculateSymbolContextFunction();
         }
         resolved_flags |= eSymbolContextFunction;
         resolved_flags |= eSymbolContextBlock;
+      }
       break;
     }
   }
 
   if (resolve_scope & eSymbolContextLineEntry) {
     lldbassert(sc.comp_unit);
     if (auto *line_table = sc.comp_unit->GetLineTable()) {
       if (line_table->FindLineEntryByAddress(addr, sc.line_entry))
         resolved_flags |= eSymbolContextLineEntry;
for (const LineColumnEntry &group : lines) {
                                   file_index, *lfh, entry);
       }
       LineInfo last_line(group.LineNumbers.back().Flags);
       TerminateLineSequence(*line_table, *lfh, virtual_addr, file_index,
                             last_line.getEndLine(), std::move(sequence));
     }
   }
 
-  if (line_table->GetSize() == 0)
-    return false;
-
   comp_unit.SetLineTable(line_table.release());
-  return true;
+  return line_table->GetSize() > 0;
 }
 
 bool SymbolFileNativePDB::ParseDebugMacros(CompileUnit &comp_unit) {
   // PDB doesn't contain information about macros
   return false;
 }
 
 bool SymbolFileNativePDB::ParseSupportFiles(CompileUnit &comp_unit,

source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

if (m_cached_compile_unit_count == 0) {
     // *.dll. For example, a compiland with name `Import:KERNEL32.dll` could be
     // found as a child of the global scope (PDB executable). Usually, such
     // compilands contain `thunk` symbols in which we are not interested for
     // now. However we still count them in the compiland list. If we perform
     // any compiland related activity, like finding symbols through
     // llvm::pdb::IPDBSession methods, such compilands will all be searched
     // automatically no matter whether we include them or not.
     m_cached_compile_unit_count = compilands->getChildCount();
-
-    // The linker can inject an additional "dummy" compilation unit into the
-    // PDB. Ignore this special compile unit for our purposes, if it is there.
-    // It is always the last one.
-    auto last_compiland_up =
-        compilands->getChildAtIndex(m_cached_compile_unit_count - 1);
-    lldbassert(last_compiland_up.get());
-    std::string name = last_compiland_up->getName();
-    if (name == "* Linker *")
-      --m_cached_compile_unit_count;
   }
   return m_cached_compile_unit_count;
 }
 
 void SymbolFilePDB::GetCompileUnitIndex(
     const llvm::pdb::PDBSymbolCompiland &pdb_compiland, uint32_t &index) {
   auto results_up = m_global_scope_up->findAllChildren<PDBSymbolCompiland>();
   if (!results_up)

unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp

TEST_F(SymbolFilePDBTests, TestLineTablesMatchAll) {
 
   SymbolVendor *plugin = module->GetSymbolVendor();
   SymbolFile *symfile = plugin->GetSymbolFile();
 
   FileSpec source_file("test-pdb.cpp");
   FileSpec header1("test-pdb.h");
   FileSpec header2("test-pdb-nested.h");
   uint32_t cus = symfile->GetNumCompileUnits();
-  EXPECT_EQ(2u, cus);
+  EXPECT_EQ(3u, cus);
 
   SymbolContextList sc_list;
   lldb::SymbolContextItem scope =
       lldb::eSymbolContextCompUnit | lldb::eSymbolContextLineEntry;
 
   uint32_t count =
       symfile->ResolveSymbolContext(source_file, 0, true, scope, sc_list);
   EXPECT_EQ(1u, count);
TEST_F(SymbolFilePDBTests, TestLineTablesMatchSpecific) {
 
   SymbolVendor *plugin = module->GetSymbolVendor();
   SymbolFile *symfile = plugin->GetSymbolFile();
 
   FileSpec source_file("test-pdb.cpp");
   FileSpec header1("test-pdb.h");
   FileSpec header2("test-pdb-nested.h");
   uint32_t cus = symfile->GetNumCompileUnits();
-  EXPECT_EQ(2u, cus);
+  EXPECT_EQ(3u, cus);
 
   SymbolContextList sc_list;
   lldb::SymbolContextItem scope =
       lldb::eSymbolContextCompUnit | lldb::eSymbolContextLineEntry;
 
   // First test with line 7, and verify that only line 7 entries are added.
   uint32_t count =
       symfile->ResolveSymbolContext(source_file, 7, true, scope, sc_list);