[clang][DebugInfo] Emit unified (Itanium) mangled name to structor declarations

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

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)
 

clang/include/clang/Driver/Options.td

@@ -4767,6 +4767,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, [], [],

clang/lib/AST/ItaniumMangle.cpp

@@ -6029,6 +6029,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';
@@ -6039,6 +6041,9 @@ void CXXNameMangler::mangleCXXCtorType(CXXCtorType T,
   case Ctor_Base:
     Out << '2';
     break;
+  case Ctor_Unified:
+    Out << '4';
+    break;
   case Ctor_Comdat:
     Out << '5';
     break;
@@ -6056,6 +6061,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";
@@ -6066,6 +6073,9 @@ void CXXNameMangler::mangleCXXDtorType(CXXDtorType T) {
   case Dtor_Base:
     Out << "D2";
     break;
+  case Dtor_Unified:
+    Out << "D4";
+    break;
   case Dtor_Comdat:
     Out << "D5";
     break;

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?");
 }

clang/lib/CodeGen/CGClass.cpp

@@ -1499,6 +1499,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");
 

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;

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

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");
     }

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");
   }

clang/lib/Driver/ToolChains/Clang.cpp

@@ -4595,6 +4595,10 @@ renderDebugOptions(const ToolChain &TC, const Driver &D, const llvm::Triple &T,
                    options::OPT_gno_key_instructions, false))
     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");
 

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]*]])

clang/test/DebugInfo/CXX/debug-info-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"

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]+]], !{{[^,]+}}}

llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp

@@ -1403,11 +1403,8 @@ bool DwarfUnit::applySubprogramDefinitionAttributes(const DISubprogram *SP,
 
   // Add the linkage name if we have one and it isn't in the Decl.
   StringRef LinkageName = SP->getLinkageName();
-  assert(((LinkageName.empty() || DeclLinkageName.empty()) ||
-          LinkageName == DeclLinkageName) &&
-         "decl has a linkage name and it is different");
-  if (DeclLinkageName.empty() &&
-      // Always emit it for abstract subprograms.
+  // Always emit linkage name for abstract subprograms.
+  if (DeclLinkageName != LinkageName &&
       (DD->useAllLinkageNames() || DU->getAbstractScopeDIEs().lookup(SP)))
     addLinkageName(SPDie, LinkageName);