https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/127829
>From 1aabc8a93ffac06755cbaf5e6c1fa8913cd63729 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Wed, 19 Feb 2025 12:47:30 +0000 Subject: [PATCH 1/3] [lldb][TypeSystemClang] Create MainFileID for TypeSystemClang ASTContexts --- .../TypeSystem/Clang/TypeSystemClang.cpp | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index 1e0c7f0514941..563961b9a4971 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -13,6 +13,7 @@ #include "llvm/Support/Casting.h" #include "llvm/Support/FormatAdapters.h" #include "llvm/Support/FormatVariadic.h" +#include "llvm/Support/SmallVectorMemoryBuffer.h" #include <mutex> #include <memory> @@ -361,6 +362,39 @@ static void SetMemberOwningModule(clang::Decl *member, } } +/// Creates a dummy main file for the given SourceManager. +/// This file only serves as a container for include locations to other +/// FileIDs that are put into this type system (either by the ASTImporter +/// or when TypeSystemClang generates source locations for declarations). +/// This file is not reflected to disk. +static clang::FileID CreateDummyMainFile(clang::SourceManager &sm, + clang::FileManager &fm) { + llvm::StringRef main_file_path = "<LLDB Dummy Main File>"; + // The file contents are empty and should never be seen by the user. The new + // line is just there to not throw off any line counting logic that might + // expect files to end with a newline. + llvm::StringRef main_file_contents = "\n"; + const time_t mod_time = 0; + const off_t file_size = static_cast<off_t>(main_file_contents.size()); + + // Create a virtual FileEntry for our dummy file. + auto fe = fm.getVirtualFileRef(main_file_path, file_size, mod_time); + + // Overwrite the file buffer with our empty file contents. + llvm::SmallVector<char, 64> buffer; + buffer.append(main_file_contents.begin(), main_file_contents.end()); + auto file_contents = std::make_unique<llvm::SmallVectorMemoryBuffer>( + std::move(buffer), main_file_path); + sm.overrideFileContents(fe, std::move(file_contents)); + + // Create the actual file id for the FileEntry and set it as the main file. + clang::FileID fid = + sm.createFileID(fe, SourceLocation(), clang::SrcMgr::C_User); + sm.setMainFileID(fid); + + return fid; +} + char TypeSystemClang::ID; bool TypeSystemClang::IsOperator(llvm::StringRef name, @@ -692,6 +726,11 @@ void TypeSystemClang::CreateASTContext() { m_diagnostic_consumer_up = std::make_unique<NullDiagnosticConsumer>(); m_ast_up->getDiagnostics().setClient(m_diagnostic_consumer_up.get(), false); + // Set up the MainFileID of this ASTContext. All other FileIDs created + // by this TypeSystem will act as-if included into this dummy main file. + auto fid = CreateDummyMainFile(*m_source_manager_up, *m_file_manager_up); + assert(m_ast_up->getSourceManager().getMainFileID() == fid); + // This can be NULL if we don't know anything about the architecture or if // the target for an architecture isn't enabled in the llvm/clang that we // built >From 5d4cbd3e306f02eee80b4fd614b9e155f75d3d1d Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Wed, 19 Feb 2025 13:37:01 +0000 Subject: [PATCH 2/3] [lldb][TypeSystemClang] Set location on functions, parameters, enums and structures --- .../SymbolFile/DWARF/DWARFASTParserClang.cpp | 10 ++-- .../SymbolFile/NativePDB/PdbAstBuilder.cpp | 2 +- .../Plugins/SymbolFile/PDB/PDBASTParser.cpp | 3 +- .../TypeSystem/Clang/TypeSystemClang.cpp | 48 ++++++++++++++----- .../TypeSystem/Clang/TypeSystemClang.h | 46 +++++++++++------- lldb/unittests/Symbol/TestTypeSystemClang.cpp | 7 +-- 6 files changed, 78 insertions(+), 38 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 2d4d22559963f..abf3b22b0ae15 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -1372,7 +1372,7 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die, ignore_containing_context ? m_ast.GetTranslationUnitDecl() : containing_decl_ctx, GetOwningClangModule(die), name, clang_type, attrs.storage, - attrs.is_inline); + attrs.is_inline, attrs.decl); std::free(name_buf); if (has_template_params) { @@ -1382,11 +1382,11 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die, ignore_containing_context ? m_ast.GetTranslationUnitDecl() : containing_decl_ctx, GetOwningClangModule(die), attrs.name.GetStringRef(), clang_type, - attrs.storage, attrs.is_inline); + attrs.storage, attrs.is_inline, attrs.decl); clang::FunctionTemplateDecl *func_template_decl = m_ast.CreateFunctionTemplateDecl( containing_decl_ctx, GetOwningClangModule(die), - template_function_decl, template_param_infos); + template_function_decl, template_param_infos, attrs.decl); m_ast.CreateFunctionTemplateSpecializationInfo( template_function_decl, func_template_decl, template_param_infos); } @@ -1858,7 +1858,7 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc, clang::ClassTemplateSpecializationDecl *class_specialization_decl = m_ast.CreateClassTemplateSpecializationDecl( containing_decl_ctx, GetOwningClangModule(die), class_template_decl, - tag_decl_kind, template_param_infos); + tag_decl_kind, template_param_infos, attrs.decl); clang_type = m_ast.CreateClassTemplateSpecializationType(class_specialization_decl); @@ -1870,7 +1870,7 @@ DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc, clang_type = m_ast.CreateRecordType( containing_decl_ctx, GetOwningClangModule(die), attrs.accessibility, attrs.name.GetCString(), tag_decl_kind, attrs.class_language, metadata, - attrs.exports_symbols); + attrs.exports_symbols, attrs.decl); } TypeSP type_sp = dwarf->MakeType( diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp index 5d4b22d08b111..ecb1a7dc571b4 100644 --- a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp +++ b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp @@ -1128,7 +1128,7 @@ void PdbAstBuilder::CreateFunctionParameters(PdbCompilandSymId func_id, CompilerType param_type_ct = m_clang.GetType(qt); clang::ParmVarDecl *param = m_clang.CreateParameterDeclaration( &function_decl, OptionalClangModuleID(), param_name.str().c_str(), - param_type_ct, clang::SC_None, true); + param_type_ct, clang::SC_None, clang::SourceLocation(), true); lldbassert(m_uid_to_decl.count(toOpaqueUid(param_uid)) == 0); m_uid_to_decl[toOpaqueUid(param_uid)] = param; diff --git a/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp b/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp index c6dd72e22fb4c..e98ccf87cc2f7 100644 --- a/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp +++ b/lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp @@ -969,7 +969,8 @@ PDBASTParser::GetDeclForSymbol(const llvm::pdb::PDBSymbol &symbol) { clang::ParmVarDecl *param = m_ast.CreateParameterDeclaration( decl, OptionalClangModuleID(), nullptr, - arg_type->GetForwardCompilerType(), clang::SC_None, true); + arg_type->GetForwardCompilerType(), clang::SC_None, + clang::SourceLocation(), true); if (param) params.push_back(param); } diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index 563961b9a4971..c9a2c33567aaf 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -1282,7 +1282,7 @@ CompilerType TypeSystemClang::CreateRecordType( clang::DeclContext *decl_ctx, OptionalClangModuleID owning_module, AccessType access_type, llvm::StringRef name, int kind, LanguageType language, std::optional<ClangASTMetadata> metadata, - bool exports_symbols) { + bool exports_symbols, const Declaration &declaration) { ASTContext &ast = getASTContext(); if (decl_ctx == nullptr) @@ -1337,6 +1337,10 @@ CompilerType TypeSystemClang::CreateRecordType( decl->setAnonymousStructOrUnion(true); } + auto location = GetLocForDecl(declaration); + decl->setLocStart(location); + decl->setLocation(location); + if (metadata) SetMetadata(decl, *metadata); @@ -1454,7 +1458,8 @@ static TemplateParameterList *CreateTemplateParameterList( clang::FunctionTemplateDecl *TypeSystemClang::CreateFunctionTemplateDecl( clang::DeclContext *decl_ctx, OptionalClangModuleID owning_module, clang::FunctionDecl *func_decl, - const TemplateParameterInfos &template_param_infos) { + const TemplateParameterInfos &template_param_infos, + const Declaration &declaration) { // /// Create a function template node. ASTContext &ast = getASTContext(); @@ -1468,6 +1473,7 @@ clang::FunctionTemplateDecl *TypeSystemClang::CreateFunctionTemplateDecl( func_tmpl_decl->setDeclName(func_decl->getDeclName()); func_tmpl_decl->setTemplateParameters(template_param_list); func_tmpl_decl->init(func_decl); + func_tmpl_decl->setLocation(GetLocForDecl(declaration)); SetOwningModule(func_tmpl_decl, owning_module); for (size_t i = 0, template_param_decl_count = template_param_decls.size(); @@ -1693,7 +1699,8 @@ ClassTemplateSpecializationDecl * TypeSystemClang::CreateClassTemplateSpecializationDecl( DeclContext *decl_ctx, OptionalClangModuleID owning_module, ClassTemplateDecl *class_template_decl, int kind, - const TemplateParameterInfos &template_param_infos) { + const TemplateParameterInfos &template_param_infos, + const Declaration &declaration) { ASTContext &ast = getASTContext(); llvm::SmallVector<clang::TemplateArgument, 2> args( template_param_infos.Size() + @@ -1728,6 +1735,8 @@ TypeSystemClang::CreateClassTemplateSpecializationDecl( class_template_specialization_decl->setSpecializationKind( TSK_ExplicitSpecialization); + class_template_specialization_decl->setLocation(GetLocForDecl(declaration)); + return class_template_specialization_decl; } @@ -2188,7 +2197,8 @@ std::string TypeSystemClang::GetTypeNameForDecl(const NamedDecl *named_decl, FunctionDecl *TypeSystemClang::CreateFunctionDeclaration( clang::DeclContext *decl_ctx, OptionalClangModuleID owning_module, llvm::StringRef name, const CompilerType &function_clang_type, - clang::StorageClass storage, bool is_inline) { + clang::StorageClass storage, bool is_inline, + const Declaration &declaration) { FunctionDecl *func_decl = nullptr; ASTContext &ast = getASTContext(); if (!decl_ctx) @@ -2209,6 +2219,11 @@ FunctionDecl *TypeSystemClang::CreateFunctionDeclaration( func_decl->setConstexprKind(isConstexprSpecified ? ConstexprSpecKind::Constexpr : ConstexprSpecKind::Unspecified); + + const clang::SourceLocation location = GetLocForDecl(declaration); + func_decl->setLocation(location); + func_decl->setRangeEnd(location); + SetOwningModule(func_decl, owning_module); decl_ctx->addDecl(func_decl); @@ -2258,7 +2273,7 @@ CompilerType TypeSystemClang::CreateFunctionType( ParmVarDecl *TypeSystemClang::CreateParameterDeclaration( clang::DeclContext *decl_ctx, OptionalClangModuleID owning_module, const char *name, const CompilerType ¶m_type, int storage, - bool add_decl) { + clang::SourceLocation loc, bool add_decl) { ASTContext &ast = getASTContext(); auto *decl = ParmVarDecl::CreateDeserialized(ast, GlobalDeclID()); decl->setDeclContext(decl_ctx); @@ -2266,6 +2281,7 @@ ParmVarDecl *TypeSystemClang::CreateParameterDeclaration( decl->setDeclName(&ast.Idents.get(name)); decl->setType(ClangUtil::GetQualType(param_type)); decl->setStorageClass(static_cast<clang::StorageClass>(storage)); + decl->setLocation(loc); SetOwningModule(decl, owning_module); if (add_decl) decl_ctx->addDecl(decl); @@ -2355,10 +2371,10 @@ CompilerType TypeSystemClang::CreateEnumerationType( OptionalClangModuleID owning_module, const Declaration &decl, const CompilerType &integer_clang_type, bool is_scoped, std::optional<clang::EnumExtensibilityAttr::Kind> enum_kind) { - // TODO: Do something intelligent with the Declaration object passed in - // like maybe filling in the SourceLocation with it... ASTContext &ast = getASTContext(); + auto location = GetLocForDecl(decl); + // TODO: ask about these... // const bool IsFixed = false; EnumDecl *enum_decl = EnumDecl::CreateDeserialized(ast, GlobalDeclID()); @@ -2368,6 +2384,8 @@ CompilerType TypeSystemClang::CreateEnumerationType( enum_decl->setScoped(is_scoped); enum_decl->setScopedUsingClassTag(is_scoped); enum_decl->setFixed(false); + enum_decl->setLocation(location); + enum_decl->setLocStart(location); SetOwningModule(enum_decl, owning_module); if (decl_ctx) decl_ctx->addDecl(enum_decl); @@ -7794,10 +7812,11 @@ TypeSystemClang::CreateParameterDeclarations( llvm::StringRef name = !parameter_names.empty() ? parameter_names[param_index] : ""; - auto *param = - CreateParameterDeclaration(func, /*owning_module=*/{}, name.data(), - GetType(prototype.getParamType(param_index)), - clang::SC_None, /*add_decl=*/false); + // FIXME: we should pass the location of the parameter not the function + auto *param = CreateParameterDeclaration( + func, /*owning_module=*/{}, name.data(), + GetType(prototype.getParamType(param_index)), clang::SC_None, + func->getLocation(), /*add_decl=*/false); assert(param); params.push_back(param); @@ -7810,7 +7829,8 @@ clang::CXXMethodDecl *TypeSystemClang::AddMethodToCXXRecordType( lldb::opaque_compiler_type_t type, llvm::StringRef name, const char *mangled_name, const CompilerType &method_clang_type, lldb::AccessType access, bool is_virtual, bool is_static, bool is_inline, - bool is_explicit, bool is_attr_used, bool is_artificial) { + bool is_explicit, bool is_attr_used, bool is_artificial, + const Declaration &declaration) { if (!type || !method_clang_type.IsValid() || name.empty()) return nullptr; @@ -7951,6 +7971,10 @@ clang::CXXMethodDecl *TypeSystemClang::AddMethodToCXXRecordType( cxx_method_decl->setParams(CreateParameterDeclarations( cxx_method_decl, *method_function_prototype, /*parameter_names=*/{})); + const clang::SourceLocation location = GetLocForDecl(declaration); + cxx_method_decl->setLocation(location); + cxx_method_decl->setRangeEnd(location); + AddAccessSpecifierDecl(cxx_record_decl, getASTContext(), GetCXXRecordDeclAccess(cxx_record_decl), access_specifier); diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h index 99d9becffd128..1e3b5a46e86d3 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h @@ -326,13 +326,12 @@ class TypeSystemClang : public TypeSystem { bool is_framework = false, bool is_explicit = false); - CompilerType - CreateRecordType(clang::DeclContext *decl_ctx, - OptionalClangModuleID owning_module, - lldb::AccessType access_type, llvm::StringRef name, int kind, - lldb::LanguageType language, - std::optional<ClangASTMetadata> metadata = std::nullopt, - bool exports_symbols = false); + CompilerType CreateRecordType( + clang::DeclContext *decl_ctx, OptionalClangModuleID owning_module, + lldb::AccessType access_type, llvm::StringRef name, int kind, + lldb::LanguageType language, + std::optional<ClangASTMetadata> metadata = std::nullopt, + bool exports_symbols = false, const Declaration &declaration = {}); class TemplateParameterInfos { public: @@ -420,7 +419,8 @@ class TypeSystemClang : public TypeSystem { clang::FunctionTemplateDecl *CreateFunctionTemplateDecl( clang::DeclContext *decl_ctx, OptionalClangModuleID owning_module, - clang::FunctionDecl *func_decl, const TemplateParameterInfos &infos); + clang::FunctionDecl *func_decl, const TemplateParameterInfos &infos, + const Declaration &declaration); void CreateFunctionTemplateSpecializationInfo( clang::FunctionDecl *func_decl, clang::FunctionTemplateDecl *Template, @@ -437,7 +437,7 @@ class TypeSystemClang : public TypeSystem { clang::ClassTemplateSpecializationDecl *CreateClassTemplateSpecializationDecl( clang::DeclContext *decl_ctx, OptionalClangModuleID owning_module, clang::ClassTemplateDecl *class_template_decl, int kind, - const TemplateParameterInfos &infos); + const TemplateParameterInfos &infos, const Declaration &declaration); CompilerType CreateClassTemplateSpecializationType(clang::ClassTemplateSpecializationDecl * @@ -476,7 +476,8 @@ class TypeSystemClang : public TypeSystem { clang::FunctionDecl *CreateFunctionDeclaration( clang::DeclContext *decl_ctx, OptionalClangModuleID owning_module, llvm::StringRef name, const CompilerType &function_Type, - clang::StorageClass storage, bool is_inline); + clang::StorageClass storage, bool is_inline, + const Declaration &declaration = {}); CompilerType CreateFunctionType(const CompilerType &result_type, const CompilerType *args, @@ -484,11 +485,10 @@ class TypeSystemClang : public TypeSystem { clang::CallingConv cc = clang::CC_C, clang::RefQualifierKind ref_qual = clang::RQ_None); - clang::ParmVarDecl * - CreateParameterDeclaration(clang::DeclContext *decl_ctx, - OptionalClangModuleID owning_module, - const char *name, const CompilerType ¶m_type, - int storage, bool add_decl = false); + clang::ParmVarDecl *CreateParameterDeclaration( + clang::DeclContext *decl_ctx, OptionalClangModuleID owning_module, + const char *name, const CompilerType ¶m_type, int storage, + clang::SourceLocation loc, bool add_decl = false); CompilerType CreateBlockPointerType(const CompilerType &function_type); @@ -996,7 +996,8 @@ class TypeSystemClang : public TypeSystem { lldb::opaque_compiler_type_t type, llvm::StringRef name, const char *mangled_name, const CompilerType &method_type, lldb::AccessType access, bool is_virtual, bool is_static, bool is_inline, - bool is_explicit, bool is_attr_used, bool is_artificial); + bool is_explicit, bool is_attr_used, bool is_artificial, + const Declaration &declaration = {}); void AddMethodOverridesForCXXRecordType(lldb::opaque_compiler_type_t type); @@ -1188,6 +1189,19 @@ class TypeSystemClang : public TypeSystem { std::optional<uint64_t> GetObjCBitSize(clang::QualType qual_type, ExecutionContextScope *exe_scope); + /// Turns the given \c decl into a \c clang::SourceLocation. + /// + /// Will create a \c FileID in this \c DWARFASTParserClang's \c ASTContext + /// if necessary. + /// + /// If no \c FileID could be found/created, returns an empty \c + /// SourceLocation. + /// + /// FIXME: currently a no-op. + clang::SourceLocation GetLocForDecl(const lldb_private::Declaration &decl) { + return {}; + } + // Classes that inherit from TypeSystemClang can see and modify these std::string m_target_triple; std::unique_ptr<clang::ASTContext> m_ast_up; diff --git a/lldb/unittests/Symbol/TestTypeSystemClang.cpp b/lldb/unittests/Symbol/TestTypeSystemClang.cpp index a9b0c87c4fbce..4dab1bea477b4 100644 --- a/lldb/unittests/Symbol/TestTypeSystemClang.cpp +++ b/lldb/unittests/Symbol/TestTypeSystemClang.cpp @@ -545,7 +545,8 @@ TEST_F(TestTypeSystemClang, TemplateArguments) { ClassTemplateSpecializationDecl *spec_decl = m_ast->CreateClassTemplateSpecializationDecl( m_ast->GetTranslationUnitDecl(), OptionalClangModuleID(), decl, - llvm::to_underlying(clang::TagTypeKind::Struct), infos); + llvm::to_underlying(clang::TagTypeKind::Struct), infos, + Declaration()); ASSERT_NE(spec_decl, nullptr); CompilerType type = m_ast->CreateClassTemplateSpecializationType(spec_decl); ASSERT_TRUE(type); @@ -876,7 +877,7 @@ TEST_F(TestTypeSystemClang, TestFunctionTemplateConstruction) { // Create the actual function template. clang::FunctionTemplateDecl *func_template = m_ast->CreateFunctionTemplateDecl(TU, OptionalClangModuleID(), func, - empty_params); + empty_params, Declaration()); EXPECT_EQ(TU, func_template->getDeclContext()); EXPECT_EQ("foo", func_template->getName()); @@ -908,7 +909,7 @@ TEST_F(TestTypeSystemClang, TestFunctionTemplateInRecordConstruction) { // Create the actual function template. clang::FunctionTemplateDecl *func_template = m_ast->CreateFunctionTemplateDecl(record, OptionalClangModuleID(), func, - empty_params); + empty_params, Declaration()); EXPECT_EQ(record, func_template->getDeclContext()); EXPECT_EQ("foo", func_template->getName()); >From a05a2ca8f8bcf3fb9017d54248fbd32b2e4797b3 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Wed, 19 Feb 2025 16:42:37 +0000 Subject: [PATCH 3/3] [lldb][TypeSystemClang] Create FileIDs/SourceLocations from DWARF --- .../TypeSystem/Clang/TypeSystemClang.cpp | 33 +++++ .../TypeSystem/Clang/TypeSystemClang.h | 22 ++- .../diagnostics/TestExprDiagnostics.py | 127 +++++++++++++++++- .../commands/expression/diagnostics/main.cpp | 11 ++ .../TestExprInsideLambdas.py | 23 ++-- .../TestCppFunctionLocalClass.py | 12 +- .../Shell/Settings/TestFrameFormatName.test | 2 +- ...ast-from-dwarf-unamed-and-anon-structs.cpp | 4 +- lldb/unittests/Symbol/CMakeLists.txt | 3 + lldb/unittests/Symbol/Inputs/dummy.cpp | 9 ++ lldb/unittests/Symbol/TestTypeSystemClang.cpp | 105 +++++++++++++++ 11 files changed, 309 insertions(+), 42 deletions(-) create mode 100644 lldb/unittests/Symbol/Inputs/dummy.cpp diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index c9a2c33567aaf..81654e43d2538 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -11,6 +11,7 @@ #include "clang/AST/DeclBase.h" #include "clang/AST/ExprCXX.h" #include "llvm/Support/Casting.h" +#include "llvm/Support/Error.h" #include "llvm/Support/FormatAdapters.h" #include "llvm/Support/FormatVariadic.h" #include "llvm/Support/SmallVectorMemoryBuffer.h" @@ -9945,3 +9946,35 @@ void TypeSystemClang::LogCreation() const { LLDB_LOG(log, "Created new TypeSystem for (ASTContext*){0:x} '{1}'", &getASTContext(), getDisplayName()); } + +clang::SourceLocation TypeSystemClang::GetLocForDecl(const Declaration &decl) { + // If the Declaration is invalid there is nothing to do. + if (!decl.IsValid()) + return {}; + + clang::SourceManager &sm = getASTContext().getSourceManager(); + clang::FileManager &fm = sm.getFileManager(); + + auto fe = fm.getFileRef(decl.GetFile().GetPath()); + if (!fe) { + llvm::consumeError(fe.takeError()); + return {}; + } + + clang::FileID fid = sm.translateFile(*fe); + if (fid.isInvalid()) { + // We see the file for the first time, so create a dummy file for it now. + + // Connect the new dummy file to the main file via some fake include + // location. This is necessary as all file's in the SourceManager need to be + // reachable via an include chain from the main file. + SourceLocation ToIncludeLocOrFakeLoc; + assert(sm.getMainFileID().isValid()); + ToIncludeLocOrFakeLoc = sm.getLocForStartOfFile(sm.getMainFileID()); + fid = sm.createFileID(*fe, ToIncludeLocOrFakeLoc, clang::SrcMgr::C_User); + } + + // Clang requires column numbers to be >= 1.. + return sm.translateLineCol(fid, decl.GetLine(), + std::max<uint16_t>(decl.GetColumn(), 1)); +} diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h index 1e3b5a46e86d3..9d0e83e40f657 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h @@ -1165,6 +1165,15 @@ class TypeSystemClang : public TypeSystem { bool SetDeclIsForcefullyCompleted(const clang::TagDecl *td); + /// Turns the given \c decl into a \c clang::SourceLocation. + /// + /// Will create a \c FileID in this \c DWARFASTParserClang's \c ASTContext + /// if necessary. + /// + /// If no \c FileID could be found/created, returns an empty \c + /// SourceLocation. + clang::SourceLocation GetLocForDecl(const lldb_private::Declaration &decl); + private: /// Returns the PrintingPolicy used when generating the internal type names. /// These type names are mostly used for the formatter selection. @@ -1189,19 +1198,6 @@ class TypeSystemClang : public TypeSystem { std::optional<uint64_t> GetObjCBitSize(clang::QualType qual_type, ExecutionContextScope *exe_scope); - /// Turns the given \c decl into a \c clang::SourceLocation. - /// - /// Will create a \c FileID in this \c DWARFASTParserClang's \c ASTContext - /// if necessary. - /// - /// If no \c FileID could be found/created, returns an empty \c - /// SourceLocation. - /// - /// FIXME: currently a no-op. - clang::SourceLocation GetLocForDecl(const lldb_private::Declaration &decl) { - return {}; - } - // Classes that inherit from TypeSystemClang can see and modify these std::string m_target_triple; std::unique_ptr<clang::ASTContext> m_ast_up; diff --git a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py index b476a807c6dc0..6716c3d39572c 100644 --- a/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py +++ b/lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py @@ -15,6 +15,8 @@ def setUp(self): self.main_source = "main.cpp" self.main_source_spec = lldb.SBFileSpec(self.main_source) + self.main_source_diag_path = os.path.join("diagnostics", "main.cpp") + self.header_source_diag_path = os.path.join("diagnostics", "lib.h") def test_source_and_caret_printing(self): """Test that the source and caret positions LLDB prints are correct""" @@ -113,9 +115,6 @@ def test_source_and_caret_printing(self): value.GetError().GetCString(), ) - # Declarations from the debug information currently have no debug information. It's not clear what - # we should do in this case, but we should at least not print anything that's wrong. - # In the future our declarations should have valid source locations. value = frame.EvaluateExpression("struct FooBar { double x };", top_level_opts) self.assertFalse(value.GetError().Success()) self.assertIn( @@ -140,10 +139,13 @@ def test_source_and_caret_printing(self): """ 1 | foo(1, 2) | ^~~ -note: candidate function not viable: requires single argument 'x', but 2 arguments were provided """, value.GetError().GetCString(), ) + self.assertIn( + "candidate function not viable: requires single argument 'x', but 2 arguments were provided", + value.GetError().GetCString(), + ) # Redefine something that we defined in a user-expression. We should use the previous expression file name # for the original decl. @@ -260,3 +262,120 @@ def check_error(diags): self.assertEqual(err_ty.GetIntegerValue(), lldb.eErrorTypeExpression) diags = data.GetValueForKey("errors").GetItemAtIndex(0) check_error(diags) + + def test_source_locations_from_debug_information(self): + """Test that the source locations from debug information are correct""" + self.build() + + (_, _, thread, _) = lldbutil.run_to_source_breakpoint( + self, "// Break here", self.main_source_spec + ) + + frame = thread.GetFrameAtIndex(0) + + # Test source locations of functions + value = frame.EvaluateExpression("foo(1, 2)") + self.assertFalse(value.GetError().Success()) + error_msg = value.GetError().GetCString() + self.assertIn( + """:1:1: no matching function for call to 'foo' + 1 | foo(1, 2) + | ^~~ +""", + error_msg, + ) + + self.assertIn( + """ + 1 | void foo() {} + | ^ +""", + error_msg, + ) + + self.assertIn( + """ + 2 | void foo(char, char const* x) {} + | ^ +""", + error_msg, + ) + + self.assertIn( + """ + 3 | void foo(int x) {} + | ^ +""", + error_msg, + ) + + self.assertIn(f"{self.header_source_diag_path}:", error_msg) + self.assertIn(f"{self.main_source_diag_path}:", error_msg) + + top_level_opts = lldb.SBExpressionOptions() + top_level_opts.SetTopLevel(True) + + # Test source locations of records + value = frame.EvaluateExpression("struct FooBar { double x; };", top_level_opts) + self.assertFalse(value.GetError().Success()) + error_msg = value.GetError().GetCString() + self.assertIn( + """:1:8: redefinition of 'FooBar' + 1 | struct FooBar { double x; }; +""", + error_msg, + ) + self.assertIn( + """previous definition is here + 5 | struct FooBar { + | ^ +""", + error_msg, + ) + self.assertIn(f"{self.main_source_diag_path}:", error_msg) + + # Test source locations of enums + value = frame.EvaluateExpression("enum class EnumInSource {};", top_level_opts) + self.assertFalse(value.GetError().Success()) + error_msg = value.GetError().GetCString() + self.assertIn( + """previous declaration is here + 9 | enum class EnumInSource { A, B, C }; + | ^ +""", + error_msg, + ) + self.assertIn(f"{self.main_source_diag_path}:", error_msg) + + # TODO: add tests for source snippet display from files (with invalid and valid files)...temporarily move header from python? + # TODO: add test for DW_AT_decl_column (unit-test?) + # TODO: add test for checksum mismatch + + @skipIf( + debug_info=no_match("dsym"), + bugnumber="Template function decl can only be found via dsym", + ) + def test_source_locations_from_debug_information_template_func(self): + """Test that the source locations from debug information are correct + for template functions""" + self.build() + + (_, _, thread, _) = lldbutil.run_to_source_breakpoint( + self, "// Break here", self.main_source_spec + ) + + frame = thread.GetFrameAtIndex(0) + + # Test source locations of template functions + value = frame.EvaluateExpression("templateFunc<int>(1)") + self.assertFalse(value.GetError().Success()) + error_msg = value.GetError().GetCString() + self.assertIn( + """11:1: non-template declaration found by name lookup + 11 | template <typename T> void templateFunc() {} + | ^ +""", + error_msg, + ) + + self.assertIn(f"{self.main_source_diag_path}:", error_msg) diff --git a/lldb/test/API/commands/expression/diagnostics/main.cpp b/lldb/test/API/commands/expression/diagnostics/main.cpp index f4ad1ad220cf8..e420bfc0dd628 100644 --- a/lldb/test/API/commands/expression/diagnostics/main.cpp +++ b/lldb/test/API/commands/expression/diagnostics/main.cpp @@ -1,11 +1,22 @@ +#include "lib.h" + void foo(int x) {} struct FooBar { int i; }; +enum class EnumInSource { A, B, C }; + +template <typename T> void templateFunc() {} + int main() { FooBar f; foo(1); + foo('c', "abc"); + foo(); + EnumInSource e = EnumInSource::A; + templateFunc<int>(); + return 0; // Break here } diff --git a/lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py b/lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py index e35cfa6a289a7..682970fb1bda8 100644 --- a/lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py +++ b/lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py @@ -10,11 +10,12 @@ class ExprInsideLambdaTestCase(TestBase): - def expectExprError(self, expr: str, expected: str): + def expectExprError(self, expr: str, expected_errors: str): frame = self.thread.GetFrameAtIndex(0) value = frame.EvaluateExpression(expr) errmsg = value.GetError().GetCString() - self.assertIn(expected, errmsg) + for expected in expected_errors: + self.assertIn(expected, errmsg) def test_expr_inside_lambda(self): """Test that lldb evaluating expressions inside lambda expressions works correctly.""" @@ -76,7 +77,7 @@ def test_expr_inside_lambda(self): self.expect_expr("local_var", result_type="int", result_value="137") # Check access to variable in previous frame which we didn't capture - self.expectExprError("local_var_copy", "use of undeclared identifier") + self.expectExprError("local_var_copy", ["use of undeclared identifier"]) lldbutil.continue_to_breakpoint(process, bkpt) @@ -110,19 +111,17 @@ def test_expr_inside_lambda(self): # Check access to outer top-level structure's members self.expectExprError( "class_var", - ("use of non-static data member" " 'class_var' of 'Foo' from nested type"), + ["use of non-static data member 'class_var' of 'Foo' from nested type"], ) - self.expectExprError( - "base_var", ("use of non-static data member" " 'base_var'") - ) + self.expectExprError("base_var", ["use of non-static data member 'base_var'"]) self.expectExprError( "local_var", - ( - "use of non-static data member 'local_var'" - " of '(unnamed class)' from nested type 'LocalLambdaClass'" - ), + [ + "use of non-static data member 'local_var' of '(unnamed class at", + "from nested type 'LocalLambdaClass'", + ], ) # Inside non_capturing_method @@ -133,5 +132,5 @@ def test_expr_inside_lambda(self): self.expectExprError( "class_var", - ("use of non-static data member" " 'class_var' of 'Foo' from nested type"), + ["use of non-static data member 'class_var' of 'Foo' from nested type"], ) diff --git a/lldb/test/API/lang/cpp/function-local-class/TestCppFunctionLocalClass.py b/lldb/test/API/lang/cpp/function-local-class/TestCppFunctionLocalClass.py index ab81411a9f230..64664c7105ade 100644 --- a/lldb/test/API/lang/cpp/function-local-class/TestCppFunctionLocalClass.py +++ b/lldb/test/API/lang/cpp/function-local-class/TestCppFunctionLocalClass.py @@ -34,16 +34,8 @@ def test(self): result_type="TypedefUnnamed2", result_children=[ValueCheck(name="b", value="3")], ) - self.expect_expr( - "unnamed", - result_type="(unnamed struct)", - result_children=[ValueCheck(name="i", value="4")], - ) - self.expect_expr( - "unnamed2", - result_type="(unnamed struct)", - result_children=[ValueCheck(name="j", value="5")], - ) + self.expect("expression unnamed", substrs=["(unnamed struct at", "i = 4"]) + self.expect("expression unnamed2", substrs=["(unnamed struct at", "j = 5"]) # Try a class that is only forward declared. self.expect_expr("fwd", result_type="Forward *") diff --git a/lldb/test/Shell/Settings/TestFrameFormatName.test b/lldb/test/Shell/Settings/TestFrameFormatName.test index caa3242527c6e..8ad3b37c90573 100644 --- a/lldb/test/Shell/Settings/TestFrameFormatName.test +++ b/lldb/test/Shell/Settings/TestFrameFormatName.test @@ -16,7 +16,7 @@ run c # NAME_WITH_ARGS: frame int ns::foo<int ()>(str="bar") c -# NAME_WITH_ARGS: frame int ns::foo<(anonymous namespace)::$_0>(t=(anonymous namespace)::(unnamed class) @ {{.*}}) +# NAME_WITH_ARGS: frame int ns::foo<(anonymous namespace)::$_0>(t=(anonymous namespace)::(unnamed class at {{.*}}) @ {{.*}}) c # NAME_WITH_ARGS: frame int ns::foo<int (*)()>(t=({{.*}}`(anonymous namespace)::anon_bar() at {{.*}})) c diff --git a/lldb/test/Shell/SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp b/lldb/test/Shell/SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp index 5cc39bc351b29..9c661384da462 100644 --- a/lldb/test/Shell/SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp +++ b/lldb/test/Shell/SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp @@ -15,7 +15,7 @@ struct A { } C; } a; -// CHECK: A::(anonymous struct) +// CHECK: A::(anonymous struct at // CHECK: |-DefinitionData is_anonymous pass_in_registers aggregate standard_layout trivially_copyable pod trivial literal -// CHECK: A::(unnamed struct) +// CHECK: A::(unnamed struct at // CHECK: |-DefinitionData pass_in_registers aggregate standard_layout trivially_copyable pod trivial literal diff --git a/lldb/unittests/Symbol/CMakeLists.txt b/lldb/unittests/Symbol/CMakeLists.txt index e1d24357e33db..b1e1edf138d66 100644 --- a/lldb/unittests/Symbol/CMakeLists.txt +++ b/lldb/unittests/Symbol/CMakeLists.txt @@ -27,5 +27,8 @@ add_lldb_unittest(SymbolTests set(test_inputs inlined-functions.yaml + dummy.cpp + dummy_header1.h + dummy_header2.h ) add_unittest_inputs(SymbolTests "${test_inputs}") diff --git a/lldb/unittests/Symbol/Inputs/dummy.cpp b/lldb/unittests/Symbol/Inputs/dummy.cpp new file mode 100644 index 0000000000000..86a157931b774 --- /dev/null +++ b/lldb/unittests/Symbol/Inputs/dummy.cpp @@ -0,0 +1,9 @@ +// Some comment + +#include "dummy_header1.h" + +int main() { + return 0; +} + +// Some comment at the end diff --git a/lldb/unittests/Symbol/TestTypeSystemClang.cpp b/lldb/unittests/Symbol/TestTypeSystemClang.cpp index 4dab1bea477b4..c28d5deebe6c1 100644 --- a/lldb/unittests/Symbol/TestTypeSystemClang.cpp +++ b/lldb/unittests/Symbol/TestTypeSystemClang.cpp @@ -10,6 +10,7 @@ #include "Plugins/TypeSystem/Clang/TypeSystemClang.h" #include "TestingSupport/SubsystemRAII.h" #include "TestingSupport/Symbol/ClangTestUtils.h" +#include "TestingSupport/TestUtilities.h" #include "lldb/Core/Declaration.h" #include "lldb/Host/FileSystem.h" #include "lldb/Host/HostInfo.h" @@ -17,6 +18,7 @@ #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/ExprCXX.h" +#include "clang/Basic/SourceManager.h" #include "gtest/gtest.h" using namespace clang; @@ -51,6 +53,16 @@ class TestTypeSystemClang : public testing::Test { return ClangUtil::GetQualType( m_ast->GetBuiltinTypeByName(ConstString(name))); } + + /// Returns the textual representation of the given source location. + std::string GetLocAsStr(clang::SourceLocation loc) { + return loc.printToString(m_ast->getASTContext().getSourceManager()); + } + /// Returns the textual representation of the given Declaration's source + /// location. + std::string GetDeclLocAsStr(const lldb_private::Declaration &decl) { + return GetLocAsStr(m_ast->GetLocForDecl(decl)); + } }; TEST_F(TestTypeSystemClang, TestGetBasicTypeFromEnum) { @@ -1121,3 +1133,96 @@ TEST_F(TestTypeSystemClang, AddMethodToCXXRecordType_ParmVarDecls) { EXPECT_EQ(method_it->getParamDecl(0)->getDeclContext(), *method_it); EXPECT_EQ(method_it->getParamDecl(1)->getDeclContext(), *method_it); } + +static llvm::StringRef s_invalid_loc = "<invalid loc>"; + +TEST_F(TestTypeSystemClang, TestGetLocForDecl_InvalidDeclaration) { + // Invalid Declaration should produce an invalid location. + lldb_private::Declaration decl; + EXPECT_EQ(s_invalid_loc, GetDeclLocAsStr(decl)); +} + +TEST_F(TestTypeSystemClang, TestGetLocForDecl) { + // Test that GetLocForDecl returns an invalid source location when + // the file provided couldn't be found. But returns valid locations + // for files present on disk. + + EXPECT_EQ(s_invalid_loc, + GetDeclLocAsStr(Declaration(FileSpec("invalid.cpp"), 1, 0))); + + std::string test_file_name = "dummy.cpp"; + auto make_declaration = [&](uint32_t line, uint32_t col) { + return Declaration(FileSpec(GetInputFilePath(test_file_name)), line, col); + }; + + auto sloc_contains = [&](uint32_t line, uint32_t col, + std::string_view expected) { + auto declaration_string = GetDeclLocAsStr(make_declaration(line, col)); + return declaration_string.find(expected) != std::string::npos; + }; + + EXPECT_TRUE(sloc_contains(1, 1, test_file_name + ":1:1")); + + // Max source location. + EXPECT_TRUE(sloc_contains(9, 27, test_file_name + ":9:27")); + + // Clang requires columns to start at 1. + EXPECT_TRUE(sloc_contains(1, 0, test_file_name + ":1:1")); + EXPECT_TRUE(sloc_contains(2, 0, test_file_name + ":2:1")); + + // Clang will also clamp source locations if they exceed the file line/column + // limit. + EXPECT_TRUE(sloc_contains(1000, 0, test_file_name + ":9:27")); + EXPECT_TRUE(sloc_contains(1000, 1000, test_file_name + ":9:27")); + + // Clang will produce invalid source locations for line numbers of 0 though. + EXPECT_EQ(GetDeclLocAsStr(make_declaration(0, 1)), s_invalid_loc); + EXPECT_EQ(GetDeclLocAsStr(make_declaration(0, 0)), s_invalid_loc); + EXPECT_EQ(GetDeclLocAsStr(make_declaration(0, 1000)), s_invalid_loc); + + // Check that we hooked up the correct file contents to the FileID + SourceLocation loc = m_ast->GetLocForDecl(make_declaration(5, 5)); + clang::SourceManager &sm = m_ast->getASTContext().getSourceManager(); + EXPECT_TRUE(sm.getBufferData(sm.getFileID(loc)) + .contains("// Some comment at the end")); +} + +TEST_F(TestTypeSystemClang, TestGetLocForDecl_MultipleLocs) { + // Test having multiple source files passed to GetLocForDecl. + auto sloc_contains = [&](SourceLocation loc, std::string_view expected) { + auto declaration_string = GetLocAsStr(loc); + return declaration_string.find(expected) != std::string::npos; + }; + + Declaration main_decl(FileSpec(GetInputFilePath("dummy.cpp")), 2, 1); + SourceLocation main_loc = m_ast->GetLocForDecl(main_decl); + EXPECT_TRUE(sloc_contains(main_loc, "dummy.cpp:2:1")); + + Declaration header_decl1(FileSpec(GetInputFilePath("dummy_header1.h")), 1, 1); + SourceLocation header_loc1 = m_ast->GetLocForDecl(header_decl1); + EXPECT_TRUE(sloc_contains(header_loc1, "dummy_header1.h:1:1")); + + Declaration header_decl2(FileSpec(GetInputFilePath("dummy_header2.h")), 3, 4); + SourceLocation header_loc2 = m_ast->GetLocForDecl(header_decl2); + EXPECT_TRUE(sloc_contains(header_loc2, "dummy_header2.h:3:4")); + + // This tests that the file tree we create in the Clang SourceManager is + // allowing Clang to establish a deterministic ordering between all the source + // locations. If this wasn't the case, then isBeforeInTranslationUnit (which + // is for example used by Clang when ordering diagnostics) would assert. + clang::SourceManager &SM = m_ast->getASTContext().getSourceManager(); + EXPECT_TRUE(SM.isBeforeInTranslationUnit(main_loc, header_loc1)); + EXPECT_TRUE(SM.isBeforeInTranslationUnit(header_loc1, header_loc2)); + + // Each ASTContext has a fake MainFileID which we include all other FileIDs + // into. + FileID fake_main_id = SM.getMainFileID(); + SourceLocation fake_main = SM.getLocForStartOfFile(fake_main_id); + ASSERT_TRUE(fake_main.isValid()); + EXPECT_TRUE(SM.isBeforeInTranslationUnit(fake_main, main_loc)); + EXPECT_EQ(SM.getBufferData(fake_main_id), "\n"); + + auto maybe_file_entry = SM.getFileEntryRefForID(fake_main_id); + ASSERT_TRUE(maybe_file_entry.has_value()); + EXPECT_EQ(maybe_file_entry->getName(), "<LLDB Dummy Main File>"); +} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits