https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/149827
>From b8f31ec8f556169a45370938cf28bd8582134a5c Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Sun, 3 Aug 2025 11:38:07 +0100 Subject: [PATCH 1/8] [clang][Attr] Remove 'literal label' form of AsmLabelAttr This was added purely for the needs of LLDB. However, starting with https://github.com/llvm/llvm-project/pull/151355 and https://github.com/llvm/llvm-project/pull/148877 we no longer create literal AsmLabels in LLDB either. So this is unused and we can get rid of it. In the near future LLDB will want to add special support for mangling `AsmLabel`s, and the "literal label" codepath in the mangler made this more cumbersome. (cherry picked from commit 342f0d5cd722ae7ec3f48f136472923ce38f9981) --- clang/include/clang/Basic/Attr.td | 17 ++++------------- clang/lib/AST/Mangle.cpp | 4 ++-- clang/lib/Sema/SemaDecl.cpp | 13 +++++-------- clang/unittests/AST/DeclTest.cpp | 9 +-------- .../TypeSystem/Clang/TypeSystemClang.cpp | 7 +++---- 5 files changed, 15 insertions(+), 35 deletions(-) diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index b3ff45b3e90a3..a0efb21218312 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -1059,22 +1059,13 @@ def AVRSignal : InheritableAttr, TargetSpecificAttr<TargetAVR> { def AsmLabel : InheritableAttr { let Spellings = [CustomKeyword<"asm">, CustomKeyword<"__asm__">]; let Args = [ - // Label specifies the mangled name for the decl. - StringArgument<"Label">, - - // IsLiteralLabel specifies whether the label is literal (i.e. suppresses - // the global C symbol prefix) or not. If not, the mangle-suppression prefix - // ('\01') is omitted from the decl name at the LLVM IR level. - // - // Non-literal labels are used by some external AST sources like LLDB. - BoolArgument<"IsLiteralLabel", /*optional=*/0, /*fake=*/1> - ]; + // Label specifies the mangled name for the decl. + StringArgument<"Label">, ]; let SemaHandler = 0; let Documentation = [AsmLabelDocs]; - let AdditionalMembers = -[{ + let AdditionalMembers = [{ bool isEquivalent(AsmLabelAttr *Other) const { - return getLabel() == Other->getLabel() && getIsLiteralLabel() == Other->getIsLiteralLabel(); + return getLabel() == Other->getLabel(); } }]; } diff --git a/clang/lib/AST/Mangle.cpp b/clang/lib/AST/Mangle.cpp index 9652fdbc4e125..0bfb51c11f0a5 100644 --- a/clang/lib/AST/Mangle.cpp +++ b/clang/lib/AST/Mangle.cpp @@ -161,9 +161,9 @@ void MangleContext::mangleName(GlobalDecl GD, raw_ostream &Out) { if (const AsmLabelAttr *ALA = D->getAttr<AsmLabelAttr>()) { // If we have an asm name, then we use it as the mangling. - // If the label isn't literal, or if this is an alias for an LLVM intrinsic, + // If the label is an alias for an LLVM intrinsic, // do not add a "\01" prefix. - if (!ALA->getIsLiteralLabel() || ALA->getLabel().starts_with("llvm.")) { + if (ALA->getLabel().starts_with("llvm.")) { Out << ALA->getLabel(); return; } diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index e2ac648320c0f..885d04b9ab926 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -8113,9 +8113,7 @@ NamedDecl *Sema::ActOnVariableDeclarator( } } - NewVD->addAttr(AsmLabelAttr::Create(Context, Label, - /*IsLiteralLabel=*/true, - SE->getStrTokenLoc(0))); + NewVD->addAttr(AsmLabelAttr::Create(Context, Label, SE->getStrTokenLoc(0))); } else if (!ExtnameUndeclaredIdentifiers.empty()) { llvm::DenseMap<IdentifierInfo*,AsmLabelAttr*>::iterator I = ExtnameUndeclaredIdentifiers.find(NewVD->getIdentifier()); @@ -10345,9 +10343,8 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC, if (Expr *E = D.getAsmLabel()) { // The parser guarantees this is a string. StringLiteral *SE = cast<StringLiteral>(E); - NewFD->addAttr(AsmLabelAttr::Create(Context, SE->getString(), - /*IsLiteralLabel=*/true, - SE->getStrTokenLoc(0))); + NewFD->addAttr( + AsmLabelAttr::Create(Context, SE->getString(), SE->getStrTokenLoc(0))); } else if (!ExtnameUndeclaredIdentifiers.empty()) { llvm::DenseMap<IdentifierInfo*,AsmLabelAttr*>::iterator I = ExtnameUndeclaredIdentifiers.find(NewFD->getIdentifier()); @@ -20598,8 +20595,8 @@ void Sema::ActOnPragmaRedefineExtname(IdentifierInfo* Name, LookupOrdinaryName); AttributeCommonInfo Info(AliasName, SourceRange(AliasNameLoc), AttributeCommonInfo::Form::Pragma()); - AsmLabelAttr *Attr = AsmLabelAttr::CreateImplicit( - Context, AliasName->getName(), /*IsLiteralLabel=*/true, Info); + AsmLabelAttr *Attr = + AsmLabelAttr::CreateImplicit(Context, AliasName->getName(), Info); // If a declaration that: // 1) declares a function or a variable diff --git a/clang/unittests/AST/DeclTest.cpp b/clang/unittests/AST/DeclTest.cpp index ed635da683aab..afaf413493299 100644 --- a/clang/unittests/AST/DeclTest.cpp +++ b/clang/unittests/AST/DeclTest.cpp @@ -74,7 +74,6 @@ TEST(Decl, AsmLabelAttr) { StringRef Code = R"( struct S { void f() {} - void g() {} }; )"; auto AST = @@ -87,11 +86,8 @@ TEST(Decl, AsmLabelAttr) { const auto *DeclS = selectFirst<CXXRecordDecl>("d", match(cxxRecordDecl().bind("d"), Ctx)); NamedDecl *DeclF = *DeclS->method_begin(); - NamedDecl *DeclG = *(++DeclS->method_begin()); - // Attach asm labels to the decls: one literal, and one not. - DeclF->addAttr(AsmLabelAttr::Create(Ctx, "foo", /*LiteralLabel=*/true)); - DeclG->addAttr(AsmLabelAttr::Create(Ctx, "goo", /*LiteralLabel=*/false)); + DeclF->addAttr(AsmLabelAttr::Create(Ctx, "foo")); // Mangle the decl names. std::string MangleF, MangleG; @@ -99,14 +95,11 @@ TEST(Decl, AsmLabelAttr) { ItaniumMangleContext::create(Ctx, Diags)); { llvm::raw_string_ostream OS_F(MangleF); - llvm::raw_string_ostream OS_G(MangleG); MC->mangleName(DeclF, OS_F); - MC->mangleName(DeclG, OS_G); } ASSERT_EQ(MangleF, "\x01" "foo"); - ASSERT_EQ(MangleG, "goo"); } TEST(Decl, MangleDependentSizedArray) { diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index 8dd6aa08ba2aa..4af900a174e4f 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -2183,8 +2183,7 @@ FunctionDecl *TypeSystemClang::CreateFunctionDeclaration( // This is done separately for member functions in // AddMethodToCXXRecordType. if (!asm_label.empty()) - func_decl->addAttr(clang::AsmLabelAttr::CreateImplicit(ast, asm_label, - /*literal=*/true)); + func_decl->addAttr(clang::AsmLabelAttr::CreateImplicit(ast, asm_label)); SetOwningModule(func_decl, owning_module); decl_ctx->addDecl(func_decl); @@ -7823,8 +7822,8 @@ clang::CXXMethodDecl *TypeSystemClang::AddMethodToCXXRecordType( cxx_method_decl->addAttr(clang::UsedAttr::CreateImplicit(getASTContext())); if (!asm_label.empty()) - cxx_method_decl->addAttr(clang::AsmLabelAttr::CreateImplicit( - getASTContext(), asm_label, /*literal=*/true)); + cxx_method_decl->addAttr( + clang::AsmLabelAttr::CreateImplicit(getASTContext(), asm_label)); // Parameters on member function declarations in DWARF generally don't // have names, so we omit them when creating the ParmVarDecls. >From 3ddc4d48d4f3315141bdc4a922a52da48bd5df68 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Mon, 4 Aug 2025 14:20:45 +0100 Subject: [PATCH 2/8] [clang][Mangle] Inject structor type into mangled name when mangling for LLDB JIT expressions This patch adds special handling for `AsmLabel`s created by LLDB. LLDB uses `AsmLabel`s to encode information about a function declaration to make it easier to locate function symbols when JITing C++ expressions. For constructors/destructors LLDB doesn't know at the time of creating the `AsmLabelAttr` which structor variant the expression evaluator will need to call (this is decided when compiling the expression). So we make the Clang mangler inject this information into our custom label when we're JITting the expression. --- clang/lib/AST/Mangle.cpp | 33 +++++++++++++- clang/unittests/AST/DeclTest.cpp | 75 ++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 1 deletion(-) diff --git a/clang/lib/AST/Mangle.cpp b/clang/lib/AST/Mangle.cpp index 0bfb51c11f0a5..1131477fa7200 100644 --- a/clang/lib/AST/Mangle.cpp +++ b/clang/lib/AST/Mangle.cpp @@ -152,6 +152,33 @@ bool MangleContext::shouldMangleDeclName(const NamedDecl *D) { return shouldMangleCXXName(D); } +static llvm::StringRef g_lldb_func_call_label_prefix = "$__lldb_func:"; + +/// Given an LLDB function call label, this function prints the label +/// into \c Out, together with the structor type of \c GD (if the +/// decl is a constructor/destructor). LLDB knows how to handle mangled +/// names with this encoding. +/// +/// Example input label: +/// $__lldb_func::123:456:~Foo +/// +/// Example output: +/// $__lldb_func:D1:123:456:~Foo +/// +static void emitLLDBAsmLabel(llvm::StringRef label, GlobalDecl GD, + llvm::raw_ostream &Out) { + assert(label.starts_with(g_lldb_func_call_label_prefix)); + + Out << g_lldb_func_call_label_prefix; + + if (llvm::isa<clang::CXXConstructorDecl>(GD.getDecl())) + Out << "C" << GD.getCtorType(); + else if (llvm::isa<clang::CXXDestructorDecl>(GD.getDecl())) + Out << "D" << GD.getDtorType(); + + Out << label.substr(g_lldb_func_call_label_prefix.size()); +} + void MangleContext::mangleName(GlobalDecl GD, raw_ostream &Out) { const ASTContext &ASTContext = getASTContext(); const NamedDecl *D = cast<NamedDecl>(GD.getDecl()); @@ -185,7 +212,11 @@ void MangleContext::mangleName(GlobalDecl GD, raw_ostream &Out) { if (!UserLabelPrefix.empty()) Out << '\01'; // LLVM IR Marker for __asm("foo") - Out << ALA->getLabel(); + if (ALA->getLabel().starts_with(g_lldb_func_call_label_prefix)) + emitLLDBAsmLabel(ALA->getLabel(), GD, Out); + else + Out << ALA->getLabel(); + return; } diff --git a/clang/unittests/AST/DeclTest.cpp b/clang/unittests/AST/DeclTest.cpp index afaf413493299..6c7b65d7e82d7 100644 --- a/clang/unittests/AST/DeclTest.cpp +++ b/clang/unittests/AST/DeclTest.cpp @@ -16,6 +16,7 @@ #include "clang/AST/Mangle.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/ABI.h" #include "clang/Basic/Diagnostic.h" #include "clang/Basic/LLVM.h" #include "clang/Basic/TargetInfo.h" @@ -102,6 +103,80 @@ TEST(Decl, AsmLabelAttr) { "foo"); } +TEST(Decl, AsmLabelAttr_LLDB) { + StringRef Code = R"( + struct S { + void f() {} + S() = default; + ~S() = default; + }; + )"; + auto AST = + tooling::buildASTFromCodeWithArgs(Code, {"-target", "i386-apple-darwin"}); + ASTContext &Ctx = AST->getASTContext(); + assert(Ctx.getTargetInfo().getUserLabelPrefix() == StringRef("_") && + "Expected target to have a global prefix"); + DiagnosticsEngine &Diags = AST->getDiagnostics(); + + const auto *DeclS = + selectFirst<CXXRecordDecl>("d", match(cxxRecordDecl().bind("d"), Ctx)); + + auto *DeclF = *DeclS->method_begin(); + auto *Ctor = *DeclS->ctor_begin(); + auto *Dtor = DeclS->getDestructor(); + + ASSERT_TRUE(DeclF); + ASSERT_TRUE(Ctor); + ASSERT_TRUE(Dtor); + + DeclF->addAttr(AsmLabelAttr::Create(Ctx, "$__lldb_func::123:123:_Z1fv")); + Ctor->addAttr(AsmLabelAttr::Create(Ctx, "$__lldb_func::123:123:S")); + Dtor->addAttr(AsmLabelAttr::Create(Ctx, "$__lldb_func::123:123:~S")); + + std::unique_ptr<ItaniumMangleContext> MC( + ItaniumMangleContext::create(Ctx, Diags)); + + { + std::string Mangled; + llvm::raw_string_ostream OS_Mangled(Mangled); + MC->mangleName(DeclF, OS_Mangled); + + ASSERT_EQ(Mangled, "\x01$__lldb_func::123:123:_Z1fv"); + }; + + { + std::string Mangled; + llvm::raw_string_ostream OS_Mangled(Mangled); + MC->mangleName(GlobalDecl(Ctor, CXXCtorType::Ctor_Complete), OS_Mangled); + + ASSERT_EQ(Mangled, "\x01$__lldb_func:C0:123:123:S"); + }; + + { + std::string Mangled; + llvm::raw_string_ostream OS_Mangled(Mangled); + MC->mangleName(GlobalDecl(Ctor, CXXCtorType::Ctor_Base), OS_Mangled); + + ASSERT_EQ(Mangled, "\x01$__lldb_func:C1:123:123:S"); + }; + + { + std::string Mangled; + llvm::raw_string_ostream OS_Mangled(Mangled); + MC->mangleName(GlobalDecl(Dtor, CXXDtorType::Dtor_Deleting), OS_Mangled); + + ASSERT_EQ(Mangled, "\x01$__lldb_func:D0:123:123:~S"); + }; + + { + std::string Mangled; + llvm::raw_string_ostream OS_Mangled(Mangled); + MC->mangleName(GlobalDecl(Dtor, CXXDtorType::Dtor_Base), OS_Mangled); + + ASSERT_EQ(Mangled, "\x01$__lldb_func:D2:123:123:~S"); + }; +} + TEST(Decl, MangleDependentSizedArray) { StringRef Code = R"( template <int ...N> >From a650687a8404b11c0a89d407ddffb7ab92adfd54 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Mon, 4 Aug 2025 07:49:40 +0100 Subject: [PATCH 3/8] [ItaniumDemangle] Add getter for constructor/destructor variants This patch adds a way to retrieve the constructor/destructor variant from the Itanium demangle tree. This will be used by LLDB. --- libcxxabi/src/demangle/ItaniumDemangle.h | 2 ++ llvm/include/llvm/Demangle/Demangle.h | 3 +++ llvm/include/llvm/Demangle/ItaniumDemangle.h | 2 ++ llvm/lib/Demangle/ItaniumDemangle.cpp | 10 +++++++--- 4 files changed, 14 insertions(+), 3 deletions(-) diff --git a/libcxxabi/src/demangle/ItaniumDemangle.h b/libcxxabi/src/demangle/ItaniumDemangle.h index 6f27da7b9cadf..7b3983bc89367 100644 --- a/libcxxabi/src/demangle/ItaniumDemangle.h +++ b/libcxxabi/src/demangle/ItaniumDemangle.h @@ -1766,6 +1766,8 @@ class CtorDtorName final : public Node { template<typename Fn> void match(Fn F) const { F(Basename, IsDtor, Variant); } + int getVariant() const { return Variant; } + void printLeft(OutputBuffer &OB) const override { if (IsDtor) OB += "~"; diff --git a/llvm/include/llvm/Demangle/Demangle.h b/llvm/include/llvm/Demangle/Demangle.h index d9b08b2d856dc..6af8fad9ceb86 100644 --- a/llvm/include/llvm/Demangle/Demangle.h +++ b/llvm/include/llvm/Demangle/Demangle.h @@ -127,6 +127,9 @@ struct ItaniumPartialDemangler { /// If this symbol describes a constructor or destructor. DEMANGLE_ABI bool isCtorOrDtor() const; + /// If this symbol describes a constructor or destructor. + std::optional<int> getCtorOrDtorVariant() const; + /// If this symbol describes a function. DEMANGLE_ABI bool isFunction() const; diff --git a/llvm/include/llvm/Demangle/ItaniumDemangle.h b/llvm/include/llvm/Demangle/ItaniumDemangle.h index 62d427c3966bb..c0db02f8e7fef 100644 --- a/llvm/include/llvm/Demangle/ItaniumDemangle.h +++ b/llvm/include/llvm/Demangle/ItaniumDemangle.h @@ -1766,6 +1766,8 @@ class CtorDtorName final : public Node { template<typename Fn> void match(Fn F) const { F(Basename, IsDtor, Variant); } + int getVariant() const { return Variant; } + void printLeft(OutputBuffer &OB) const override { if (IsDtor) OB += "~"; diff --git a/llvm/lib/Demangle/ItaniumDemangle.cpp b/llvm/lib/Demangle/ItaniumDemangle.cpp index 1009cc91ca12a..a5d7a5576fccf 100644 --- a/llvm/lib/Demangle/ItaniumDemangle.cpp +++ b/llvm/lib/Demangle/ItaniumDemangle.cpp @@ -560,13 +560,17 @@ bool ItaniumPartialDemangler::hasFunctionQualifiers() const { } bool ItaniumPartialDemangler::isCtorOrDtor() const { + return getCtorOrDtorVariant().has_value(); +} + +std::optional<int> ItaniumPartialDemangler::getCtorOrDtorVariant() const { const Node *N = static_cast<const Node *>(RootNode); while (N) { switch (N->getKind()) { default: - return false; + return std::nullopt; case Node::KCtorDtorName: - return true; + return static_cast<const CtorDtorName *>(N)->getVariant(); case Node::KAbiTagAttr: N = static_cast<const AbiTagAttr *>(N)->Base; @@ -588,7 +592,7 @@ bool ItaniumPartialDemangler::isCtorOrDtor() const { break; } } - return false; + return std::nullopt; } bool ItaniumPartialDemangler::isFunction() const { >From a35bb5aa37c61e1dd428bfec19ef1500ad7aa598 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Mon, 4 Aug 2025 14:21:16 +0100 Subject: [PATCH 4/8] [lldb][Expression] Encode structor variant into FunctionCallLabel This patch is an implementation of [this discussion](https://discourse.llvm.org/t/rfc-lldb-handling-abi-tagged-constructors-destructors-in-expression-evaluator/82816/7) about handling ABI-tagged structors during expression evaluation. **Motivation** LLDB encodes the mangled name of a `DW_TAG_subprogram` into `AsmLabel`s on function and method Clang AST nodes. This means that when calls to these functions get lowered into IR (when running JITted expressions), the address resolver can locate the appropriate symbol by mangled name (and it's guaranteed to find the symbol because we got the mangled name from debug-info, instead of letting Clang mangle it based on AST structure). However, we don't do this for `CXXConstructorDecl`s/`CXXDestructorDecl`s because these structor declarations in DWARF don't have a linkage name. This is because there can be multiple variants of a structor, each with a distinct mangling in the Itanium ABI. Each structor variant has its own definition `DW_TAG_subprogram`. So LLDB doesn't know which mangled name to put into the `AsmLabel`. Currently this means using an ABI-tagged constructor in LLDB expressions won't work (see [this RFC](https://discourse.llvm.org/t/rfc-lldb-handling-abi-tagged-constructors-destructors-in-expression-evaluator/82816) for concrete examples). **Proposed Solution** The `FunctionCallLabel` encoding that we put into `AsmLabel`s already supports stuffing more info about a DIE into it. So this patch extends the `FunctionCallLabel` to contain an optional discriminator (a sequence of bytes) which the `SymbolFileDWARF` plugin interprets as the constructor/destructor variant of that DIE. There's a few subtleties here: 1. At the point at which LLDB first constructs the label, it has no way of knowing (just by looking the debug-info declaration), which structor variant the expression evaluator is supposed to call. That's something that gets decided when compiling the expression. So we let the Clang mangler inject the correct structor variant into the `AsmLabel` during JITing. I adjusted the `AsmLabelAttr` mangling for this. An option would've been to create a new Clang attribute which behaved like an `AsmLabel` but with these special semantics for LLDB. My main concern there is that we'd have to adjust all the `AsmLabelAttr` checks around Clang to also now account for this new attribute. 2. The compiler is free to omit the `C1` variant of a constructor if the `C2` variant is sufficient. In that case it may alias `C1` to `C2`, leaving us with only the `C2` `DW_TAG_subprogram` in the object file. Linux is one of the platforms where this occurs. For those cases there's heuristic in `SymbolFileDWARF` where we pick `C2` if we asked for `C1` but it doesn't exist. This may not always be correct (if for some reason the compiler decided to drop `C1` for other reasons). --- lldb/include/lldb/Expression/Expression.h | 8 +- lldb/source/Expression/Expression.cpp | 29 ++-- .../SymbolFile/DWARF/DWARFASTParserClang.cpp | 47 +++++- .../SymbolFile/DWARF/SymbolFileDWARF.cpp | 152 +++++++++++++++--- .../SymbolFile/DWARF/SymbolFileDWARF.h | 4 + .../API/lang/cpp/abi_tag_structors/Makefile | 3 + .../abi_tag_structors/TestAbiTagStructors.py | 30 ++++ .../API/lang/cpp/abi_tag_structors/main.cpp | 38 +++++ lldb/unittests/Expression/ExpressionTest.cpp | 37 +++-- lldb/unittests/Symbol/TestTypeSystemClang.cpp | 14 +- 10 files changed, 305 insertions(+), 57 deletions(-) create mode 100644 lldb/test/API/lang/cpp/abi_tag_structors/Makefile create mode 100644 lldb/test/API/lang/cpp/abi_tag_structors/TestAbiTagStructors.py create mode 100644 lldb/test/API/lang/cpp/abi_tag_structors/main.cpp diff --git a/lldb/include/lldb/Expression/Expression.h b/lldb/include/lldb/Expression/Expression.h index 20067f469895b..847226167d584 100644 --- a/lldb/include/lldb/Expression/Expression.h +++ b/lldb/include/lldb/Expression/Expression.h @@ -103,11 +103,15 @@ class Expression { /// /// The format being: /// -/// <prefix>:<module uid>:<symbol uid>:<name> +/// <prefix>:<discriminator>:<module uid>:<symbol uid>:<name> /// /// The label string needs to stay valid for the entire lifetime /// of this object. struct FunctionCallLabel { + /// Arbitrary string which language plugins can interpret for their + /// own needs. + llvm::StringRef discriminator; + /// Unique identifier of the lldb_private::Module /// which contains the symbol identified by \c symbol_id. lldb::user_id_t module_id; @@ -133,7 +137,7 @@ struct FunctionCallLabel { /// /// The representation roundtrips through \c fromString: /// \code{.cpp} - /// llvm::StringRef encoded = "$__lldb_func:0x0:0x0:_Z3foov"; + /// llvm::StringRef encoded = "$__lldb_func:blah:0x0:0x0:_Z3foov"; /// FunctionCallLabel label = *fromString(label); /// /// assert (label.toString() == encoded); diff --git a/lldb/source/Expression/Expression.cpp b/lldb/source/Expression/Expression.cpp index 796851ff15ca3..16ecb1d7deef8 100644 --- a/lldb/source/Expression/Expression.cpp +++ b/lldb/source/Expression/Expression.cpp @@ -34,10 +34,10 @@ Expression::Expression(ExecutionContextScope &exe_scope) llvm::Expected<FunctionCallLabel> lldb_private::FunctionCallLabel::fromString(llvm::StringRef label) { - llvm::SmallVector<llvm::StringRef, 4> components; - label.split(components, ":", /*MaxSplit=*/3); + llvm::SmallVector<llvm::StringRef, 5> components; + label.split(components, ":", /*MaxSplit=*/4); - if (components.size() != 4) + if (components.size() != 5) return llvm::createStringError("malformed function call label."); if (components[0] != FunctionCallLabelPrefix) @@ -45,8 +45,10 @@ lldb_private::FunctionCallLabel::fromString(llvm::StringRef label) { "expected function call label prefix '{0}' but found '{1}' instead.", FunctionCallLabelPrefix, components[0])); - llvm::StringRef module_label = components[1]; - llvm::StringRef die_label = components[2]; + llvm::StringRef discriminator = components[1]; + llvm::StringRef module_label = components[2]; + llvm::StringRef die_label = components[3]; + llvm::StringRef lookup_name = components[4]; lldb::user_id_t module_id = 0; if (!llvm::to_integer(module_label, module_id)) @@ -58,20 +60,23 @@ lldb_private::FunctionCallLabel::fromString(llvm::StringRef label) { return llvm::createStringError( llvm::formatv("failed to parse symbol ID from '{0}'.", die_label)); - return FunctionCallLabel{/*.module_id=*/module_id, + return FunctionCallLabel{/*.discriminator=*/discriminator, + /*.module_id=*/module_id, /*.symbol_id=*/die_id, - /*.lookup_name=*/components[3]}; + /*.lookup_name=*/lookup_name}; } std::string lldb_private::FunctionCallLabel::toString() const { - return llvm::formatv("{0}:{1:x}:{2:x}:{3}", FunctionCallLabelPrefix, - module_id, symbol_id, lookup_name) + return llvm::formatv("{0}:{1}:{2:x}:{3:x}:{4}", FunctionCallLabelPrefix, + discriminator, module_id, symbol_id, lookup_name) .str(); } void llvm::format_provider<FunctionCallLabel>::format( const FunctionCallLabel &label, raw_ostream &OS, StringRef Style) { - OS << llvm::formatv("FunctionCallLabel{ module_id: {0:x}, symbol_id: {1:x}, " - "lookup_name: {2} }", - label.module_id, label.symbol_id, label.lookup_name); + OS << llvm::formatv("FunctionCallLabel{ discriminator: {0}, module_id: " + "{1:x}, symbol_id: {2:x}, " + "lookup_name: {3} }", + label.discriminator, label.module_id, label.symbol_id, + label.lookup_name); } diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 781c1c6c5745d..0b6a4148738d4 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -250,11 +250,52 @@ static unsigned GetCXXMethodCVQuals(const DWARFDIE &subprogram, return cv_quals; } +static const char *GetMangledOrStructorName(const DWARFDIE &die) { + const char *name = die.GetMangledName(/*substitute_name_allowed*/ false); + if (name) + return name; + + name = die.GetName(); + if (!name) + return nullptr; + + DWARFDIE parent = die.GetParent(); + if (!parent.IsStructUnionOrClass()) + return nullptr; + + const char *parent_name = parent.GetName(); + if (!parent_name) + return nullptr; + + // Constructor. + if (::strcmp(parent_name, name) == 0) + return name; + + // Destructor. + if (name[0] == '~' && ::strcmp(parent_name, name + 1) == 0) + return name; + + return nullptr; +} + static std::string MakeLLDBFuncAsmLabel(const DWARFDIE &die) { - char const *name = die.GetMangledName(/*substitute_name_allowed*/ false); + char const *name = GetMangledOrStructorName(die); if (!name) return {}; + auto *cu = die.GetCU(); + if (!cu) + return {}; + + // FIXME: When resolving function call labels, we check that + // that the definition's DW_AT_specification points to the + // declaration that we encoded into the label here. But if the + // declaration came from a type-unit (and the definition from + // .debug_info), that check won't work. So for now, don't use + // function call labels for declaration DIEs from type-units. + if (cu->IsTypeUnit()) + return {}; + SymbolFileDWARF *dwarf = die.GetDWARF(); if (!dwarf) return {}; @@ -285,7 +326,9 @@ static std::string MakeLLDBFuncAsmLabel(const DWARFDIE &die) { if (die_id == LLDB_INVALID_UID) return {}; - return FunctionCallLabel{/*module_id=*/module_id, + // Note, discriminator is added by Clang during mangling. + return FunctionCallLabel{/*discriminator=*/{}, + /*module_id=*/module_id, /*symbol_id=*/die_id, /*.lookup_name=*/name} .toString(); diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index 42a66ce75d6d6..2dfb04cfd3ab5 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -7,7 +7,9 @@ //===----------------------------------------------------------------------===// #include "SymbolFileDWARF.h" +#include "clang/Basic/ABI.h" #include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/StringExtras.h" #include "llvm/DebugInfo/DWARF/DWARFAddressRange.h" #include "llvm/DebugInfo/DWARF/DWARFDebugLoc.h" #include "llvm/Support/Casting.h" @@ -78,6 +80,7 @@ #include "llvm/DebugInfo/DWARF/DWARFContext.h" #include "llvm/DebugInfo/DWARF/DWARFDebugAbbrev.h" +#include "llvm/Demangle/Demangle.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/FormatVariadic.h" @@ -2482,6 +2485,134 @@ bool SymbolFileDWARF::ResolveFunction(const DWARFDIE &orig_die, return false; } +static int ClangToItaniumCtorKind(clang::CXXCtorType kind) { + switch (kind) { + case clang::CXXCtorType::Ctor_Complete: + return 1; + case clang::CXXCtorType::Ctor_Base: + return 2; + case clang::CXXCtorType::Ctor_CopyingClosure: + case clang::CXXCtorType::Ctor_DefaultClosure: + case clang::CXXCtorType::Ctor_Comdat: + llvm_unreachable("Unexpected constructor kind."); + } +} + +static int ClangToItaniumDtorKind(clang::CXXDtorType kind) { + switch (kind) { + case clang::CXXDtorType::Dtor_Deleting: + return 0; + case clang::CXXDtorType::Dtor_Complete: + return 1; + case clang::CXXDtorType::Dtor_Base: + return 2; + case clang::CXXDtorType::Dtor_Comdat: + llvm_unreachable("Unexpected destructor kind."); + } +} + +static std::optional<int> +GetItaniumCtorDtorVariant(llvm::StringRef discriminator) { + const bool is_ctor = discriminator.consume_front("C"); + if (!is_ctor && !discriminator.consume_front("D")) + return std::nullopt; + + uint64_t structor_kind; + if (!llvm::to_integer(discriminator, structor_kind)) + return std::nullopt; + + if (is_ctor) { + if (structor_kind > clang::CXXCtorType::Ctor_DefaultClosure) + return std::nullopt; + + return ClangToItaniumCtorKind( + static_cast<clang::CXXCtorType>(structor_kind)); + } + + if (structor_kind > clang::CXXDtorType::Dtor_Comdat) + return std::nullopt; + + return ClangToItaniumDtorKind(static_cast<clang::CXXDtorType>(structor_kind)); +} + +DWARFDIE SymbolFileDWARF::FindFunctionDefinition(const FunctionCallLabel &label, + const DWARFDIE &declaration) { + DWARFDIE definition; + llvm::DenseMap<int, DWARFDIE> structor_variant_to_die; + + // eFunctionNameTypeFull for mangled name lookup. + // eFunctionNameTypeMethod is required for structor lookups (since we look + // those up by DW_AT_name). + Module::LookupInfo info(ConstString(label.lookup_name), + lldb::eFunctionNameTypeFull | + lldb::eFunctionNameTypeMethod, + lldb::eLanguageTypeUnknown); + + m_index->GetFunctions(info, *this, {}, [&](DWARFDIE entry) { + if (entry.GetAttributeValueAsUnsigned(llvm::dwarf::DW_AT_declaration, 0)) + return IterationAction::Continue; + + auto spec = entry.GetAttributeValueAsReferenceDIE(DW_AT_specification); + if (!spec) + return IterationAction::Continue; + + if (spec != declaration) + return IterationAction::Continue; + + // We're not picking a specific structor variant DIE, so we're done here. + if (label.discriminator.empty()) { + definition = entry; + return IterationAction::Stop; + } + + const char *mangled = + entry.GetMangledName(/*substitute_name_allowed=*/false); + if (!mangled) + return IterationAction::Continue; + + llvm::ItaniumPartialDemangler D; + if (D.partialDemangle(mangled)) + return IterationAction::Continue; + + auto structor_variant = D.getCtorOrDtorVariant(); + if (!structor_variant) + return IterationAction::Continue; + + auto [_, inserted] = structor_variant_to_die.try_emplace(*structor_variant, + std::move(entry)); + assert(inserted); + + // The compiler may choose to alias the constructor variants + // (notably this happens on Linux), so we might not have a definition + // DIE for some structor variants. Hence we iterate over all variants + // and pick the most appropriate one out of those. + return IterationAction::Continue; + }); + + if (definition.IsValid()) + return definition; + + auto label_variant = GetItaniumCtorDtorVariant(label.discriminator); + if (!label_variant) + return {}; + + auto it = structor_variant_to_die.find(*label_variant); + + // Found the exact variant. + if (it != structor_variant_to_die.end()) + return it->getSecond(); + + // We need a C1 constructor. If debug-info only contains a DIE for C2, + // assume C1 was aliased to C2. + if (!label.lookup_name.starts_with("~") && label_variant == 1) { + if (auto it = structor_variant_to_die.find(2); + it != structor_variant_to_die.end()) + return it->getSecond(); + } + + return {}; +} + llvm::Expected<SymbolContext> SymbolFileDWARF::ResolveFunctionCallLabel(const FunctionCallLabel &label) { std::lock_guard<std::recursive_mutex> guard(GetModuleMutex()); @@ -2494,25 +2625,8 @@ SymbolFileDWARF::ResolveFunctionCallLabel(const FunctionCallLabel &label) { // Label was created using a declaration DIE. Need to fetch the definition // to resolve the function call. if (die.GetAttributeValueAsUnsigned(llvm::dwarf::DW_AT_declaration, 0)) { - Module::LookupInfo info(ConstString(label.lookup_name), - lldb::eFunctionNameTypeFull, - lldb::eLanguageTypeUnknown); - - m_index->GetFunctions(info, *this, {}, [&](DWARFDIE entry) { - if (entry.GetAttributeValueAsUnsigned(llvm::dwarf::DW_AT_declaration, 0)) - return IterationAction::Continue; - - // We don't check whether the specification DIE for this function - // corresponds to the declaration DIE because the declaration might be in - // a type-unit but the definition in the compile-unit (and it's - // specifcation would point to the declaration in the compile-unit). We - // rely on the mangled name within the module to be enough to find us the - // unique definition. - die = entry; - return IterationAction::Stop; - }); - - if (die.GetAttributeValueAsUnsigned(llvm::dwarf::DW_AT_declaration, 0)) + die = FindFunctionDefinition(label, die); + if (!die.IsValid()) return llvm::createStringError( llvm::formatv("failed to find definition DIE for {0}", label)); } diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h index 3ec538da8cf77..843346619db37 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h @@ -372,6 +372,10 @@ class SymbolFileDWARF : public SymbolFileCommon { /// Returns the DWARFIndex for this symbol, if it exists. DWARFIndex *getIndex() { return m_index.get(); } +private: + DWARFDIE FindFunctionDefinition(const FunctionCallLabel &label, + const DWARFDIE &declaration); + protected: SymbolFileDWARF(const SymbolFileDWARF &) = delete; const SymbolFileDWARF &operator=(const SymbolFileDWARF &) = delete; diff --git a/lldb/test/API/lang/cpp/abi_tag_structors/Makefile b/lldb/test/API/lang/cpp/abi_tag_structors/Makefile new file mode 100644 index 0000000000000..99998b20bcb05 --- /dev/null +++ b/lldb/test/API/lang/cpp/abi_tag_structors/Makefile @@ -0,0 +1,3 @@ +CXX_SOURCES := main.cpp + +include Makefile.rules diff --git a/lldb/test/API/lang/cpp/abi_tag_structors/TestAbiTagStructors.py b/lldb/test/API/lang/cpp/abi_tag_structors/TestAbiTagStructors.py new file mode 100644 index 0000000000000..b549736c1405d --- /dev/null +++ b/lldb/test/API/lang/cpp/abi_tag_structors/TestAbiTagStructors.py @@ -0,0 +1,30 @@ +""" +Test that we can call structors/destructors +annotated (and thus mangled) with ABI tags. +""" + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class AbiTagStructorsTestCase(TestBase): + def test_abi_tag_lookup(self): + self.build() + lldbutil.run_to_source_breakpoint( + self, "Break here", lldb.SBFileSpec("main.cpp", False) + ) + + self.expect_expr("Tagged()", result_type="Tagged") + self.expect_expr("t1 = t2", result_type="Tagged") + + self.expect("expr Tagged t3(t1)", error=False) + self.expect("expr t1.~Tagged()", error=False) + + # Calls to deleting and base object destructor variants (D0 and D2 in Itanium ABI) + self.expect_expr( + "struct D : public HasVirtualDtor {}; D d; d.func()", + result_type="int", + result_value="10", + ) diff --git a/lldb/test/API/lang/cpp/abi_tag_structors/main.cpp b/lldb/test/API/lang/cpp/abi_tag_structors/main.cpp new file mode 100644 index 0000000000000..b7783ccc49460 --- /dev/null +++ b/lldb/test/API/lang/cpp/abi_tag_structors/main.cpp @@ -0,0 +1,38 @@ +#include <cstdio> + +struct Tagged { + [[gnu::abi_tag("Default")]] Tagged() { std::puts(__func__); } + [[gnu::abi_tag("Copy")]] Tagged(const Tagged &) { std::puts(__func__); } + [[gnu::abi_tag("CopyAssign")]] Tagged &operator=(const Tagged &) { + std::puts(__func__); + return *this; + } + [[gnu::abi_tag("Dtor")]] ~Tagged() { std::puts(__func__); } +}; + +struct Base { + virtual ~Base() { std::puts(__func__); } + virtual int func() { return 5; } +}; + +struct HasVirtualDtor : public Base { + int func() override { return 10; } + + [[gnu::abi_tag("VirtualDtor")]] ~HasVirtualDtor() override { + std::puts(__func__); + } +}; + +int main() { + Tagged t1; + Tagged t2(t1); + t1 = t2; + + Base b; + HasVirtualDtor vdtor; + vdtor.func(); + + std::puts("Break here"); + + return 0; +} diff --git a/lldb/unittests/Expression/ExpressionTest.cpp b/lldb/unittests/Expression/ExpressionTest.cpp index 12f6dd515fd11..ceb567c28ab99 100644 --- a/lldb/unittests/Expression/ExpressionTest.cpp +++ b/lldb/unittests/Expression/ExpressionTest.cpp @@ -23,15 +23,15 @@ struct LabelTestCase { static LabelTestCase g_label_test_cases[] = { // Failure modes - {"bar:0x0:0x0:_Z3foov", + {"bar:blah:0x0:0x0:_Z3foov", {}, {"expected function call label prefix '$__lldb_func' but found 'bar' " "instead."}}, - {"$__lldb_func :0x0:0x0:_Z3foov", + {"$__lldb_func :blah:0x0:0x0:_Z3foov", {}, {"expected function call label prefix '$__lldb_func' but found " "'$__lldb_func ' instead."}}, - {"$__lldb_funcc:0x0:0x0:_Z3foov", + {"$__lldb_funcc:blah:0x0:0x0:_Z3foov", {}, {"expected function call label prefix '$__lldb_func' but found " "'$__lldb_funcc' instead."}}, @@ -39,47 +39,52 @@ static LabelTestCase g_label_test_cases[] = { {"foo", {}, {"malformed function call label."}}, {"$__lldb_func", {}, {"malformed function call label."}}, {"$__lldb_func:", {}, {"malformed function call label."}}, - {"$__lldb_func:0x0:0x0", {}, {"malformed function call label."}}, - {"$__lldb_func:abc:0x0:_Z3foov", + {"$__lldb_func:blah", {}, {"malformed function call label."}}, + {"$__lldb_func:blah:0x0", {}, {"malformed function call label."}}, + {"$__lldb_func:111:0x0:0x0", {}, {"malformed function call label."}}, + {"$__lldb_func:111:abc:0x0:_Z3foov", {}, {"failed to parse module ID from 'abc'."}}, - {"$__lldb_func:-1:0x0:_Z3foov", + {"$__lldb_func:111:-1:0x0:_Z3foov", {}, {"failed to parse module ID from '-1'."}}, - {"$__lldb_func:0x0invalid:0x0:_Z3foov", + {"$__lldb_func:111:0x0invalid:0x0:_Z3foov", {}, {"failed to parse module ID from '0x0invalid'."}}, - {"$__lldb_func:0x0 :0x0:_Z3foov", + {"$__lldb_func:111:0x0 :0x0:_Z3foov", {}, {"failed to parse module ID from '0x0 '."}}, - {"$__lldb_func:0x0:abc:_Z3foov", + {"$__lldb_func:blah:0x0:abc:_Z3foov", {}, {"failed to parse symbol ID from 'abc'."}}, - {"$__lldb_func:0x5:-1:_Z3foov", + {"$__lldb_func:blah:0x5:-1:_Z3foov", {}, {"failed to parse symbol ID from '-1'."}}, - {"$__lldb_func:0x5:0x0invalid:_Z3foov", + {"$__lldb_func:blah:0x5:0x0invalid:_Z3foov", {}, {"failed to parse symbol ID from '0x0invalid'."}}, - {"$__lldb_func:0x5:0x0 :_Z3foov", + {"$__lldb_func:blah:0x5:0x0 :_Z3foov", {}, {"failed to parse symbol ID from '0x0 '."}}, - {"$__lldb_func:0x0:0x0:_Z3foov", + {"$__lldb_func:blah:0x0:0x0:_Z3foov", { + /*.discriminator=*/"blah", /*.module_id=*/0x0, /*.symbol_id=*/0x0, /*.lookup_name=*/"_Z3foov", }, {}}, - {"$__lldb_func:0x0:0x0:abc:def:::a", + {"$__lldb_func::0x0:0x0:abc:def:::a", { + /*.discriminator=*/"", /*.module_id=*/0x0, /*.symbol_id=*/0x0, /*.lookup_name=*/"abc:def:::a", }, {}}, - {"$__lldb_func:0xd2:0xf0:$__lldb_func", + {"$__lldb_func:0x45:0xd2:0xf0:$__lldb_func", { + /*.discriminator=*/"0x45", /*.module_id=*/0xd2, /*.symbol_id=*/0xf0, /*.lookup_name=*/"$__lldb_func", @@ -106,6 +111,7 @@ TEST_P(ExpressionTestFixture, FunctionCallLabel) { EXPECT_EQ(decoded_or_err->toString(), encoded); EXPECT_EQ(label_str, encoded); + EXPECT_EQ(decoded_or_err->discriminator, label.discriminator); EXPECT_EQ(decoded_or_err->module_id, label.module_id); EXPECT_EQ(decoded_or_err->symbol_id, label.symbol_id); EXPECT_EQ(decoded_or_err->lookup_name, label.lookup_name); @@ -113,6 +119,7 @@ TEST_P(ExpressionTestFixture, FunctionCallLabel) { auto roundtrip_or_err = FunctionCallLabel::fromString(label_str); EXPECT_THAT_EXPECTED(roundtrip_or_err, llvm::Succeeded()); + EXPECT_EQ(roundtrip_or_err->discriminator, label.discriminator); EXPECT_EQ(roundtrip_or_err->module_id, label.module_id); EXPECT_EQ(roundtrip_or_err->symbol_id, label.symbol_id); EXPECT_EQ(roundtrip_or_err->lookup_name, label.lookup_name); diff --git a/lldb/unittests/Symbol/TestTypeSystemClang.cpp b/lldb/unittests/Symbol/TestTypeSystemClang.cpp index b993b82612497..f673cceae00dd 100644 --- a/lldb/unittests/Symbol/TestTypeSystemClang.cpp +++ b/lldb/unittests/Symbol/TestTypeSystemClang.cpp @@ -1150,12 +1150,12 @@ TEST_F(TestTypeSystemClang, AsmLabel_CtorDtor) { is_explicit, is_attr_used, is_artificial); auto *ctor = m_ast->AddMethodToCXXRecordType( - t.GetOpaqueQualType(), "S", /*asm_label=*/"$__lldb_func:0x0:0x0:S", + t.GetOpaqueQualType(), "S", /*asm_label=*/"$__lldb_func::0x0:0x0:S", function_type, lldb::AccessType::eAccessPublic, is_virtual, is_static, is_inline, is_explicit, is_attr_used, is_artificial); auto *dtor = m_ast->AddMethodToCXXRecordType( - t.GetOpaqueQualType(), "~S", /*asm_label=*/"$__lldb_func:0x0:0x0:~S", + t.GetOpaqueQualType(), "~S", /*asm_label=*/"$__lldb_func::0x0:0x0:~S", function_type, lldb::AccessType::eAccessPublic, is_virtual, is_static, is_inline, is_explicit, is_attr_used, is_artificial); @@ -1181,11 +1181,11 @@ TEST_F(TestTypeSystemClang, AsmLabel_CtorDtor) { EXPECT_STREQ(llvm::GlobalValue::dropLLVMManglingEscape( m_ast->DeclGetMangledName(ctor).GetStringRef()) .data(), - "$__lldb_func:0x0:0x0:S"); + "$__lldb_func:C0:0x0:0x0:S"); EXPECT_STREQ(llvm::GlobalValue::dropLLVMManglingEscape( m_ast->DeclGetMangledName(dtor).GetStringRef()) .data(), - "$__lldb_func:0x0:0x0:~S"); + "$__lldb_func:D1:0x0:0x0:~S"); } struct AsmLabelTestCase { @@ -1215,10 +1215,10 @@ class TestTypeSystemClangAsmLabel }; static AsmLabelTestCase g_asm_label_test_cases[] = { - {/*mangled=*/"$__lldb_func:0x0:0x0:_Z3foov", + {/*mangled=*/"$__lldb_func::0x0:0x0:_Z3foov", /*expected=*/"_Z3foov"}, - {/*mangled=*/"$__lldb_func:0x0:0x0:foo", - /*expected=*/"$__lldb_func:0x0:0x0:foo"}, + {/*mangled=*/"$__lldb_func::0x0:0x0:foo", + /*expected=*/"$__lldb_func::0x0:0x0:foo"}, {/*mangled=*/"foo", /*expected=*/"foo"}, {/*mangled=*/"_Z3foov", >From 6507359dd26f8a08223352f4781557dad1017a56 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Fri, 8 Aug 2025 09:25:40 +0100 Subject: [PATCH 5/8] fixup! remove type-unit special-case --- .../Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 9 --------- 1 file changed, 9 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 0b6a4148738d4..8ac68c8b76a30 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -287,15 +287,6 @@ static std::string MakeLLDBFuncAsmLabel(const DWARFDIE &die) { if (!cu) return {}; - // FIXME: When resolving function call labels, we check that - // that the definition's DW_AT_specification points to the - // declaration that we encoded into the label here. But if the - // declaration came from a type-unit (and the definition from - // .debug_info), that check won't work. So for now, don't use - // function call labels for declaration DIEs from type-units. - if (cu->IsTypeUnit()) - return {}; - SymbolFileDWARF *dwarf = die.GetDWARF(); if (!dwarf) return {}; >From ce75a0a3e6b6bda80cfa1cb17d324103ac1440e9 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Fri, 8 Aug 2025 09:37:25 +0100 Subject: [PATCH 6/8] fixup! clean up error handling --- .../SymbolFile/DWARF/SymbolFileDWARF.cpp | 33 ++++++++++++------- .../SymbolFile/DWARF/SymbolFileDWARF.h | 5 +-- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index 2dfb04cfd3ab5..457a75906490e 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -2535,8 +2535,9 @@ GetItaniumCtorDtorVariant(llvm::StringRef discriminator) { return ClangToItaniumDtorKind(static_cast<clang::CXXDtorType>(structor_kind)); } -DWARFDIE SymbolFileDWARF::FindFunctionDefinition(const FunctionCallLabel &label, - const DWARFDIE &declaration) { +llvm::Expected<DWARFDIE> +SymbolFileDWARF::FindFunctionDefinition(const FunctionCallLabel &label, + const DWARFDIE &declaration) { DWARFDIE definition; llvm::DenseMap<int, DWARFDIE> structor_variant_to_die; @@ -2570,6 +2571,8 @@ DWARFDIE SymbolFileDWARF::FindFunctionDefinition(const FunctionCallLabel &label, if (!mangled) return IterationAction::Continue; + // FIXME: we should make DWARF encode the structor variant instead of + // needing to re-demangle. llvm::ItaniumPartialDemangler D; if (D.partialDemangle(mangled)) return IterationAction::Continue; @@ -2594,7 +2597,9 @@ DWARFDIE SymbolFileDWARF::FindFunctionDefinition(const FunctionCallLabel &label, auto label_variant = GetItaniumCtorDtorVariant(label.discriminator); if (!label_variant) - return {}; + return llvm::createStringError( + llvm::formatv("failed to retrieve structor variant from label: {0}", + label.discriminator)); auto it = structor_variant_to_die.find(*label_variant); @@ -2604,13 +2609,16 @@ DWARFDIE SymbolFileDWARF::FindFunctionDefinition(const FunctionCallLabel &label, // We need a C1 constructor. If debug-info only contains a DIE for C2, // assume C1 was aliased to C2. + // + // FIXME: can DWARF encode this information for us? if (!label.lookup_name.starts_with("~") && label_variant == 1) { if (auto it = structor_variant_to_die.find(2); it != structor_variant_to_die.end()) return it->getSecond(); } - return {}; + return llvm::createStringError(llvm::formatv( + "failed to find structor variant DIE for label: {0}", label)); } llvm::Expected<SymbolContext> @@ -2625,20 +2633,21 @@ SymbolFileDWARF::ResolveFunctionCallLabel(const FunctionCallLabel &label) { // Label was created using a declaration DIE. Need to fetch the definition // to resolve the function call. if (die.GetAttributeValueAsUnsigned(llvm::dwarf::DW_AT_declaration, 0)) { - die = FindFunctionDefinition(label, die); - if (!die.IsValid()) - return llvm::createStringError( - llvm::formatv("failed to find definition DIE for {0}", label)); + auto die_or_err = FindFunctionDefinition(label, die); + if (!die_or_err) + return llvm::joinErrors( + llvm::createStringError("failed to find definition DIE"), + die_or_err.takeError()); + + die = *die_or_err; } SymbolContextList sc_list; if (!ResolveFunction(die, /*include_inlines=*/false, sc_list)) - return llvm::createStringError( - llvm::formatv("failed to resolve function for {0}", label)); + return llvm::createStringError("failed to resolve function"); if (sc_list.IsEmpty()) - return llvm::createStringError( - llvm::formatv("failed to find function for {0}", label)); + return llvm::createStringError("failed to find function"); assert(sc_list.GetSize() == 1); diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h index 843346619db37..ec1d6f62dbe7a 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h @@ -373,8 +373,9 @@ class SymbolFileDWARF : public SymbolFileCommon { DWARFIndex *getIndex() { return m_index.get(); } private: - DWARFDIE FindFunctionDefinition(const FunctionCallLabel &label, - const DWARFDIE &declaration); + llvm::Expected<DWARFDIE> + FindFunctionDefinition(const FunctionCallLabel &label, + const DWARFDIE &declaration); protected: SymbolFileDWARF(const SymbolFileDWARF &) = delete; >From 97f71409924764aa76348936b32cfb1346774fe7 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Fri, 8 Aug 2025 10:05:43 +0100 Subject: [PATCH 7/8] fixup! more adjustments to error messages --- lldb/source/Expression/IRExecutionUnit.cpp | 2 +- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lldb/source/Expression/IRExecutionUnit.cpp b/lldb/source/Expression/IRExecutionUnit.cpp index 5e40df282e7b0..6b07032539f83 100644 --- a/lldb/source/Expression/IRExecutionUnit.cpp +++ b/lldb/source/Expression/IRExecutionUnit.cpp @@ -798,7 +798,7 @@ ResolveFunctionCallLabel(const FunctionCallLabel &label, auto sc_or_err = symbol_file->ResolveFunctionCallLabel(label); if (!sc_or_err) return llvm::joinErrors( - llvm::createStringError("failed to resolve function by UID"), + llvm::createStringError("failed to resolve function by UID:"), sc_or_err.takeError()); SymbolContextList sc_list; diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index 457a75906490e..57f64aadef9ab 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -2618,7 +2618,7 @@ SymbolFileDWARF::FindFunctionDefinition(const FunctionCallLabel &label, } return llvm::createStringError(llvm::formatv( - "failed to find structor variant DIE for label: {0}", label)); + "failed to find DIE for structor variant: {0}", label.discriminator)); } llvm::Expected<SymbolContext> @@ -2636,7 +2636,7 @@ SymbolFileDWARF::ResolveFunctionCallLabel(const FunctionCallLabel &label) { auto die_or_err = FindFunctionDefinition(label, die); if (!die_or_err) return llvm::joinErrors( - llvm::createStringError("failed to find definition DIE"), + llvm::createStringError("failed to find definition DIE:"), die_or_err.takeError()); die = *die_or_err; >From bb10018c57c92206a23f993e1b25d0eb7cd0e454 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Fri, 8 Aug 2025 10:06:22 +0100 Subject: [PATCH 8/8] fixup! add test for ctor declaration separate from definition --- .../TestExprDefinitionInDylib.py | 11 +++++++++++ .../API/lang/cpp/expr-definition-in-dylib/lib.cpp | 10 ++++++++++ lldb/test/API/lang/cpp/expr-definition-in-dylib/lib.h | 7 +++++++ .../API/lang/cpp/expr-definition-in-dylib/main.cpp | 1 + 4 files changed, 29 insertions(+) diff --git a/lldb/test/API/lang/cpp/expr-definition-in-dylib/TestExprDefinitionInDylib.py b/lldb/test/API/lang/cpp/expr-definition-in-dylib/TestExprDefinitionInDylib.py index 02c34b3132722..61599d7d2b132 100644 --- a/lldb/test/API/lang/cpp/expr-definition-in-dylib/TestExprDefinitionInDylib.py +++ b/lldb/test/API/lang/cpp/expr-definition-in-dylib/TestExprDefinitionInDylib.py @@ -31,3 +31,14 @@ def test(self): ) self.expect_expr("f.method()", result_value="-72", result_type="int") + + self.expect_expr("Foo()", result_type="Foo") + + # FIXME: the function call label fails to resolve because the definition lives in a + # different CU than the declaration. The fallback lookup name-lookup fails because + # the structors are ABI-tagged. + self.expect( + "expr Bar()", + error=True, + substrs=["Couldn't look up symbols:", "$__lldb_func:C0"], + ) diff --git a/lldb/test/API/lang/cpp/expr-definition-in-dylib/lib.cpp b/lldb/test/API/lang/cpp/expr-definition-in-dylib/lib.cpp index ad148cebb00d1..1a08817f5cda1 100644 --- a/lldb/test/API/lang/cpp/expr-definition-in-dylib/lib.cpp +++ b/lldb/test/API/lang/cpp/expr-definition-in-dylib/lib.cpp @@ -1,3 +1,13 @@ #include "lib.h" +#include <cstdio> + int Foo::method() { return -72; } + +Foo::Foo() { std::puts(__func__); } + +Foo::~Foo() { std::puts(__func__); } + +Bar::Bar() { std::puts(__func__); } + +Bar::~Bar() { std::puts(__func__); } diff --git a/lldb/test/API/lang/cpp/expr-definition-in-dylib/lib.h b/lldb/test/API/lang/cpp/expr-definition-in-dylib/lib.h index 9568db2166ec4..5ec227946cba0 100644 --- a/lldb/test/API/lang/cpp/expr-definition-in-dylib/lib.h +++ b/lldb/test/API/lang/cpp/expr-definition-in-dylib/lib.h @@ -3,6 +3,13 @@ struct Foo { int method(); + Foo(); + ~Foo(); +}; + +struct Bar { + [[gnu::abi_tag("Ctor")]] Bar(); + [[gnu::abi_tag("Dtor")]] ~Bar(); }; #endif // LIB_H_IN diff --git a/lldb/test/API/lang/cpp/expr-definition-in-dylib/main.cpp b/lldb/test/API/lang/cpp/expr-definition-in-dylib/main.cpp index 2fddb2b7b3e74..4d6bece21ecac 100644 --- a/lldb/test/API/lang/cpp/expr-definition-in-dylib/main.cpp +++ b/lldb/test/API/lang/cpp/expr-definition-in-dylib/main.cpp @@ -2,5 +2,6 @@ int main() { Foo f; + Bar b; return f.method(); } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits