Author: Michael Buch Date: 2025-09-09T09:50:11+01:00 New Revision: 06d202b6cb13b38165af03dbf12a3beaac0b38ea
URL: https://github.com/llvm/llvm-project/commit/06d202b6cb13b38165af03dbf12a3beaac0b38ea DIFF: https://github.com/llvm/llvm-project/commit/06d202b6cb13b38165af03dbf12a3beaac0b38ea.diff LOG: [clang][DebugInfo] Emit unified (Itanium) mangled name to structor declarations (#154142) Depends on https://github.com/llvm/llvm-project/pull/154137 This patch is motivated by https://github.com/llvm/llvm-project/pull/149827, where we plan on using mangled names on structor declarations to find the exact structor definition that LLDB's expression evaluator should call. Given a `DW_TAG_subprogram` for a function declaration, the most convenient way for a debugger to find the corresponding definition is to use the `DW_AT_linkage_name` (i.e., the mangled name). However, we currently can't do that for constructors/destructors because Clang doesn't attach linkage names to them. This is because, depending on ABI, there can be multiple definitions for a single constructor/destructor declaration. The way GCC works around this is by producing a `C4`/`D4` "unified" mangling for structor declarations (see [godbolt](https://godbolt.org/z/Wds6cja9K)). GDB uses this to locate the relevant definitions. This patch aligns Clang with GCC's DWARF output and allows us to implement the same lookup scheme in LLDB. Added: clang/test/DebugInfo/CXX/local-structor-linkage-names.cpp clang/test/DebugInfo/CXX/structor-linkage-names.cpp Modified: clang/include/clang/Basic/ABI.h clang/include/clang/Basic/DebugOptions.def clang/include/clang/Driver/Options.td clang/lib/AST/ItaniumMangle.cpp clang/lib/AST/MicrosoftMangle.cpp clang/lib/CodeGen/CGClass.cpp clang/lib/CodeGen/CGDebugInfo.cpp clang/lib/CodeGen/CGDebugInfo.h clang/lib/CodeGen/ItaniumCXXABI.cpp clang/lib/CodeGen/MicrosoftCXXABI.cpp clang/lib/Driver/ToolChains/Clang.cpp clang/test/DebugInfo/CXX/artificial-arg.cpp clang/test/DebugInfo/ObjCXX/cyclic.mm Removed: ################################################################################ diff --git a/clang/include/clang/Basic/ABI.h b/clang/include/clang/Basic/ABI.h index 231bad799a42c..8279529c316cf 100644 --- a/clang/include/clang/Basic/ABI.h +++ b/clang/include/clang/Basic/ABI.h @@ -27,14 +27,16 @@ enum CXXCtorType { Ctor_Comdat, ///< The COMDAT used for ctors Ctor_CopyingClosure, ///< Copying closure variant of a ctor Ctor_DefaultClosure, ///< Default closure variant of a ctor + Ctor_Unified, ///< GCC-style unified dtor }; /// C++ destructor types. enum CXXDtorType { - Dtor_Deleting, ///< Deleting dtor - Dtor_Complete, ///< Complete object dtor - Dtor_Base, ///< Base object dtor - Dtor_Comdat ///< The COMDAT used for dtors + Dtor_Deleting, ///< Deleting dtor + Dtor_Complete, ///< Complete object dtor + Dtor_Base, ///< Base object dtor + Dtor_Comdat, ///< The COMDAT used for dtors + Dtor_Unified, ///< GCC-style unified dtor }; } // end namespace clang diff --git a/clang/include/clang/Basic/DebugOptions.def b/clang/include/clang/Basic/DebugOptions.def index c6e736e92744c..a768b12fa4e0d 100644 --- a/clang/include/clang/Basic/DebugOptions.def +++ b/clang/include/clang/Basic/DebugOptions.def @@ -125,6 +125,12 @@ DEBUGOPT(DebugNameTable, 2, 0, Compatible) /// Whether to use DWARF base address specifiers in .debug_ranges. DEBUGOPT(DebugRangesBaseAddress, 1, 0, Compatible) +/// Whether to add linkage names to constructor/destructor declarations. +/// This is an escape hatch for cases where attaching the additional linkage +/// names would increase debug-info size (particularly the .debug_str section) +/// too much. +DEBUGOPT(DebugStructorDeclLinkageNames, 1, 0, Benign) + /// Whether to embed source in DWARF debug line section. DEBUGOPT(EmbedSource, 1, 0, Compatible) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 718808d583e8c..ea5a94f840470 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -4789,6 +4789,18 @@ def gembed_source : Flag<["-"], "gembed-source">, Group<g_flags_Group>, def gno_embed_source : Flag<["-"], "gno-embed-source">, Group<g_flags_Group>, Flags<[NoXarchOption]>, HelpText<"Restore the default behavior of not embedding source text in DWARF debug sections">; +defm structor_decl_linkage_names + : BoolGOption<"structor-decl-linkage-names", + CodeGenOpts<"DebugStructorDeclLinkageNames">, DefaultTrue, + NegFlag<SetFalse>, + PosFlag<SetTrue, [], [], + "Attach linkage names to C++ constructor/destructor " + "declarations in DWARF." + "Implies -g.">, + BothFlags<[], [ClangOption, CLOption, CC1Option]>>, + DocBrief<[{On some ABIs (e.g., Itanium), constructors and destructors may have multiple variants. Historically, when generating DWARF, Clang did not attach ``DW_AT_linkage_name``s to structor DIEs because there were multiple possible manglings (depending on the structor variant) that could be used. With ``-gstructor-decl-linkage-names``, for ABIs with structor variants, we attach a "unified" mangled name to structor declarations DIEs which debuggers can use to look up all the definitions for a structor declaration. E.g., a "unified" mangled name ``_ZN3FooC4Ev`` may have multiple definitions associated with it such as ``_ZN3FooC1Ev`` and ``_ZN3FooC2Ev``. + +Enabling this flag results in a better interactive debugging experience (both GDB and LLDB have support for understanding these "unified" linkage names). However, it comes with a significant increase in debug-info size (particularly the `.debug_str` section). As an escape hatch, users can disable this feature using ``-gno-structor-decl-linkage-names``.}]>; defm key_instructions : BoolGOption<"key-instructions", CodeGenOpts<"DebugKeyInstructions">, DefaultFalse, NegFlag<SetFalse>, PosFlag<SetTrue, [], [], diff --git a/clang/lib/AST/ItaniumMangle.cpp b/clang/lib/AST/ItaniumMangle.cpp index ffadfce67d631..163cd43abd45a 100644 --- a/clang/lib/AST/ItaniumMangle.cpp +++ b/clang/lib/AST/ItaniumMangle.cpp @@ -6026,6 +6026,8 @@ void CXXNameMangler::mangleCXXCtorType(CXXCtorType T, // ::= CI2 <type> # base inheriting constructor // // In addition, C5 is a comdat name with C1 and C2 in it. + // C4 represents a ctor declaration and is used by debuggers to look up + // the various ctor variants. Out << 'C'; if (InheritedFrom) Out << 'I'; @@ -6036,6 +6038,9 @@ void CXXNameMangler::mangleCXXCtorType(CXXCtorType T, case Ctor_Base: Out << '2'; break; + case Ctor_Unified: + Out << '4'; + break; case Ctor_Comdat: Out << '5'; break; @@ -6053,6 +6058,8 @@ void CXXNameMangler::mangleCXXDtorType(CXXDtorType T) { // ::= D2 # base object destructor // // In addition, D5 is a comdat name with D1, D2 and, if virtual, D0 in it. + // D4 represents a dtor declaration and is used by debuggers to look up + // the various dtor variants. switch (T) { case Dtor_Deleting: Out << "D0"; @@ -6063,6 +6070,9 @@ void CXXNameMangler::mangleCXXDtorType(CXXDtorType T) { case Dtor_Base: Out << "D2"; break; + case Dtor_Unified: + Out << "D4"; + break; case Dtor_Comdat: Out << "D5"; break; diff --git a/clang/lib/AST/MicrosoftMangle.cpp b/clang/lib/AST/MicrosoftMangle.cpp index 2ac38a24456c7..d96472e393f68 100644 --- a/clang/lib/AST/MicrosoftMangle.cpp +++ b/clang/lib/AST/MicrosoftMangle.cpp @@ -1496,6 +1496,8 @@ void MicrosoftCXXNameMangler::mangleCXXDtorType(CXXDtorType T) { // it. case Dtor_Comdat: llvm_unreachable("not expecting a COMDAT"); + case Dtor_Unified: + llvm_unreachable("not expecting a unified dtor type"); } llvm_unreachable("Unsupported dtor type?"); } diff --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp index bae55aa1e1928..6742c196053a2 100644 --- a/clang/lib/CodeGen/CGClass.cpp +++ b/clang/lib/CodeGen/CGClass.cpp @@ -1481,6 +1481,8 @@ void CodeGenFunction::EmitDestructorBody(FunctionArgList &Args) { // we'd introduce *two* handler blocks. In the Microsoft ABI, we // always delegate because we might not have a definition in this TU. switch (DtorType) { + case Dtor_Unified: + llvm_unreachable("not expecting a unified dtor"); case Dtor_Comdat: llvm_unreachable("not expecting a COMDAT"); case Dtor_Deleting: llvm_unreachable("already handled deleting case"); diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 0385dbdac869b..578d09f7971d6 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -2177,24 +2177,47 @@ static bool isFunctionLocalClass(const CXXRecordDecl *RD) { return false; } +llvm::StringRef +CGDebugInfo::GetMethodLinkageName(const CXXMethodDecl *Method) const { + assert(Method); + + const bool IsCtorOrDtor = + isa<CXXConstructorDecl>(Method) || isa<CXXDestructorDecl>(Method); + + if (IsCtorOrDtor && !CGM.getCodeGenOpts().DebugStructorDeclLinkageNames) + return {}; + + // In some ABIs (particularly Itanium) a single ctor/dtor + // corresponds to multiple functions. Attach a "unified" + // linkage name for those (which is the convention GCC uses). + // Otherwise, attach no linkage name. + if (IsCtorOrDtor && !CGM.getTarget().getCXXABI().hasConstructorVariants()) + return {}; + + if (const auto *Ctor = llvm::dyn_cast<CXXConstructorDecl>(Method)) + return CGM.getMangledName(GlobalDecl(Ctor, CXXCtorType::Ctor_Unified)); + + if (const auto *Dtor = llvm::dyn_cast<CXXDestructorDecl>(Method)) + return CGM.getMangledName(GlobalDecl(Dtor, CXXDtorType::Dtor_Unified)); + + return CGM.getMangledName(Method); +} + llvm::DISubprogram *CGDebugInfo::CreateCXXMemberFunction( const CXXMethodDecl *Method, llvm::DIFile *Unit, llvm::DIType *RecordTy) { - bool IsCtorOrDtor = - isa<CXXConstructorDecl>(Method) || isa<CXXDestructorDecl>(Method); + assert(Method); StringRef MethodName = getFunctionName(Method); llvm::DISubroutineType *MethodTy = getOrCreateMethodType(Method, Unit); - // Since a single ctor/dtor corresponds to multiple functions, it doesn't - // make sense to give a single ctor/dtor a linkage name. StringRef MethodLinkageName; // FIXME: 'isFunctionLocalClass' seems like an arbitrary/unintentional // property to use here. It may've been intended to model "is non-external // type" but misses cases of non-function-local but non-external classes such // as those in anonymous namespaces as well as the reverse - external types // that are function local, such as those in (non-local) inline functions. - if (!IsCtorOrDtor && !isFunctionLocalClass(Method->getParent())) - MethodLinkageName = CGM.getMangledName(Method); + if (!isFunctionLocalClass(Method->getParent())) + MethodLinkageName = GetMethodLinkageName(Method); // Get the location for the method. llvm::DIFile *MethodDefUnit = nullptr; diff --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h index ff9c3cd2d1136..f86077369a42a 100644 --- a/clang/lib/CodeGen/CGDebugInfo.h +++ b/clang/lib/CodeGen/CGDebugInfo.h @@ -899,6 +899,10 @@ class CGDebugInfo { std::memcpy(Data + A.size(), B.data(), B.size()); return StringRef(Data, A.size() + B.size()); } + + /// If one exists, returns the linkage name of the specified \ + /// (non-null) \c Method. Returns empty string otherwise. + llvm::StringRef GetMethodLinkageName(const CXXMethodDecl *Method) const; }; /// A scoped helper to set the current debug location to the specified diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp index d5b5fd7200855..7dc2eaf1e9f75 100644 --- a/clang/lib/CodeGen/ItaniumCXXABI.cpp +++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp @@ -91,6 +91,8 @@ class ItaniumCXXABI : public CodeGen::CGCXXABI { case Dtor_Comdat: llvm_unreachable("emitting dtor comdat as function?"); + case Dtor_Unified: + llvm_unreachable("emitting unified dtor as function?"); } llvm_unreachable("bad dtor kind"); } @@ -108,6 +110,9 @@ class ItaniumCXXABI : public CodeGen::CGCXXABI { case Ctor_Comdat: llvm_unreachable("emitting ctor comdat as function?"); + + case Ctor_Unified: + llvm_unreachable("emitting unified ctor as function?"); } llvm_unreachable("bad dtor kind"); } diff --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp b/clang/lib/CodeGen/MicrosoftCXXABI.cpp index 88f0648660965..94190a149e859 100644 --- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp +++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp @@ -77,6 +77,8 @@ class MicrosoftCXXABI : public CGCXXABI { return false; case Dtor_Comdat: llvm_unreachable("emitting dtor comdat as function?"); + case Dtor_Unified: + llvm_unreachable("unexpected unified dtor type"); } llvm_unreachable("bad dtor kind"); } @@ -1417,6 +1419,8 @@ llvm::GlobalValue::LinkageTypes MicrosoftCXXABI::getCXXDestructorLinkage( // and are emitted everywhere they are used. They are internal if the class // is internal. return llvm::GlobalValue::LinkOnceODRLinkage; + case Dtor_Unified: + llvm_unreachable("MS C++ ABI does not support unified dtors"); case Dtor_Comdat: llvm_unreachable("MS C++ ABI does not support comdat dtors"); } diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 21e45c6b56bbb..a357a8828b13d 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -4608,6 +4608,10 @@ renderDebugOptions(const ToolChain &TC, const Driver &D, const llvm::Triple &T, KeyInstructionsOnByDefault)) CmdArgs.push_back("-gkey-instructions"); + if (!Args.hasFlag(options::OPT_gstructor_decl_linkage_names, + options::OPT_gno_structor_decl_linkage_names, true)) + CmdArgs.push_back("-gno-structor-decl-linkage-names"); + if (EmitCodeView) { CmdArgs.push_back("-gcodeview"); diff --git a/clang/test/DebugInfo/CXX/artificial-arg.cpp b/clang/test/DebugInfo/CXX/artificial-arg.cpp index a0cf131f83e15..21b8d047b3456 100644 --- a/clang/test/DebugInfo/CXX/artificial-arg.cpp +++ b/clang/test/DebugInfo/CXX/artificial-arg.cpp @@ -25,7 +25,8 @@ int main(int argc, char **argv) { // CHECK: ![[CLASSTYPE:.*]] = distinct !DICompositeType(tag: DW_TAG_class_type, name: "A", // CHECK-SAME: identifier: "_ZTS1A" // CHECK: ![[ARTARG:.*]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[CLASSTYPE]],{{.*}} DIFlagArtificial -// CHECK: !DISubprogram(name: "A", scope: ![[CLASSTYPE]] +// CHECK: !DISubprogram(name: "A" +// CHECK-SAME: scope: ![[CLASSTYPE]] // CHECK-SAME: line: 12 // CHECK-SAME: DIFlagPublic // CHECK: !DISubroutineType(types: [[FUNCTYPE:![0-9]*]]) diff --git a/clang/test/DebugInfo/CXX/local-structor-linkage-names.cpp b/clang/test/DebugInfo/CXX/local-structor-linkage-names.cpp new file mode 100644 index 0000000000000..4b4261ee3fbd8 --- /dev/null +++ b/clang/test/DebugInfo/CXX/local-structor-linkage-names.cpp @@ -0,0 +1,29 @@ +// Tests that we emit don't emit unified constructor/destructor linkage names +// for function-local constructors. + +// Check with -gstructor-decl-linkage-names. +// RUN: %clang_cc1 -triple aarch64-apple-macosx -emit-llvm -debug-info-kind=standalone \ +// RUN: -gstructor-decl-linkage-names %s -o - | FileCheck %s --check-prefixes=CHECK +// +// Check with -gno-structor-decl-linkage-names. +// RUN: %clang_cc1 -triple aarch64-apple-macosx -emit-llvm -debug-info-kind=standalone \ +// RUN: -gno-structor-decl-linkage-names %s -o - | FileCheck %s --check-prefixes=CHECK + +struct HasNestedCtor { + HasNestedCtor(); +}; + +HasNestedCtor::HasNestedCtor() { + struct Local { + Local() {} + ~Local() {} + } l; +} + +// CHECK: !DISubprogram(name: "Local" +// CHECK-NOT: linkageName +// CHECK-SAME: ) + +// CHECK: !DISubprogram(name: "~Local" +// CHECK-NOT: linkageName +// CHECK-SAME: ) diff --git a/clang/test/DebugInfo/CXX/structor-linkage-names.cpp b/clang/test/DebugInfo/CXX/structor-linkage-names.cpp new file mode 100644 index 0000000000000..b7aac198c5180 --- /dev/null +++ b/clang/test/DebugInfo/CXX/structor-linkage-names.cpp @@ -0,0 +1,89 @@ +// Tests that we emit unified constructor/destructor linkage names +// for ABIs that support it. + +// Check that -gstructor-decl-linkage-names is the default. +// RUN: %clang_cc1 -triple aarch64-apple-macosx -emit-llvm -debug-info-kind=standalone \ +// RUN: %s -o - | FileCheck %s --check-prefixes=CHECK,ITANIUM +// +// Check with -gstructor-decl-linkage-names. +// RUN: %clang_cc1 -triple aarch64-apple-macosx -emit-llvm -debug-info-kind=standalone \ +// RUN: -gstructor-decl-linkage-names %s -o - | FileCheck %s --check-prefixes=CHECK,ITANIUM +// +// Check with -gno-structor-decl-linkage-names. +// RUN: %clang_cc1 -triple aarch64-apple-macosx -emit-llvm -debug-info-kind=standalone \ +// RUN: -gno-structor-decl-linkage-names %s -o - | FileCheck %s --check-prefixes=CHECK,DISABLE +// +// Check ABI without structor variants. +// RUN: %clang_cc1 -triple x86_64-windows-msvc -emit-llvm -debug-info-kind=standalone \ +// RUN: -gstructor-decl-linkage-names %s -o - | FileCheck %s --check-prefixes=CHECK,MSABI + +struct Base { + Base(int x); + ~Base(); +}; + +Base::Base(int x) {} +Base::~Base() {} + +// Check that we emit unified ctor/dtor (C4/D4) on Itanium but not for MS-ABI. + +// CHECK: ![[BASE_CTOR_DECL:[0-9]+]] = !DISubprogram(name: "Base" +// MSABI-NOT: linkageName: +// DISABLE-NOT: linkageName: +// ITANIUM-SAME: linkageName: "_ZN4BaseC4Ei" +// CHECK-SAME: spFlags: 0 + +// CHECK: ![[BASE_DTOR_DECL:[0-9]+]] = !DISubprogram(name: "~Base" +// MSABI-NOT: linkageName: +// DISABLE-NOT: linkageName: +// ITANIUM-SAME: linkageName: "_ZN4BaseD4Ev" +// CHECK-SAME: spFlags: 0 + +// Check that the ctor/dtor definitions have linkage names that aren't +// the ones on the declaration. + +// CHECK: !DISubprogram(name: "Base" +// MSABI-SAME: linkageName: +// ITANIUM-SAME: linkageName: "_ZN4BaseC2Ei" +// CHECK-SAME: spFlags: DISPFlagDefinition +// CHECK-SAME: declaration: ![[BASE_CTOR_DECL]] + +// ITANIUM: !DISubprogram(name: "Base" +// ITANIUM-SAME: linkageName: "_ZN4BaseC1Ei" +// ITANIUM-SAME: spFlags: DISPFlagDefinition +// ITANIUM-SAME: declaration: ![[BASE_CTOR_DECL]] + +// CHECK: !DISubprogram(name: "~Base" +// MSABI-SAME: linkageName: +// ITANIUM-SAME: linkageName: "_ZN4BaseD2Ev" +// CHECK-SAME: spFlags: DISPFlagDefinition +// CHECK-SAME: declaration: ![[BASE_DTOR_DECL]] + +// ITANIUM: !DISubprogram(name: "~Base" +// ITANIUM-SAME: linkageName: "_ZN4BaseD1Ev" +// ITANIUM-SAME: spFlags: DISPFlagDefinition +// ITANIUM-SAME: declaration: ![[BASE_DTOR_DECL]] + +struct Derived : public Base { + using Base::Base; +} d(5); + +// CHECK: !DISubprogram(name: "Base" +// MSABI-SAME: linkageName: +// ITANIUM-SAME: linkageName: "_ZN7DerivedCI14BaseEi" +// CHECK-SAME: spFlags: {{.*}}DISPFlagDefinition +// CHECK-SAME: declaration: ![[BASE_INHERIT_CTOR_DECL:[0-9]+]] + +// CHECK: [[BASE_INHERIT_CTOR_DECL]] = !DISubprogram(name: "Base" +// MSABI-NOT: linkageName: +// DISABLE-NOT: linkageName: +// ITANIUM-SAME: linkageName: "_ZN7DerivedCI44BaseEi" +// CHECK-SAME spFlags: 0 + +// ITANIUM: !DISubprogram(name: "Base" +// ITANIUM-SAME: linkageName: "_ZN7DerivedCI24BaseEi" +// ITANIUM-SAME: spFlags: DISPFlagDefinition +// ITANIUM-SAME: declaration: ![[BASE_INHERIT_CTOR_DECL:[0-9]+]] + +// MSABI: !DISubprogram(name: "~Derived" +// DISABLE: !DISubprogram(name: "~Derived" diff --git a/clang/test/DebugInfo/ObjCXX/cyclic.mm b/clang/test/DebugInfo/ObjCXX/cyclic.mm index 2fb1611c904d0..a062b6ad50612 100644 --- a/clang/test/DebugInfo/ObjCXX/cyclic.mm +++ b/clang/test/DebugInfo/ObjCXX/cyclic.mm @@ -10,8 +10,9 @@ // CHECK-SAME: identifier: // CHECK: ![[BMEMBERS]] = !{![[BB:[0-9]+]]} B(struct A *); -// CHECK: ![[BB]] = !DISubprogram(name: "B", scope: ![[B]] -// CHECK-SAME: line: [[@LINE-2]], +// CHECK: ![[BB]] = !DISubprogram(name: "B", +// CHECK-SAME: scope: ![[B]] +// CHECK-SAME: line: [[@LINE-3]], // CHECK-SAME: type: ![[TY:[0-9]+]], // CHECK: ![[TY]] = !DISubroutineType(types: ![[ARGS:[0-9]+]]) // CHECK: ![[ARGS]] = !{null, ![[THIS:[0-9]+]], !{{[^,]+}}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
