Reland "[DebugMetadata][DwarfDebug] Support function-local types in lexical block scopes (4/7)"

dzhidzhoev · dzhidzhoev · commit bfee69658259 · 2025-10-27T12:37:43.000+01:00
This is an attempt to merge https://reviews.llvm.org/D144006 with LTO fix. The last merge attempt was #75385. The issue with it was investigated in #75385 (comment). If the same (in the sense of ODR identifier/ODR uniquing rules) local type is present in two modules, and these modules are linked together, the type gets uniqued. A DIType, that happens to be loaded first, survives linking, and the references on other types with the same ODR identifier from the modules loaded later are replaced with the references on the DIType loaded first. Since defintion subprograms, in scope of which these types are located, are not deduplicated, the linker output may contain multiple DISubprogram's having the same (uniqued) type in their retainedNodes lists. Further compilation of such modules causes crashes. To tackle that, * previous solution to handle LTO linking with local types in retainedNodes is removed (cloneLocalTypes() function), * for each loaded distinct (definition) DISubprogram, its retainedNodes list is scanned after loading, and DITypes with a scope of another subprogram are removed. If something from a Function corresponding to the DISubprogram references uniqued type, we rely on cross-CU links. With this approach, clang builds without crashes in FullLTO (which is not the case for #75385). Additionally: * a check is added to Verifier to report about local types located in a wrong retainedNodes list, * DIBuilder's methods for type creation are updated, as https://reviews.llvm.org/D144006 has gotten slightly out-of-date. Commit #75385 and the new changes are placed in separate commits to simplify review process.
diff --git a/clang/test/DebugInfo/CXX/lambda-capture-packs.cpp b/clang/test/DebugInfo/CXX/lambda-capture-packs.cpp
@@ -2,28 +2,22 @@
 // RUN:   -debug-info-kind=standalone -std=c++26 %s -o - | FileCheck %s
 
 
-// CHECK: ![[PACK1:[0-9]+]] = distinct !DISubprogram(name: "capture_pack<int>"
-// CHECK: ![[PACK2:[0-9]+]] = distinct !DISubprogram(name: "capture_pack<int, int>"
-// CHECK: ![[PACK3:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_locals<int>"
-// CHECK: ![[PACK4:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_locals<int, int>"
-// CHECK: ![[PACK5:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_this<int>"
-// CHECK: ![[PACK6:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_this<int, int>"
-// CHECK: ![[PACK7:[0-9]+]] = distinct !DISubprogram(name: "capture_binding_and_param_pack<int, int>"
-
 template<typename... Args>
 auto capture_pack(Args... args) {
   return [args..., ...params = args] {
     return 0;
   }();
 }
 
+// CHECK: ![[PACK1:[0-9]+]] = distinct !DISubprogram(name: "capture_pack<int>"
 // CHECK:      distinct !DICompositeType(tag: DW_TAG_class_type, scope: ![[PACK1]]
 // CHECK-SAME:                           elements: ![[PACK1_ELEMS:[0-9]+]]
 // CHECK-DAG:  ![[PACK1_ELEMS]] = !{![[PACK1_ARGS:[0-9]+]], ![[PACK1_PARAMS:[0-9]+]]}
 // CHECK-DAG:  ![[PACK1_ARGS]] = !DIDerivedType(tag: DW_TAG_member, name: "args"
 // CHECK-DAG:  ![[PACK1_PARAMS]] = !DIDerivedType(tag: DW_TAG_member, name: "params"
 // CHECK-NOT:  DW_TAG_member
 
+// CHECK: ![[PACK2:[0-9]+]] = distinct !DISubprogram(name: "capture_pack<int, int>"
 // CHECK:      distinct !DICompositeType(tag: DW_TAG_class_type, scope: ![[PACK2]]
 // CHECK-SAME:                           elements: ![[PACK2_ELEMS:[0-9]+]]
 // CHECK:      ![[PACK2_ELEMS]] = !{![[PACK2_ARGS:[0-9]+]]
@@ -42,6 +36,7 @@ auto capture_pack_and_locals(int x, Args... args) {
   }();
 }
 
+// CHECK: ![[PACK3:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_locals<int>"
 // CHECK:      distinct !DICompositeType(tag: DW_TAG_class_type, scope: ![[PACK3]]
 // CHECK-SAME:                           elements: ![[PACK3_ELEMS:[0-9]+]]
 // CHECK:      ![[PACK3_ELEMS]] = !{![[PACK3_ARGS:[0-9]+]]
@@ -55,6 +50,7 @@ auto capture_pack_and_locals(int x, Args... args) {
 // CHECK-DAG:  ![[PACK3_W]] = !DIDerivedType(tag: DW_TAG_member, name: "w"
 // CHECK-NOT:  DW_TAG_member
 
+// CHECK: ![[PACK4:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_locals<int, int>"
 // CHECK:      distinct !DICompositeType(tag: DW_TAG_class_type, scope: ![[PACK4]]
 // CHECK-SAME:                           elements: ![[PACK4_ELEMS:[0-9]+]]
 // CHECK:      ![[PACK4_ELEMS]] = !{![[PACK4_ARGS:[0-9]+]]
@@ -90,6 +86,7 @@ struct Foo {
   int w = 10;
 } f;
 
+// CHECK: ![[PACK5:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_this<int>"
 // CHECK:      distinct !DICompositeType(tag: DW_TAG_class_type, scope: ![[PACK5]]
 // CHECK-SAME:                           elements: ![[PACK5a_ELEMS:[0-9]+]]
 // CHECK:      ![[PACK5a_ELEMS]] = !{![[PACK5a_THIS:[0-9]+]]
@@ -120,6 +117,7 @@ struct Foo {
 // CHECK-DAG:  ![[PACK5c_THIS]] = !DIDerivedType(tag: DW_TAG_member, name: "this"
 // CHECK-NOT:  DW_TAG_member
 
+// CHECK: ![[PACK6:[0-9]+]] = distinct !DISubprogram(name: "capture_pack_and_this<int, int>"
 // CHECK:      distinct !DICompositeType(tag: DW_TAG_class_type, scope: ![[PACK6]]
 // CHECK-SAME:                           elements: ![[PACK6a_ELEMS:[0-9]+]]
 // CHECK:      ![[PACK6a_ELEMS]] = !{![[PACK6a_THIS:[0-9]+]]
@@ -168,6 +166,7 @@ auto capture_binding_and_param_pack(Args... args) {
   }();
 }
 
+// CHECK: ![[PACK7:[0-9]+]] = distinct !DISubprogram(name: "capture_binding_and_param_pack<int, int>"
 // CHECK: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "C"
 // CHECK:      distinct !DICompositeType(tag: DW_TAG_class_type, scope: ![[PACK7]]
 // CHECK-SAME:                           elements: ![[PACK7_ELEMS:[0-9]+]]
diff --git a/llvm/include/llvm/AsmParser/LLParser.h b/llvm/include/llvm/AsmParser/LLParser.h
@@ -181,6 +181,10 @@ namespace llvm {
     /// Keeps track of source locations for Values, BasicBlocks, and Functions.
     AsmParserContext *ParserContext;
 
+    /// retainedNodes of these subprograms should be cleaned up from incorrectly
+    /// scoped local types.
+    SmallVector<DISubprogram *> NewDistinctSPs;
+
     /// Only the llvm-as tool may set this to false to bypass
     /// UpgradeDebuginfo so it can generate broken bitcode.
     bool UpgradeDebugInfo;
diff --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -2536,6 +2536,21 @@ class DISubprogram : public DILocalScope {
     replaceOperandWith(7, N.get());
   }
 
+  /// Remove types that do not belong to the subprogram's scope from
+  /// retainedNodes list.
+  void cleanupRetainedNodes();
+
+  /// When DebugTypeODRUniquing is enabled, after multiple modules are loaded,
+  /// some subprograms (that are from different compilation units, usually)
+  /// may have references to the same local type in their retainedNodes lists.
+  ///
+  /// Clean up such references.
+  template <typename RangeT>
+  static void cleanupRetainedNodes(const RangeT &NewDistinctSPs) {
+    for (DISubprogram *SP : NewDistinctSPs)
+      SP->cleanupRetainedNodes();
+  }
+
   /// Check if this subprogram describes the given function.
   ///
   /// FIXME: Should this be looking through bitcasts?
diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp
@@ -54,6 +54,7 @@
 #include <cassert>
 #include <cstring>
 #include <optional>
+#include <utility>
 #include <vector>
 
 using namespace llvm;
@@ -428,6 +429,8 @@ bool LLParser::validateEndOfModule(bool UpgradeDebugInfo) {
       N.second->resolveCycles();
   }
 
+  DISubprogram::cleanupRetainedNodes(std::exchange(NewDistinctSPs, {}));
+
   for (auto *Inst : InstsWithTBAATag) {
     MDNode *MD = Inst->getMetadata(LLVMContext::MD_tbaa);
     // With incomplete IR, the tbaa metadata may have been dropped.
@@ -5987,6 +5990,10 @@ bool LLParser::parseDISubprogram(MDNode *&Result, bool IsDistinct) {
        thisAdjustment.Val, flags.Val, SPFlags, unit.Val, templateParams.Val,
        declaration.Val, retainedNodes.Val, thrownTypes.Val, annotations.Val,
        targetFuncName.Val, keyInstructions.Val));
+
+  if (IsDistinct)
+    NewDistinctSPs.push_back(cast<DISubprogram>(Result));
+
   return false;
 }
 
diff --git a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
@@ -452,6 +452,10 @@ class MetadataLoader::MetadataLoaderImpl {
   /// metadata.
   SmallDenseMap<Function *, DISubprogram *, 16> FunctionsWithSPs;
 
+  /// retainedNodes of these subprograms should be cleaned up from incorrectly
+  /// scoped local types.
+  SmallVector<DISubprogram *> NewDistinctSPs;
+
   // Map the bitcode's custom MDKind ID to the Module's MDKind ID.
   DenseMap<unsigned, unsigned> MDKindMap;
 
@@ -732,29 +736,7 @@ class MetadataLoader::MetadataLoaderImpl {
     upgradeCUVariables();
     if (ModuleLevel)
       upgradeCULocals();
-  }
-
-  void cloneLocalTypes() {
-    for (Metadata *M : MetadataList) {
-      if (auto *SP = dyn_cast_or_null<DISubprogram>(M)) {
-        auto RetainedNodes = SP->getRetainedNodes();
-        SmallVector<Metadata *> MDs(RetainedNodes.begin(), RetainedNodes.end());
-        bool HasChanged = false;
-        for (auto &N : MDs)
-          if (auto *T = dyn_cast<DIType>(N))
-            if (auto *LS = dyn_cast_or_null<DILocalScope>(T->getScope()))
-              if (auto *Parent = findEnclosingSubprogram(LS))
-                if (Parent != SP) {
-                  HasChanged = true;
-                  auto NewT = T->clone();
-                  NewT->replaceOperandWith(1, SP);
-                  N = MDNode::replaceWithUniqued(std::move(NewT));
-                }
-
-        if (HasChanged)
-          SP->replaceRetainedNodes(MDNode::get(Context, MDs));
-      }
-    }
+    DISubprogram::cleanupRetainedNodes(std::exchange(NewDistinctSPs, {}));
   }
 
   void callMDTypeCallback(Metadata **Val, unsigned TypeID);
@@ -1133,7 +1115,6 @@ Error MetadataLoader::MetadataLoaderImpl::parseMetadata(bool ModuleLevel) {
       // placeholders, that we flush here.
       resolveForwardRefsAndPlaceholders(Placeholders);
       upgradeDebugInfo(ModuleLevel);
-      cloneLocalTypes();
       // Return at the beginning of the block, since it is easy to skip it
       // entirely from there.
       Stream.ReadBlockEnd(); // Pop the abbrev block context.
@@ -1165,7 +1146,6 @@ Error MetadataLoader::MetadataLoaderImpl::parseMetadata(bool ModuleLevel) {
     case BitstreamEntry::EndBlock:
       resolveForwardRefsAndPlaceholders(Placeholders);
       upgradeDebugInfo(ModuleLevel);
-      cloneLocalTypes();
       return Error::success();
     case BitstreamEntry::Record:
       // The interesting case.
@@ -2048,6 +2028,9 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
     MetadataList.assignValue(SP, NextMetadataNo);
     NextMetadataNo++;
 
+    if (IsDistinct)
+      NewDistinctSPs.push_back(SP);
+
     // Upgrade sp->function mapping to function->sp mapping.
     if (HasFn) {
       if (auto *CMD = dyn_cast_or_null<ConstantAsMetadata>(CUorFn))
diff --git a/llvm/lib/IR/DIBuilder.cpp b/llvm/lib/IR/DIBuilder.cpp
@@ -377,10 +377,13 @@ DIDerivedType *DIBuilder::createTypedef(DIType *Ty, StringRef Name,
                                         DIScope *Context, uint32_t AlignInBits,
                                         DINode::DIFlags Flags,
                                         DINodeArray Annotations) {
-  return DIDerivedType::get(VMContext, dwarf::DW_TAG_typedef, Name, File,
-                            LineNo, getNonCompileUnitScope(Context), Ty,
-                            (uint64_t)0, AlignInBits, (uint64_t)0, std::nullopt,
-                            std::nullopt, Flags, nullptr, Annotations);
+  auto *T = DIDerivedType::get(
+      VMContext, dwarf::DW_TAG_typedef, Name, File, LineNo,
+      getNonCompileUnitScope(Context), Ty, (uint64_t)0, AlignInBits,
+      (uint64_t)0, std::nullopt, std::nullopt, Flags, nullptr, Annotations);
+  if (isa_and_nonnull<DILocalScope>(Context))
+    getSubprogramNodesTrackingVector(Context).emplace_back(T);
+  return T;
 }
 
 DIDerivedType *
@@ -604,6 +607,8 @@ DICompositeType *DIBuilder::createStructType(
       nullptr, UniqueIdentifier, nullptr, nullptr, nullptr, nullptr, nullptr,
       nullptr, Specification, NumExtraInhabitants);
   trackIfUnresolved(R);
+  if (isa_and_nonnull<DILocalScope>(Context))
+    getSubprogramNodesTrackingVector(Context).emplace_back(R);
   return R;
 }
 
@@ -688,6 +693,8 @@ DIDerivedType *DIBuilder::createSetType(DIScope *Scope, StringRef Name,
                                SizeInBits, AlignInBits, 0, std::nullopt,
                                std::nullopt, DINode::FlagZero);
   trackIfUnresolved(R);
+  if (isa_and_nonnull<DILocalScope>(Scope))
+    getSubprogramNodesTrackingVector(Scope).emplace_back(R);
   return R;
 }
 
@@ -723,6 +730,8 @@ DICompositeType *DIBuilder::createArrayType(
                               : (Metadata *)cast<DIVariable *>(RK),
       nullptr, nullptr, 0, BitStride);
   trackIfUnresolved(R);
+  if (isa_and_nonnull<DILocalScope>(Scope))
+    getSubprogramNodesTrackingVector(Scope).emplace_back(R);
   return R;
 }
 
@@ -877,9 +886,12 @@ DISubrangeType *DIBuilder::createSubrangeType(
     uint64_t SizeInBits, uint32_t AlignInBits, DINode::DIFlags Flags,
     DIType *Ty, Metadata *LowerBound, Metadata *UpperBound, Metadata *Stride,
     Metadata *Bias) {
-  return DISubrangeType::get(VMContext, Name, File, LineNo, Scope, SizeInBits,
-                             AlignInBits, Flags, Ty, LowerBound, UpperBound,
-                             Stride, Bias);
+  auto *T = DISubrangeType::get(VMContext, Name, File, LineNo, Scope,
+                                SizeInBits, AlignInBits, Flags, Ty, LowerBound,
+                                UpperBound, Stride, Bias);
+  if (isa_and_nonnull<DILocalScope>(Scope))
+    getSubprogramNodesTrackingVector(Scope).emplace_back(T);
+  return T;
 }
 
 static void checkGlobalVariableScope(DIScope *Context) {
diff --git a/llvm/lib/IR/DebugInfoMetadata.cpp b/llvm/lib/IR/DebugInfoMetadata.cpp
@@ -1425,6 +1425,45 @@ bool DISubprogram::describes(const Function *F) const {
   assert(F && "Invalid function");
   return F->getSubprogram() == this;
 }
+
+void DISubprogram::cleanupRetainedNodes() {
+  // Checks if a metadata node from retainedTypes is a type not belonging to
+  // this subprogram.
+  auto IsAlienType = [this](DINode *N) {
+    auto *T = dyn_cast_or_null<DIType>(N);
+    if (!T)
+      return false;
+
+    DISubprogram *TypeSP = nullptr;
+    // The type might have been global in the previously loaded IR modules.
+    if (auto *LS = dyn_cast_or_null<DILocalScope>(T->getScope()))
+      TypeSP = LS->getSubprogram();
+
+    return this != TypeSP;
+  };
+
+  // As this is expected to be called during module loading, before
+  // stripping old or incorrect debug info, perform minimal sanity check.
+  if (!isa_and_present<MDTuple>(getRawRetainedNodes()))
+    return;
+
+  MDTuple *RetainedNodes = cast<MDTuple>(getRawRetainedNodes());
+  SmallVector<Metadata *> MDs;
+  MDs.reserve(RetainedNodes->getNumOperands());
+  for (const MDOperand &Node : RetainedNodes->operands()) {
+    // Ignore malformed retainedNodes.
+    if (Node && !isa<DINode>(Node))
+      return;
+
+    auto *N = cast_or_null<DINode>(Node);
+    if (!IsAlienType(N))
+      MDs.push_back(N);
+  }
+
+  if (MDs.size() != RetainedNodes->getNumOperands())
+    replaceRetainedNodes(MDNode::get(getContext(), MDs));
+}
+
 DILexicalBlockBase::DILexicalBlockBase(LLVMContext &C, unsigned ID,
                                        StorageType Storage,
                                        ArrayRef<Metadata *> Ops)
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
@@ -1568,6 +1568,16 @@ void Verifier::visitDISubprogram(const DISubprogram &N) {
               "invalid retained nodes, expected DILocalVariable, DILabel, "
               "DIImportedEntity or DIType",
               &N, Node, Op);
+      if (auto *T = dyn_cast<DIType>(Op)) {
+        auto *TypeScope = dyn_cast_or_null<DILocalScope>(T->getScope());
+        DISubprogram *TypeSP = TypeScope ? TypeScope->getSubprogram() : nullptr;
+        DICompileUnit *TypeSPUnit = TypeSP ? TypeSP->getUnit() : nullptr;
+        if (isa_and_nonnull<DILocalScope>(T->getScope()))
+          CheckDI(!TypeScope || TypeSP == &N,
+                  "invalid retained node, DIType should have the scope of "
+                  "DISubprogram",
+                  &N, N.getUnit(), Node, Op, TypeScope, TypeSP, TypeSPUnit);
+      }
     }
   }
   CheckDI(!hasConflictingReferenceFlags(N.getFlags()),
diff --git a/llvm/test/Bitcode/local-type-scope.ll b/llvm/test/Bitcode/local-type-scope.ll
@@ -1,16 +1,18 @@
-; RUN: llvm-as < %s -o %t0
-; RUN: llvm-dis %t0 -o - | FileCheck %s
+; RUN: llvm-as --disable-verify < %s -o %t0
+; RUN: opt --passes=verify %t0 -o /dev/null
+; RUN: llvm-dis %t0 -o - | FileCheck %s --implicit-check-not=DICompositeType
 
-; Ensure that function-local types with the same ODR identifier belonging
-; to different subprograms are not deduplicated when a module is being loaded.
+; During module loading, if a local type appears in retainedNodes
+; field of multiple DISubprograms due to ODR-uniquing,
+; retainedNodes should be cleaned up, so that only one DISubprogram
+; will have this type in its retainedNodes.
 
 ; CHECK: [[CU:![0-9]+]] = distinct !DICompileUnit
 ; CHECK: [[BAR:![0-9]+]] = distinct !DISubprogram(name: "bar", {{.*}}, retainedNodes: [[RN_BAR:![0-9]+]])
-; CHECK: [[RN_BAR]] = !{[[T2:![0-9]+]]}
-; CHECK: [[T2]] = !DICompositeType(tag: DW_TAG_class_type, {{.*}}, identifier: "local_type")
+; CHECK: [[RN_BAR]] = !{}
 ; CHECK: [[FOO:![0-9]+]] = distinct !DISubprogram(name: "foo", {{.*}}, retainedNodes: [[RN_FOO:![0-9]+]])
 ; CHECK: [[RN_FOO]] = !{[[T1:![0-9]+]]}
-; CHECK: [[T1]] = !DICompositeType(tag: DW_TAG_class_type, {{.*}}, identifier: "local_type")
+; CHECK: [[T1]] = !DICompositeType(tag: DW_TAG_class_type, scope: [[FOO]], {{.*}}, identifier: "local_type")
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
diff --git a/llvm/test/DebugInfo/Inputs/cleanup-retained-nodes.ll b/llvm/test/DebugInfo/Inputs/cleanup-retained-nodes.ll
@@ -0,0 +1,17 @@
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@var = global i8 0, align 4, !dbg !7
+
+!llvm.module.flags = !{!0, !1, !2}
+!llvm.dbg.cu = !{!3}
+
+!0 = !{i32 7, !"Dwarf Version", i32 2}
+!1 = !{i32 2, !"Debug Info Version", i32 3}
+!2 = !{i32 1, !"wchar_size", i32 4}
+!3 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !5, producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!4 = !{}
+!5 = !DIFile(filename: "tmp2.cpp", directory: "/tmp/")
+!6 = !DICompositeType(tag: DW_TAG_class_type, scope: !3, file: !5, line: 212, size: 8, flags: DIFlagTypePassByValue, elements: !4, identifier: "type_global_in_another_module")
+!7 = !DIGlobalVariableExpression(var: !8, expr: !DIExpression())
+!8 = distinct !DIGlobalVariable(name: "var", scope: !3, file: !5, line: 1, type: !6, isLocal: false, isDefinition: true)
diff --git a/llvm/test/DebugInfo/X86/cleanup-retained-nodes.ll b/llvm/test/DebugInfo/X86/cleanup-retained-nodes.ll
diff --git a/llvm/test/DebugInfo/X86/llparser-cleanup-retained-nodes.ll b/llvm/test/DebugInfo/X86/llparser-cleanup-retained-nodes.ll
