zequanwu created this revision.
zequanwu added reviewers: rnk, labath.
Herald added a project: All.
zequanwu requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
`PdbAstBuilder::GetParentDeclContextForSymbol` finds the public symbol that has
same address as the given symbol, then creates parent DeclContext from demangled
name. When ICF happens, there are multiple public symbols share the same
address.
It may creates the wrong parent DeclContext.
This fixes it by filtering all public symbols that has the same addrss with base
name.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D133243
Files:
lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
lldb/test/Shell/SymbolFile/NativePDB/icf.cpp
Index: lldb/test/Shell/SymbolFile/NativePDB/icf.cpp
===================================================================
--- /dev/null
+++ lldb/test/Shell/SymbolFile/NativePDB/icf.cpp
@@ -0,0 +1,55 @@
+// clang-format off
+// REQUIRES: lld, x86
+
+// Test lldb is not treating function as class method or other way around when icf applies to a
+// function and a class method.
+// 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
+
+int main() {
+ A a;
+ B b;
+ return a.f1(1) + b.f2(1) + N1::f3(nullptr, 1) + N2::N3::f4(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: int main();
+// CHECK-NEXT: struct A {
+// CHECK-NEXT: int f1(int);
+// CHECK-NEXT: };
+// CHECK-NEXT: struct B {
+// CHECK-NEXT: int f2(int);
+// CHECK-NEXT: };
Index: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
@@ -470,9 +470,9 @@
return result;
}
-static llvm::Optional<PublicSym32> FindPublicSym(const SegmentOffset &addr,
- SymbolStream &syms,
- PublicsStream &publics) {
+static llvm::SmallVector<PublicSym32> FindPublicSyms(const SegmentOffset &addr,
+ SymbolStream &syms,
+ PublicsStream &publics) {
llvm::FixedStreamArray<ulittle32_t> addr_map = publics.getAddressMap();
auto iter = std::lower_bound(
addr_map.begin(), addr_map.end(), addr,
@@ -485,15 +485,17 @@
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;
+ llvm::SmallVector<PublicSym32, 1> public_syms;
+ while (iter != addr_map.end()) {
+ 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)
+ public_syms.push_back(p);
+ ++iter;
+ }
+ return public_syms;
}
clang::Decl *PdbAstBuilder::GetOrCreateSymbolForId(PdbCompilandSymId id) {
@@ -608,48 +610,56 @@
clang::DeclContext *
PdbAstBuilder::GetParentDeclContextForSymbol(const CVSymbol &sym) {
- if (!SymbolHasAddress(sym))
- return CreateDeclInfoForUndecoratedName(getSymbolName(sym)).first;
+ llvm::StringRef full_name = getSymbolName(sym);
+ llvm::StringRef base_name = full_name.rsplit("::").second;
+ if (!SymbolHasAddress(sym) || base_name.empty())
+ return CreateDeclInfoForUndecoratedName(full_name).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::SmallVector<PublicSym32> public_syms =
+ FindPublicSyms(addr, m_index.symrecords(), m_index.publics());
+
+ for (const PublicSym32 &pub : public_syms) {
+ llvm::ms_demangle::Demangler demangler;
+ StringView name{pub.Name.begin(), pub.Name.size()};
+ llvm::ms_demangle::SymbolNode *node = demangler.parse(name);
+ if (!node)
+ continue;
+ llvm::ArrayRef<llvm::ms_demangle::Node *> name_components{
+ node->Name->Components->Nodes, node->Name->Components->Count};
+ // Because ICF, we need to filter out the public symbols by base name.
+ if (name_components.empty() ||
+ base_name != name_components.back()->toString())
+ continue;
- 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();
+ name_components = name_components.drop_back();
+ 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();
+ // 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;
}
- return context;
+ return CreateDeclInfoForUndecoratedName(full_name).first;
}
clang::DeclContext *PdbAstBuilder::GetParentDeclContext(PdbSymUid uid) {
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
