[Lldb-commits] [lldb] 9b825dc - [LLDB][NativePDB] Fix PdbAstBuilder::GetParentDeclContext when ICF happens.

Zequan Wu via lldb-commits lldb-commits at lists.llvm.org
Thu Sep 8 10:42:23 PDT 2022


Author: Zequan Wu
Date: 2022-09-08T10:42:12-07:00
New Revision: 9b825dcdb267ee21e75e279c79ec2b3c50fd7346

URL: https://github.com/llvm/llvm-project/commit/9b825dcdb267ee21e75e279c79ec2b3c50fd7346
DIFF: https://github.com/llvm/llvm-project/commit/9b825dcdb267ee21e75e279c79ec2b3c50fd7346.diff

LOG: [LLDB][NativePDB] Fix PdbAstBuilder::GetParentDeclContext when ICF happens.

Removed `GetParentDeclContextForSymbol` as this is not necesssary. We can get
the demangled names from CVSymbol and then using it to create tag decl or
namespace decl. This also fixed a bug when icf applied.

Differential Revision: https://reviews.llvm.org/D133243

Added: 
    lldb/test/Shell/SymbolFile/NativePDB/icf.cpp

Modified: 
    lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
    lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.h

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
index 11a873873dc17..1be80e31c7fcc 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
@@ -456,45 +456,6 @@ static bool isLocalVariableType(SymbolKind K) {
   return false;
 }
 
-static std::string
-RenderScopeList(llvm::ArrayRef<llvm::ms_demangle::Node *> nodes) {
-  lldbassert(!nodes.empty());
-
-  std::string result = nodes.front()->toString();
-  nodes = nodes.drop_front();
-  while (!nodes.empty()) {
-    result += "::";
-    result += nodes.front()->toString(llvm::ms_demangle::OF_NoTagSpecifier);
-    nodes = nodes.drop_front();
-  }
-  return result;
-}
-
-static llvm::Optional<PublicSym32> FindPublicSym(const SegmentOffset &addr,
-                                                 SymbolStream &syms,
-                                                 PublicsStream &publics) {
-  llvm::FixedStreamArray<ulittle32_t> addr_map = publics.getAddressMap();
-  auto iter = llvm::lower_bound(
-      addr_map, addr, [&](const ulittle32_t &x, const SegmentOffset &y) {
-        CVSymbol s1 = syms.readRecord(x);
-        lldbassert(s1.kind() == S_PUB32);
-        PublicSym32 p1;
-        llvm::cantFail(SymbolDeserializer::deserializeAs<PublicSym32>(s1, p1));
-        if (p1.Segment < y.segment)
-          return true;
-        return p1.Offset < y.offset;
-      });
-  if (iter == addr_map.end())
-    return llvm::None;
-  CVSymbol sym = syms.readRecord(*iter);
-  lldbassert(sym.kind() == S_PUB32);
-  PublicSym32 p;
-  llvm::cantFail(SymbolDeserializer::deserializeAs<PublicSym32>(sym, p));
-  if (p.Segment == addr.segment && p.Offset == addr.offset)
-    return p;
-  return llvm::None;
-}
-
 clang::Decl *PdbAstBuilder::GetOrCreateSymbolForId(PdbCompilandSymId id) {
   CVSymbol cvs = m_index.ReadSymbolRecord(id);
 
@@ -605,52 +566,6 @@ PdbAstBuilder::CreateDeclInfoForUndecoratedName(llvm::StringRef name) {
   return {context, std::string(uname)};
 }
 
-clang::DeclContext *
-PdbAstBuilder::GetParentDeclContextForSymbol(const CVSymbol &sym) {
-  if (!SymbolHasAddress(sym))
-    return CreateDeclInfoForUndecoratedName(getSymbolName(sym)).first;
-  SegmentOffset addr = GetSegmentAndOffset(sym);
-  llvm::Optional<PublicSym32> pub =
-      FindPublicSym(addr, m_index.symrecords(), m_index.publics());
-  if (!pub)
-    return CreateDeclInfoForUndecoratedName(getSymbolName(sym)).first;
-
-  llvm::ms_demangle::Demangler demangler;
-  StringView name{pub->Name.begin(), pub->Name.size()};
-  llvm::ms_demangle::SymbolNode *node = demangler.parse(name);
-  if (!node)
-    return FromCompilerDeclContext(GetTranslationUnitDecl());
-  llvm::ArrayRef<llvm::ms_demangle::Node *> name_components{
-      node->Name->Components->Nodes, node->Name->Components->Count - 1};
-
-  if (!name_components.empty()) {
-    // Render the current list of scope nodes as a fully qualified name, and
-    // look it up in the debug info as a type name.  If we find something,
-    // this is a type (which may itself be prefixed by a namespace).  If we
-    // don't, this is a list of namespaces.
-    std::string qname = RenderScopeList(name_components);
-    std::vector<TypeIndex> matches = m_index.tpi().findRecordsByName(qname);
-    while (!matches.empty()) {
-      clang::QualType qt = GetOrCreateType(matches.back());
-      if (qt.isNull())
-        continue;
-      clang::TagDecl *tag = qt->getAsTagDecl();
-      if (tag)
-        return clang::TagDecl::castToDeclContext(tag);
-      matches.pop_back();
-    }
-  }
-
-  // It's not a type.  It must be a series of namespaces.
-  auto context = FromCompilerDeclContext(GetTranslationUnitDecl());
-  while (!name_components.empty()) {
-    std::string ns = name_components.front()->toString();
-    context = GetOrCreateNamespaceDecl(ns.c_str(), *context);
-    name_components = name_components.drop_front();
-  }
-  return context;
-}
-
 clang::DeclContext *PdbAstBuilder::GetParentDeclContext(PdbSymUid uid) {
   // We must do this *without* calling GetOrCreate on the current uid, as
   // that would be an infinite recursion.
@@ -662,7 +577,7 @@ clang::DeclContext *PdbAstBuilder::GetParentDeclContext(PdbSymUid uid) {
       return GetOrCreateDeclContextForUid(*scope);
 
     CVSymbol sym = m_index.ReadSymbolRecord(uid.asCompilandSym());
-    return GetParentDeclContextForSymbol(sym);
+    return CreateDeclInfoForUndecoratedName(getSymbolName(sym)).first;
   }
   case PdbSymUidKind::Type: {
     // It could be a namespace, class, or global.  We don't support nested
@@ -688,7 +603,7 @@ clang::DeclContext *PdbAstBuilder::GetParentDeclContext(PdbSymUid uid) {
     switch (global.kind()) {
     case SymbolKind::S_GDATA32:
     case SymbolKind::S_LDATA32:
-      return GetParentDeclContextForSymbol(global);
+      return CreateDeclInfoForUndecoratedName(getSymbolName(global)).first;;
     case SymbolKind::S_PROCREF:
     case SymbolKind::S_LPROCREF: {
       ProcRefSym ref{global.kind()};

diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.h b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.h
index 40425dd4c6e72..e4d9c6e6e0bbf 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.h
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.h
@@ -113,9 +113,6 @@ class PdbAstBuilder {
   clang::VarDecl *CreateVariableDecl(PdbSymUid uid,
                                      llvm::codeview::CVSymbol sym,
                                      clang::DeclContext &scope);
-  clang::DeclContext *
-  GetParentDeclContextForSymbol(const llvm::codeview::CVSymbol &sym);
-
   clang::NamespaceDecl *GetOrCreateNamespaceDecl(const char *name,
                                                  clang::DeclContext &context);
   clang::FunctionDecl *CreateFunctionDeclFromId(PdbTypeSymId func_tid,

diff  --git a/lldb/test/Shell/SymbolFile/NativePDB/icf.cpp b/lldb/test/Shell/SymbolFile/NativePDB/icf.cpp
new file mode 100644
index 0000000000000..d9a7373bb12d6
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/NativePDB/icf.cpp
@@ -0,0 +1,71 @@
+// clang-format off
+// REQUIRES: lld, x86
+
+// Test lldb finds the correct parent context decl for functions and class methods when icf happens.
+// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -GS- -fno-addrsig -c /Fo%t.obj -- %s
+// RUN: lld-link -opt:icf -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe -pdb:%t.pdb
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 lldb-test symbols --dump-ast %t.exe | FileCheck %s
+
+struct A {
+  int f1(int x) {
+    return x * 2;
+  }
+};
+struct B {
+  int f2(int x) {
+    return x * 2;
+  }
+};
+namespace N1 {
+int f3(void*, int x) {
+  return  x * 2;
+}
+} // namespace N1
+
+namespace N2 {
+namespace N3 {
+  int f4(void*, int x) {
+    return  x * 2;
+  }
+} // namespace N3
+} // namespace N2
+
+namespace N4 {
+  // Same base name as N1::f3 but 
diff erent namespaces.
+  int f3(void*, int x) {
+    return x * 2;
+  }
+  // Same base name as B::f2 but this is in namespace.
+  int f2(void*, int x) {
+    return x * 2;
+  }
+} // namespace N4
+
+
+int main() {
+  A a;
+  B b;
+  return a.f1(1) + b.f2(1) + N1::f3(nullptr, 1) + N2::N3::f4(nullptr, 1) +
+         N4::f3(nullptr, 1);
+}
+
+
+// CHECK:      namespace N1 {
+// CHECK-NEXT:     int f3(void *, int x);
+// CHECK-NEXT: }
+// CHECK-NEXT: namespace N2 {
+// CHECK-NEXT:     namespace N3 {
+// CHECK-NEXT:         int f4(void *, int x);
+// CHECK-NEXT:     }
+// CHECK-NEXT: }
+// CHECK-NEXT: namespace N4 {
+// CHECK-NEXT:     int f3(void *, int x);
+// CHECK-NEXT:     int f2(void *, int x);
+// CHECK-NEXT: }
+// CHECK-NEXT: int main();
+// CHECK-NEXT: struct A {
+// CHECK-NEXT:     int f1(int);
+// CHECK-NEXT: };
+// CHECK-NEXT: struct B {
+// CHECK-NEXT:     int f2(int);
+// CHECK-NEXT: };


        


More information about the lldb-commits mailing list