-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[LLDB][NativePDB] Allow type lookup in namespaces #149876
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
fa3c96b
288b5f7
1dbde10
8344a98
9694e94
fb5bce0
85c2c42
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1720,18 +1720,22 @@ void SymbolFileNativePDB::FindTypes(const lldb_private::TypeQuery &query, | |
|
|
||
| std::lock_guard<std::recursive_mutex> guard(GetModuleMutex()); | ||
|
|
||
| std::vector<TypeIndex> matches = | ||
| m_index->tpi().findRecordsByName(query.GetTypeBasename().GetStringRef()); | ||
|
|
||
| for (TypeIndex type_idx : matches) { | ||
| TypeSP type_sp = GetOrCreateType(type_idx); | ||
| if (!type_sp) | ||
| // We can't query for the full name because the type might reside | ||
| // in an anonymous namespace. Search for the basename in our map and check the | ||
| // matching types afterwards. | ||
| std::vector<uint32_t> matches; | ||
| m_type_base_names.GetValues(query.GetTypeBasename(), matches); | ||
|
|
||
| for (uint32_t match_idx : matches) { | ||
| std::vector context = GetContextForType(TypeIndex(match_idx)); | ||
| if (context.empty()) | ||
| continue; | ||
|
|
||
| // We resolved a type. Get the fully qualified name to ensure it matches. | ||
| ConstString name = type_sp->GetQualifiedName(); | ||
| TypeQuery type_match(name.GetStringRef(), TypeQueryOptions::e_exact_match); | ||
| if (query.ContextMatches(type_match.GetContextRef())) { | ||
| if (query.ContextMatches(context)) { | ||
| TypeSP type_sp = GetOrCreateType(TypeIndex(match_idx)); | ||
| if (!type_sp) | ||
| continue; | ||
|
|
||
| results.InsertUnique(type_sp); | ||
| if (results.Done(query)) | ||
| return; | ||
|
|
@@ -2203,9 +2207,13 @@ void SymbolFileNativePDB::BuildParentMap() { | |
| RecordIndices &indices = record_indices[tag.asTag().getUniqueName()]; | ||
| if (tag.asTag().isForwardRef()) | ||
Nerixyz marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| indices.forward = *ti; | ||
| else | ||
| else { | ||
| indices.full = *ti; | ||
|
|
||
| auto base_name = MSVCUndecoratedNameParser::DropScope(tag.name()); | ||
| m_type_base_names.Append(ConstString(base_name), ti->getIndex()); | ||
| } | ||
|
|
||
| if (indices.full != TypeIndex::None() && | ||
| indices.forward != TypeIndex::None()) { | ||
| forward_to_full[indices.forward] = indices.full; | ||
|
|
@@ -2291,6 +2299,8 @@ void SymbolFileNativePDB::BuildParentMap() { | |
| TypeIndex fwd = full_to_forward[full]; | ||
| m_parent_types[fwd] = m_parent_types[full]; | ||
| } | ||
|
|
||
| m_type_base_names.Sort(); | ||
|
||
| } | ||
|
|
||
| std::optional<PdbCompilandSymId> | ||
|
|
@@ -2353,3 +2363,52 @@ SymbolFileNativePDB::GetParentType(llvm::codeview::TypeIndex ti) { | |
| return std::nullopt; | ||
| return parent_iter->second; | ||
| } | ||
|
|
||
| std::vector<CompilerContext> | ||
| SymbolFileNativePDB::GetContextForType(TypeIndex ti) { | ||
| CVType type = m_index->tpi().getType(ti); | ||
| if (!IsTagRecord(type)) | ||
| return {}; | ||
|
|
||
| CVTagRecord tag = CVTagRecord::create(type); | ||
|
|
||
| std::optional<Type::ParsedName> parsed_name = | ||
| Type::GetTypeScopeAndBasename(tag.name()); | ||
| if (!parsed_name) | ||
| return {{tag.contextKind(), ConstString(tag.name())}}; | ||
|
|
||
| std::vector<CompilerContext> ctx; | ||
| // assume everything is a namespace at first | ||
| for (llvm::StringRef scope : parsed_name->scope) { | ||
| ctx.emplace_back(CompilerContextKind::Namespace, ConstString(scope)); | ||
| } | ||
| // we know the kind of our own type | ||
| ctx.emplace_back(tag.contextKind(), ConstString(parsed_name->basename)); | ||
|
|
||
| // try to find the kind of parents | ||
| for (auto &el : llvm::reverse(llvm::drop_end(ctx))) { | ||
| std::optional<TypeIndex> parent = GetParentType(ti); | ||
| if (!parent) | ||
| break; | ||
|
|
||
| ti = *parent; | ||
| type = m_index->tpi().getType(ti); | ||
| switch (type.kind()) { | ||
| case LF_CLASS: | ||
| case LF_STRUCTURE: | ||
| case LF_INTERFACE: | ||
| el.kind = CompilerContextKind::ClassOrStruct; | ||
| continue; | ||
| case LF_UNION: | ||
| el.kind = CompilerContextKind::Union; | ||
| continue; | ||
| case LF_ENUM: | ||
| el.kind = CompilerContextKind::Enum; | ||
| continue; | ||
| default: | ||
| break; | ||
| } | ||
| break; | ||
| } | ||
| return ctx; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -816,10 +816,12 @@ Type::GetTypeScopeAndBasename(llvm::StringRef name) { | |
| case ':': | ||
| if (prev_is_colon && template_depth == 0) { | ||
| llvm::StringRef scope_name = name.slice(name_begin, pos.index() - 1); | ||
| // The itanium demangler uses this string to represent anonymous | ||
| // The demanglers use these strings to represent anonymous | ||
| // namespaces. Convert it to a more language-agnostic form (which is | ||
| // also used in DWARF). | ||
| if (scope_name == "(anonymous namespace)") | ||
| if (scope_name == "(anonymous namespace)" || | ||
| scope_name == "`anonymous namespace'" || | ||
| scope_name == "`anonymous-namespace'") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if this is how the MS demangler presents anonymous namespaces there's probably some other places where we hardcode this that might need patching up. But that's outside the scope of this PR |
||
| scope_name = ""; | ||
| result.scope.push_back(scope_name); | ||
| name_begin = pos.index() + 1; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,135 @@ | ||
| # REQUIRES: target-windows | ||
|
|
||
| # Test namespace lookup. | ||
| # RUN: split-file %s %t | ||
| # RUN: %build --nodefaultlib -o %t.exe -- %t/main.cpp | ||
| # RUN: %lldb -f %t.exe -s \ | ||
| # RUN: %t/commands.input 2>&1 | FileCheck %s | ||
|
|
||
| #--- main.cpp | ||
|
|
||
| struct S { | ||
| char a[1]; | ||
| }; | ||
|
|
||
| namespace Outer { | ||
|
|
||
| struct S { | ||
| char a[2]; | ||
| }; | ||
|
|
||
| namespace Inner1 { | ||
| struct S { | ||
| char a[3]; | ||
| }; | ||
|
|
||
| namespace Inner2 { | ||
| struct S { | ||
| char a[4]; | ||
| }; | ||
| } // namespace Inner2 | ||
| } // namespace Inner1 | ||
|
|
||
| namespace Inner2 { | ||
| struct S { | ||
| char a[5]; | ||
| }; | ||
| } // namespace Inner2 | ||
|
|
||
| namespace { | ||
| struct A { | ||
| char a[6]; | ||
| }; | ||
| } // namespace | ||
|
|
||
| } // namespace Outer | ||
|
|
||
| namespace { | ||
| struct A { | ||
| char a[7]; | ||
| }; | ||
| } // namespace | ||
|
|
||
| int main(int argc, char **argv) { | ||
| S s; | ||
| Outer::S os; | ||
| Outer::Inner1::S oi1s; | ||
| Outer::Inner1::Inner2::S oi1i2s; | ||
| Outer::Inner2::S oi2s; | ||
| A a1; | ||
| Outer::A a2; | ||
| return sizeof(s) + sizeof(os) + sizeof(oi1s) + sizeof(oi1i2s) + sizeof(oi2s) + sizeof(a1) + sizeof(a2); | ||
| } | ||
|
|
||
| #--- commands.input | ||
|
|
||
| b main | ||
| r | ||
|
|
||
| type lookup S | ||
| type lookup ::S | ||
| type lookup Outer::S | ||
| type lookup Outer::Inner1::S | ||
| type lookup Inner1::S | ||
| type lookup Outer::Inner1::Inner2::S | ||
| type lookup Inner2::S | ||
| type lookup Outer::Inner2::S | ||
| type lookup Outer::A | ||
| type lookup A | ||
| type lookup ::A | ||
| expr sizeof(S) | ||
| expr sizeof(A) | ||
|
|
||
| quit | ||
|
|
||
| # CHECK: (lldb) type lookup S | ||
| # CHECK: struct S { | ||
| # CHECK: struct S { | ||
| # CHECK: struct S { | ||
| # CHECK: struct S { | ||
| # CHECK: struct S { | ||
| # CHECK: } | ||
| # CHECK-NEXT: (lldb) type lookup ::S | ||
| # CHECK-NEXT: struct S { | ||
| # CHECK-NEXT: char a[1]; | ||
| # CHECK-NEXT: } | ||
| # CHECK-NEXT: (lldb) type lookup Outer::S | ||
| # CHECK-NEXT: struct S { | ||
| # CHECK-NEXT: char a[2]; | ||
| # CHECK-NEXT: } | ||
| # CHECK-NEXT: (lldb) type lookup Outer::Inner1::S | ||
| # CHECK-NEXT: struct S { | ||
| # CHECK-NEXT: char a[3]; | ||
| # CHECK-NEXT: } | ||
| # CHECK-NEXT: (lldb) type lookup Inner1::S | ||
| # CHECK-NEXT: struct S { | ||
| # CHECK-NEXT: char a[3]; | ||
| # CHECK-NEXT: } | ||
| # CHECK-NEXT: (lldb) type lookup Outer::Inner1::Inner2::S | ||
| # CHECK-NEXT: struct S { | ||
| # CHECK-NEXT: char a[4]; | ||
| # CHECK-NEXT: } | ||
| # CHECK-NEXT: (lldb) type lookup Inner2::S | ||
| # CHECK-NEXT: struct S { | ||
| # CHECK: struct S { | ||
| # CHECK: } | ||
| # CHECK-NEXT: (lldb) type lookup Outer::Inner2::S | ||
| # CHECK-NEXT: struct S { | ||
| # CHECK-NEXT: char a[5]; | ||
| # CHECK-NEXT: } | ||
| # CHECK-NEXT: (lldb) type lookup Outer::A | ||
| # CHECK-NEXT: struct A { | ||
| # CHECK-NEXT: char a[6]; | ||
| # CHECK-NEXT: } | ||
| # CHECK-NEXT: (lldb) type lookup A | ||
| # CHECK-NEXT: struct A { | ||
| # CHECK: struct A { | ||
| # CHECK: } | ||
| # CHECK-NEXT: (lldb) type lookup ::A | ||
| # CHECK-NEXT: struct A { | ||
| # CHECK-NEXT: char a[7]; | ||
| # CHECK-NEXT: } | ||
| # CHECK-NEXT: (lldb) expr sizeof(S) | ||
| # CHECK-NEXT: (__size_t) $0 = 1 | ||
| # CHECK-NEXT: (lldb) expr sizeof(A) | ||
| # CHECK-NEXT: (__size_t) $1 = 7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we check (or assert) that the
m_kindis in fact a union?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although the other accessors don't do this, I added an assertion there. Also looks like there's
name()accessor accesses the wrong union element if the kind is class. In practice, that isn't an issue because all types inherit from the same base classTagRecord.