Skip to content

Commit

Permalink
[lldb] Prevent false positives with simple template names in SymbolFi…
Browse files Browse the repository at this point in the history
…leDWARF::FindTypes

The provided test case was crashing because of confusion attempting to find types for `ns::Foo` under -gsimple-template-names. (This looks broken normally because it's attempting to find `ns::Foo` rather than `ns::Foo<T>`)

Looking up types can't give false positives, as opposed to looking up functions as mentioned in https://reviews.llvm.org/D137098.

Reviewed By: Michael137

Differential Revision: https://reviews.llvm.org/D140240
  • Loading branch information
aeubanks committed Dec 20, 2022
1 parent 2cf550a commit d483d48
Show file tree
Hide file tree
Showing 12 changed files with 109 additions and 5 deletions.
2 changes: 2 additions & 0 deletions lldb/include/lldb/Symbol/CompilerType.h
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,8 @@ class CompilerType {

bool IsScalarType() const;

bool IsTemplateType() const;

bool IsTypedefType() const;

bool IsVoidType() const;
Expand Down
3 changes: 3 additions & 0 deletions lldb/include/lldb/Symbol/Type.h
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,9 @@ class Type : public std::enable_shared_from_this<Type>, public UserID {

bool IsAggregateType();

// Returns if the type is a templated decl. Does not look through typedefs.
bool IsTemplateType();

bool IsValidType() { return m_encoding_uid_type != eEncodingInvalid; }

bool IsTypedef() { return m_encoding_uid_type == eEncodingIsTypedefUID; }
Expand Down
2 changes: 2 additions & 0 deletions lldb/include/lldb/Symbol/TypeSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,8 @@ class TypeSystem : public PluginInterface,
const char *name, bool omit_empty_base_classes,
std::vector<uint32_t> &child_indexes) = 0;

virtual bool IsTemplateType(lldb::opaque_compiler_type_t type);

virtual size_t GetNumTemplateArguments(lldb::opaque_compiler_type_t type,
bool expand_pack);

Expand Down
22 changes: 17 additions & 5 deletions lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2501,6 +2501,11 @@ void SymbolFileDWARF::FindTypes(
if (!DeclContextMatchesThisSymbolFile(parent_decl_ctx))
return;

// Unlike FindFunctions(), FindTypes() following cannot produce false
// positives.

const llvm::StringRef name_ref = name.GetStringRef();
auto name_bracket_index = name_ref.find('<');
m_index->GetTypes(name, [&](DWARFDIE die) {
if (!DIEInDeclContext(parent_decl_ctx, die))
return true; // The containing decl contexts don't match
Expand All @@ -2509,6 +2514,13 @@ void SymbolFileDWARF::FindTypes(
if (!matching_type)
return true;

// With -gsimple-template-names, a templated type's DW_AT_name will not
// contain the template parameters. Make sure that if the original query
// didn't contain a '<', we filter out entries with template parameters.
if (name_bracket_index == llvm::StringRef::npos &&
matching_type->IsTemplateType())
return true;

// We found a type pointer, now find the shared pointer form our type
// list
types.InsertUnique(matching_type->shared_from_this());
Expand All @@ -2519,11 +2531,11 @@ void SymbolFileDWARF::FindTypes(
// contain the template parameters. Try again stripping '<' and anything
// after, filtering out entries with template parameters that don't match.
if (types.GetSize() < max_matches) {
const llvm::StringRef name_ref = name.GetStringRef();
auto it = name_ref.find('<');
if (it != llvm::StringRef::npos) {
const llvm::StringRef name_no_template_params = name_ref.slice(0, it);
const llvm::StringRef template_params = name_ref.slice(it, name_ref.size());
if (name_bracket_index != llvm::StringRef::npos) {
const llvm::StringRef name_no_template_params =
name_ref.slice(0, name_bracket_index);
const llvm::StringRef template_params =
name_ref.slice(name_bracket_index, name_ref.size());
m_index->GetTypes(ConstString(name_no_template_params), [&](DWARFDIE die) {
if (!DIEInDeclContext(parent_decl_ctx, die))
return true; // The containing decl contexts don't match
Expand Down
11 changes: 11 additions & 0 deletions lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7131,6 +7131,17 @@ TypeSystemClang::GetIndexOfChildWithName(lldb::opaque_compiler_type_t type,
return UINT32_MAX;
}

bool TypeSystemClang::IsTemplateType(lldb::opaque_compiler_type_t type) {
if (!type)
return false;
CompilerType ct(weak_from_this(), type);
const clang::Type *clang_type = ClangUtil::GetQualType(ct).getTypePtr();
if (auto *cxx_record_decl = dyn_cast<clang::TagType>(clang_type))
return isa<clang::ClassTemplateSpecializationDecl>(
cxx_record_decl->getDecl());
return false;
}

size_t
TypeSystemClang::GetNumTemplateArguments(lldb::opaque_compiler_type_t type,
bool expand_pack) {
Expand Down
2 changes: 2 additions & 0 deletions lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,8 @@ class TypeSystemClang : public TypeSystem {
const char *name, bool omit_empty_base_classes,
std::vector<uint32_t> &child_indexes) override;

bool IsTemplateType(lldb::opaque_compiler_type_t type) override;

size_t GetNumTemplateArguments(lldb::opaque_compiler_type_t type,
bool expand_pack) override;

Expand Down
7 changes: 7 additions & 0 deletions lldb/source/Symbol/CompilerType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,13 @@ bool CompilerType::IsScalarType() const {
return false;
}

bool CompilerType::IsTemplateType() const {
if (IsValid())
if (auto type_system_sp = GetTypeSystem())
return type_system_sp->IsTemplateType(m_type);
return false;
}

bool CompilerType::IsTypedefType() const {
if (IsValid())
if (auto type_system_sp = GetTypeSystem())
Expand Down
4 changes: 4 additions & 0 deletions lldb/source/Symbol/Type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,10 @@ bool Type::IsAggregateType() {
return GetForwardCompilerType().IsAggregateType();
}

bool Type::IsTemplateType() {
return GetForwardCompilerType().IsTemplateType();
}

lldb::TypeSP Type::GetTypedefType() {
lldb::TypeSP type_sp;
if (IsTypedef()) {
Expand Down
4 changes: 4 additions & 0 deletions lldb/source/Symbol/TypeSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,10 @@ CompilerType TypeSystem::GetTypeForFormatters(void *type) {
return CompilerType(weak_from_this(), type);
}

bool TypeSystem::IsTemplateType(lldb::opaque_compiler_type_t type) {
return false;
}

size_t TypeSystem::GetNumTemplateArguments(lldb::opaque_compiler_type_t type,
bool expand_pack) {
return 0;
Expand Down
3 changes: 3 additions & 0 deletions lldb/test/API/lang/cpp/unique-types4/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
CXX_SOURCES := main.cpp

include Makefile.rules
31 changes: 31 additions & 0 deletions lldb/test/API/lang/cpp/unique-types4/TestUniqueTypes4.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
"""
Test that we return only the requested template instantiation.
"""

import lldb
import lldbsuite.test.lldbutil as lldbutil
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *

class UniqueTypesTestCase4(TestBase):
def do_test(self, debug_flags):
"""Test that we display the correct template instantiation."""
self.build(dictionary=debug_flags)
lldbutil.run_to_source_breakpoint(self, "// Set breakpoint here", lldb.SBFileSpec("main.cpp"))
# FIXME: these should successfully print the values
self.expect("print ns::Foo<double>::value", substrs=["no member named"], error=True)
self.expect("print ns::Foo<int>::value", substrs=["no member named"], error=True)
self.expect("print ns::Bar<double>::value", substrs=["no member named"], error=True)
self.expect("print ns::Bar<int>::value", substrs=["no member named"], error=True)
self.expect("print ns::FooDouble::value", substrs=["Couldn't lookup symbols"], error=True)
self.expect("print ns::FooInt::value", substrs=["Couldn't lookup symbols"], error=True)

@skipIf(compiler=no_match("clang"))
@skipIf(compiler_version=["<", "15.0"])
def test_simple_template_names(self):
self.do_test(dict(CFLAGS_EXTRAS="-gsimple-template-names"))

@skipIf(compiler=no_match("clang"))
@skipIf(compiler_version=["<", "15.0"])
def test_no_simple_template_names(self):
self.do_test(dict(CFLAGS_EXTRAS="-gno-simple-template-names"))
23 changes: 23 additions & 0 deletions lldb/test/API/lang/cpp/unique-types4/main.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
namespace ns {

template <typename T> struct Foo {
static T value;
};

template <typename T> using Bar = Foo<T>;

using FooInt = Foo<int>;
using FooDouble = Foo<double>;

} // namespace ns

ns::Foo<double> a;
ns::Foo<int> b;
ns::Bar<double> c;
ns::Bar<int> d;
ns::FooInt e;
ns::FooDouble f;

int main() {
// Set breakpoint here
}

0 comments on commit d483d48

Please sign in to comment.