diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h @@ -328,6 +328,8 @@ (!packed_args || !packed_args->packed_args); } + bool hasParameterPack() const { return static_cast(packed_args); } + llvm::SmallVector names; llvm::SmallVector args; diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -1357,9 +1357,11 @@ } namespace { - bool IsValueParam(const clang::TemplateArgument &argument) { - return argument.getKind() == TemplateArgument::Integral; - } +/// Returns true iff the given TemplateArgument should be represented as an +/// NonTypeTemplateParmDecl in the AST. +bool IsValueParam(const clang::TemplateArgument &argument) { + return argument.getKind() == TemplateArgument::Integral; +} } static TemplateParameterList *CreateTemplateParameterList( @@ -1463,6 +1465,99 @@ template_args_ptr, nullptr); } +/// Returns true if the given template parameter can represent the given value. +/// For example, `typename T` can represent `int` but not integral values such +/// as `int I = 3`. +static bool TemplateParameterAllowsValue(NamedDecl *param, + const TemplateArgument &value) { + if (auto *type_param = llvm::dyn_cast(param)) { + // Compare the argument kind, i.e. ensure that != . + if (value.getKind() != TemplateArgument::Type) + return false; + } else if (auto *type_param = + llvm::dyn_cast(param)) { + // Compare the argument kind, i.e. ensure that != . + if (!IsValueParam(value)) + return false; + // Compare the integral type, i.e. ensure that != . + if (type_param->getType() != value.getIntegralType()) + return false; + } else { + // There is no way to create other parameter decls at the moment, so we + // can't reach this case during normal LLDB usage. Log that this happened + // and assert. + Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS); + LLDB_LOG(log, + "Don't know how to compare template parameter to passed" + " value. Decl kind of parameter is: {0}", + param->getDeclKindName()); + lldbassert(false && "Can't compare this TemplateParmDecl subclass"); + // In release builds just fall back to marking the parameter as not + // accepting the value so that we don't try to fit an instantiation to a + // template that doesn't fit. E.g., avoid that `S<1>` is being connected to + // `template struct S;`. + return false; + } + return true; +} + +/// Returns true if the given class template declaration could produce an +/// instantiation with the specified values. +/// For example, `` allows the arguments `float`, but not for +/// example `bool, float` or `3` (as an integer parameter value). +static bool ClassTemplateAllowsToInstantiationArgs( + ClassTemplateDecl *class_template_decl, + const TypeSystemClang::TemplateParameterInfos &instantiation_values) { + + TemplateParameterList ¶ms = *class_template_decl->getTemplateParameters(); + + // Save some work by iterating only once over the found parameters and + // calculate the information related to parameter packs. + + // Contains the first pack parameter (or non if there are none). + llvm::Optional pack_parameter; + // Contains the number of non-pack parameters. + size_t non_pack_params = params.size(); + for (size_t i = 0; i < params.size(); ++i) { + NamedDecl *param = params.getParam(i); + if (param->isParameterPack()) { + pack_parameter = param; + non_pack_params = i; + break; + } + } + + // The found template needs to have compatible non-pack template arguments. + // E.g., ensure that != . + // The pack parameters are compared later. + if (non_pack_params != instantiation_values.args.size()) + return false; + + // Ensure that != . + if (pack_parameter.hasValue() != instantiation_values.hasParameterPack()) + return false; + + // Compare the first pack parameter that was found with the first pack + // parameter value. The special case of having an empty parameter pack value + // always fits to a pack parameter. + // E.g., ensure that != . + if (pack_parameter && !instantiation_values.packed_args->args.empty() && + !TemplateParameterAllowsValue( + *pack_parameter, instantiation_values.packed_args->args.front())) + return false; + + // Compare all the non-pack parameters now. + // E.g., ensure that != . + for (const auto pair : llvm::zip_first(instantiation_values.args, params)) { + const TemplateArgument &passed_arg = std::get<0>(pair); + NamedDecl *found_param = std::get<1>(pair); + if (!TemplateParameterAllowsValue(found_param, passed_arg)) + return false; + } + + return class_template_decl; +} + ClassTemplateDecl *TypeSystemClang::CreateClassTemplateDecl( DeclContext *decl_ctx, OptionalClangModuleID owning_module, lldb::AccessType access_type, const char *class_name, int kind, @@ -1476,12 +1571,22 @@ IdentifierInfo &identifier_info = ast.Idents.get(class_name); DeclarationName decl_name(&identifier_info); + // Search the AST for an existing ClassTemplateDecl that could be reused. clang::DeclContext::lookup_result result = decl_ctx->lookup(decl_name); - for (NamedDecl *decl : result) { class_template_decl = dyn_cast(decl); - if (class_template_decl) - return class_template_decl; + if (!class_template_decl) + continue; + // The class template has to be able to represents the instantiation + // values we received. Without this we might end up putting an instantiation + // with arguments such as to a template such as: + // template struct S; + // Connecting the instantiation to an incompatible template could cause + // problems later on. + if (!ClassTemplateAllowsToInstantiationArgs(class_template_decl, + template_param_infos)) + continue; + return class_template_decl; } llvm::SmallVector template_param_decls; diff --git a/lldb/test/API/lang/cpp/forward-declared-template-specialization/Makefile b/lldb/test/API/lang/cpp/forward-declared-template-specialization/Makefile new file mode 100644 --- /dev/null +++ b/lldb/test/API/lang/cpp/forward-declared-template-specialization/Makefile @@ -0,0 +1,3 @@ +CXX_SOURCES := main.cpp + +include Makefile.rules diff --git a/lldb/test/API/lang/cpp/forward-declared-template-specialization/TestCppForwardDeclaredTemplateSpecialization.py b/lldb/test/API/lang/cpp/forward-declared-template-specialization/TestCppForwardDeclaredTemplateSpecialization.py new file mode 100644 --- /dev/null +++ b/lldb/test/API/lang/cpp/forward-declared-template-specialization/TestCppForwardDeclaredTemplateSpecialization.py @@ -0,0 +1,19 @@ +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + +class TestCase(TestBase): + + mydir = TestBase.compute_mydir(__file__) + + def test(self): + """ + Tests a forward declared template and a normal template in the same + executable. GCC/Clang emit very limited debug information for forward + declared templates that might trip up LLDB. + """ + self.build() + lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.cpp")) + + self.expect_expr("a; b", result_type="Temp") diff --git a/lldb/test/API/lang/cpp/forward-declared-template-specialization/main.cpp b/lldb/test/API/lang/cpp/forward-declared-template-specialization/main.cpp new file mode 100644 --- /dev/null +++ b/lldb/test/API/lang/cpp/forward-declared-template-specialization/main.cpp @@ -0,0 +1,16 @@ +// Forward declare a template and a specialization; +template class Temp; +template <> class Temp; + +// Force that debug informatin for the specialization is emitted. +// Clang and GCC will create debug information that lacks any description +// of the template argument 'int'. +Temp *a; + +// Define the template and create an implicit instantiation. +template class Temp { int f; }; +Temp b; + +int main() { + return 0; // break here +} diff --git a/lldb/test/API/lang/cpp/incompatible-class-templates/Makefile b/lldb/test/API/lang/cpp/incompatible-class-templates/Makefile new file mode 100644 --- /dev/null +++ b/lldb/test/API/lang/cpp/incompatible-class-templates/Makefile @@ -0,0 +1,3 @@ +CXX_SOURCES := main.cpp other.cpp + +include Makefile.rules diff --git a/lldb/test/API/lang/cpp/incompatible-class-templates/TestCppIncompatibleClassTemplates.py b/lldb/test/API/lang/cpp/incompatible-class-templates/TestCppIncompatibleClassTemplates.py new file mode 100644 --- /dev/null +++ b/lldb/test/API/lang/cpp/incompatible-class-templates/TestCppIncompatibleClassTemplates.py @@ -0,0 +1,19 @@ +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + +class TestCase(TestBase): + + mydir = TestBase.compute_mydir(__file__) + + def test(self): + """ + Test debugging a binary that has two templates with the same name + but different template parameters. + """ + self.build() + lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.cpp")) + + # Try using both templates in the same expression. This shouldn't crash. + self.expect_expr("Template1.x + Template2.x", result_type="int") diff --git a/lldb/test/API/lang/cpp/incompatible-class-templates/main.cpp b/lldb/test/API/lang/cpp/incompatible-class-templates/main.cpp new file mode 100644 --- /dev/null +++ b/lldb/test/API/lang/cpp/incompatible-class-templates/main.cpp @@ -0,0 +1,11 @@ +int other(); + +namespace { +template struct Temp { int x; }; +// This emits the 'Temp' template in this TU. +Temp Template1; +} // namespace + +int main() { + return Template1.x + other(); // break here +} diff --git a/lldb/test/API/lang/cpp/incompatible-class-templates/other.cpp b/lldb/test/API/lang/cpp/incompatible-class-templates/other.cpp new file mode 100644 --- /dev/null +++ b/lldb/test/API/lang/cpp/incompatible-class-templates/other.cpp @@ -0,0 +1,7 @@ +namespace { +template struct Temp { int x; }; +// This emits the 'Temp' template from this TU. +Temp Template2; +} // namespace + +int other() { return Template2.x; } diff --git a/lldb/unittests/Symbol/TestTypeSystemClang.cpp b/lldb/unittests/Symbol/TestTypeSystemClang.cpp --- a/lldb/unittests/Symbol/TestTypeSystemClang.cpp +++ b/lldb/unittests/Symbol/TestTypeSystemClang.cpp @@ -515,6 +515,187 @@ } } +class TestCreateClassTemplateDecl : public TestTypeSystemClang { +protected: + /// The class templates created so far by the Expect* functions below. + llvm::DenseSet m_created_templates; + + /// Utility function for creating a class template. + ClassTemplateDecl * + CreateClassTemplate(const TypeSystemClang::TemplateParameterInfos &infos) { + ClassTemplateDecl *decl = m_ast->CreateClassTemplateDecl( + m_ast->GetTranslationUnitDecl(), OptionalClangModuleID(), eAccessPublic, + "foo", TTK_Struct, infos); + return decl; + } + + /// Creates a new class template with the given template parameters. + /// Asserts that a new ClassTemplateDecl is created. + /// \param description The gtest scope string that should describe the input. + /// \param infos The template parameters that the class template should have. + /// \returns The created ClassTemplateDecl. + ClassTemplateDecl * + ExpectNewTemplate(std::string description, + const TypeSystemClang::TemplateParameterInfos &infos) { + SCOPED_TRACE(description); + ClassTemplateDecl *first_template = CreateClassTemplate(infos); + // A new template should have been created. + EXPECT_FALSE(m_created_templates.contains(first_template)) + << "Didn't create new class template but reused this existing decl:\n" + << ClangUtil::DumpDecl(first_template); + m_created_templates.insert(first_template); + + // Creating a new template with the same arguments should always return + // the template created above. + ClassTemplateDecl *second_template = CreateClassTemplate(infos); + EXPECT_EQ(first_template, second_template) + << "Second attempt to create class template didn't reuse first decl:\n" + << ClangUtil::DumpDecl(first_template) << "\nInstead created/reused:\n" + << ClangUtil::DumpDecl(second_template); + return first_template; + } + + /// Tries to create a new class template but asserts that an existing class + /// template in the current AST is reused (in contract so a new class + /// template being created). + /// \param description The gtest scope string that should describe the input. + /// \param infos The template parameters that the class template should have. + void + ExpectReusedTemplate(std::string description, + const TypeSystemClang::TemplateParameterInfos &infos, + ClassTemplateDecl *expected) { + SCOPED_TRACE(description); + ClassTemplateDecl *td = CreateClassTemplate(infos); + EXPECT_EQ(td, expected) + << "Created/reused class template is:\n" + << ClangUtil::DumpDecl(td) << "\nExpected to reuse:\n" + << ClangUtil::DumpDecl(expected); + } +}; + +TEST_F(TestCreateClassTemplateDecl, FindExistingTemplates) { + // This tests the logic in TypeSystemClang::CreateClassTemplateDecl that + // decides whether an existing ClassTemplateDecl in the AST can be reused. + // The behaviour should follow the C++ rules for redeclaring templates + // (e.g., parameter names can be changed/omitted.) + + // This describes a class template *instantiation* from which we will infer + // the structure of the class template. + TypeSystemClang::TemplateParameterInfos infos; + + // Test an empty template parameter list: <> + ExpectNewTemplate("<>", infos); + + // Test that with T = int creates a new template. + infos.names = {"T"}; + infos.args = {TemplateArgument(m_ast->getASTContext().IntTy)}; + ClassTemplateDecl *single_type_arg = ExpectNewTemplate("", infos); + + // Test that changing the parameter name doesn't create a new class template. + infos.names = {"A"}; + ExpectReusedTemplate(" (A = int)", infos, single_type_arg); + + // Test that changing the used type doesn't create a new class template. + infos.args = {TemplateArgument(m_ast->getASTContext().FloatTy)}; + ExpectReusedTemplate(" (A = float)", infos, single_type_arg); + + // Test that creates a new template with A = int + // and I = 47; + infos.names.push_back("I"); + infos.args.push_back(TemplateArgument(m_ast->getASTContext(), + llvm::APSInt(llvm::APInt(8, 47)), + m_ast->getASTContext().SignedCharTy)); + ClassTemplateDecl *type_and_char_value = + ExpectNewTemplate(" (I = 47)", infos); + + // Change the value of the I parameter to 123. The previously created + // class template should still be reused. + infos.args.pop_back(); + infos.args.push_back(TemplateArgument(m_ast->getASTContext(), + llvm::APSInt(llvm::APInt(8, 123)), + m_ast->getASTContext().SignedCharTy)); + ExpectReusedTemplate(" (I = 123)", infos, + type_and_char_value); + + // Change the type of the I parameter to int so we have . + // The class template from above can't be reused. + infos.args.pop_back(); + infos.args.push_back(TemplateArgument(m_ast->getASTContext(), + llvm::APSInt(llvm::APInt(32, 47)), + m_ast->getASTContext().IntTy)); + ExpectNewTemplate(" (I = 123)", infos); + + // Test a second type parameter will also cause a new template to be created. + // We now have . + infos.names.push_back("B"); + infos.args.push_back(TemplateArgument(m_ast->getASTContext().IntTy)); + ClassTemplateDecl *type_and_char_value_and_type = + ExpectNewTemplate("", infos); + + // Remove all the names from the parameters which shouldn't influence the + // way the templates get merged. + infos.names = {"", "", ""}; + ExpectReusedTemplate("", infos, + type_and_char_value_and_type); +} + +TEST_F(TestCreateClassTemplateDecl, FindExistingTemplatesWithParameterPack) { + // The same as FindExistingTemplates but for templates with parameter packs. + + TypeSystemClang::TemplateParameterInfos infos; + infos.packed_args = + std::make_unique(); + infos.packed_args->names = {"", ""}; + infos.packed_args->args = {TemplateArgument(m_ast->getASTContext().IntTy), + TemplateArgument(m_ast->getASTContext().IntTy)}; + ClassTemplateDecl *type_pack = + ExpectNewTemplate(" (int, int)", infos); + + // Special case: An instantiation for a parameter pack with no values fits + // to whatever class template we find. There isn't enough information to + // do an actual comparison here. + infos.packed_args = + std::make_unique(); + ExpectReusedTemplate("<...> (no values in pack)", infos, type_pack); + + // Change the type content of pack type values. + infos.packed_args->names = {"", ""}; + infos.packed_args->args = {TemplateArgument(m_ast->getASTContext().IntTy), + TemplateArgument(m_ast->getASTContext().LongTy)}; + ExpectReusedTemplate(" (int, long)", infos, type_pack); + + // Change the number of pack values. + infos.packed_args->args = {TemplateArgument(m_ast->getASTContext().IntTy)}; + ExpectReusedTemplate(" (int)", infos, type_pack); + + // The names of the pack values shouldn't matter. + infos.packed_args->names = {"A", "B"}; + ExpectReusedTemplate(" (int)", infos, type_pack); + + // Changing the kind of template argument will create a new template. + infos.packed_args->args = {TemplateArgument(m_ast->getASTContext(), + llvm::APSInt(llvm::APInt(32, 1)), + m_ast->getASTContext().IntTy)}; + ClassTemplateDecl *int_pack = ExpectNewTemplate(" (int = 1)", infos); + + // Changing the value of integral parameters will not create a new template. + infos.packed_args->args = {TemplateArgument( + m_ast->getASTContext(), llvm::APSInt(llvm::APInt(32, 123)), + m_ast->getASTContext().IntTy)}; + ExpectReusedTemplate(" (int = 123)", infos, int_pack); + + // Changing the integral type will create a new template. + infos.packed_args->args = {TemplateArgument(m_ast->getASTContext(), + llvm::APSInt(llvm::APInt(64, 1)), + m_ast->getASTContext().LongTy)}; + ExpectNewTemplate(" (long = 1)", infos); + + // Prependinding a non-pack parameter will create a new template. + infos.names = {"T"}; + infos.args = {TemplateArgument(m_ast->getASTContext().IntTy)}; + ExpectNewTemplate(" (T = int, long = 1)", infos); +} + TEST_F(TestTypeSystemClang, OnlyPackName) { TypeSystemClang::TemplateParameterInfos infos; infos.pack_name = "A";