[mlir][llvm] Drop cyclic dependencies during debug metadata import.

[gysit]

Summary

This revision fixes the import of LLVM IR to handle debug metadata with
cyclic dependencies. It deletes the elements list of the composite type
if a cyclic dependency is detected. The revision is meant as a band aid
to avoid infinite recursion during the import of cyclic debug metadata.
Long term solutions are currently discussed here:
https://discourse.llvm.org/t/handling-cyclic-dependencies-in-debug-info/67526/4

[definelicht]

I think the title of the commit should reflect the band aid nature of it 🙂

[gysit]

Update commit message.

[Dinistro]

LGTM!

mlir/lib/Target/LLVMIR/DebugImporter.h

private:
   DISubprogramAttr translateImpl(llvm::DISubprogram *node);
   DISubrangeAttr translateImpl(llvm::DISubrange *node);
   DISubroutineTypeAttr translateImpl(llvm::DISubroutineType *node);
   DITypeAttr translateImpl(llvm::DIType *node);
 
   /// A mapping between LLVM debug metadata and the corresponding attribute.
   DenseMap<llvm::DINode *, DINodeAttr> nodeToAttr;
 
+  /// A stack that stores the metadata nodes that are being traversed. The stack
+  /// is used to detect cyclic dependencies during the metadata translation.
+  SetVector<llvm::DINode *> translationStack;
+
   MLIRContext *context;
   ModuleOp mlirModule;
 };
 
 } // namespace detail
 } // namespace LLVM
 } // namespace mlir
 
 #endif // MLIR_LIB_TARGET_LLVMIR_DEBUIMPORTN_H_

mlir/lib/Target/LLVMIR/DebugImporter.cpp

 //===- DebugImporter.cpp - LLVM to MLIR Debug conversion ------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "DebugImporter.h"
 #include "mlir/Dialect/LLVMIR/LLVMAttrs.h"
 #include "mlir/IR/Attributes.h"
 #include "mlir/IR/BuiltinAttributes.h"
 #include "mlir/IR/Location.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
 
 using namespace mlir;
 using namespace mlir::LLVM;

 
 DICompositeTypeAttr DebugImporter::translateImpl(llvm::DICompositeType *node) {
   std::optional<DIFlags> flags = symbolizeDIFlags(node->getFlags());
   SmallVector<DINodeAttr> elements;
   for (llvm::DINode *element : node->getElements()) {
     assert(element && "expected a non-null element type");
     elements.push_back(translate(element));
   }
+  // Drop the elements parameter if a cyclic dependency is detected. We
+  // currently cannot model these cycles and thus drop the parameter if
+  // required. A cyclic dependency is detected if one of the element nodes
+  // translates to a nullptr since the node is already on the translation stack.
+  // TODO: Support debug metadata with cyclic dependencies.
+  if (llvm::is_contained(elements, nullptr))
+    elements.clear();
   return DICompositeTypeAttr::get(
       context, node->getTag(), StringAttr::get(context, node->getName()),
       translate(node->getFile()), node->getLine(), translate(node->getScope()),
       translate(node->getBaseType()), flags.value_or(DIFlags::Zero),
       node->getSizeInBits(), node->getAlignInBits(), elements);
 }
 
 DIDerivedTypeAttr DebugImporter::translateImpl(llvm::DIDerivedType *node) {
+  // Return nullptr if the base type is a cyclic dependency.
+  DITypeAttr baseType = translate(node->getBaseType());
+  if (node->getBaseType() && !baseType)
+    return nullptr;
   return DIDerivedTypeAttr::get(
       context, node->getTag(),
       node->getRawName() ? StringAttr::get(context, node->getName()) : nullptr,
-      translate(node->getBaseType()), node->getSizeInBits(),
-      node->getAlignInBits(), node->getOffsetInBits());
+      baseType, node->getSizeInBits(), node->getAlignInBits(),
+      node->getOffsetInBits());
 }
 
 DIFileAttr DebugImporter::translateImpl(llvm::DIFile *node) {
   return DIFileAttr::get(context, node->getFilename(), node->getDirectory());
 }
 
 DILexicalBlockAttr DebugImporter::translateImpl(llvm::DILexicalBlock *node) {
   return DILexicalBlockAttr::get(context, translate(node->getScope()),

 
 DIScopeAttr DebugImporter::translateImpl(llvm::DIScope *node) {
   return cast<DIScopeAttr>(translate(static_cast<llvm::DINode *>(node)));
 }
 
 DISubprogramAttr DebugImporter::translateImpl(llvm::DISubprogram *node) {
   std::optional<DISubprogramFlags> subprogramFlags =
       symbolizeDISubprogramFlags(node->getSubprogram()->getSPFlags());
+  // Return nullptr if the scope or type is a cyclic dependency.
+  DIScopeAttr scope = translate(node->getScope());
+  if (node->getScope() && !scope)
+    return nullptr;
+  DISubroutineTypeAttr type = translate(node->getType());
+  if (node->getType() && !type)
+    return nullptr;
   return DISubprogramAttr::get(
-      context, translate(node->getUnit()), translate(node->getScope()),
+      context, translate(node->getUnit()), scope,
       StringAttr::get(context, node->getName()),
       node->getRawLinkageName()
           ? StringAttr::get(context, node->getLinkageName())
           : nullptr,
       translate(node->getFile()), node->getLine(), node->getScopeLine(),
-      subprogramFlags.value(), translate(node->getType()));
+      subprogramFlags.value(), type);
 }
 
 DISubrangeAttr DebugImporter::translateImpl(llvm::DISubrange *node) {
   auto getIntegerAttrOrNull = [&](llvm::DISubrange::BoundType data) {
     if (auto *constInt = llvm::dyn_cast_or_null<llvm::ConstantInt *>(data))
       return IntegerAttr::get(IntegerType::get(context, 64),
                               constInt->getSExtValue());
     return IntegerAttr();
if (!type) {
       // void result type. Translate the nullptr to an explicit
       // DIVoidResultTypeAttr since the attribute list cannot contain a nullptr
       // entry.
       types.push_back(DIVoidResultTypeAttr::get(context));
       continue;
     }
     types.push_back(translate(type));
   }
+  // Return nullptr if any of the types is a cyclic dependency.
+  if (llvm::is_contained(types, nullptr))
+    return nullptr;
   return DISubroutineTypeAttr::get(context, node->getCC(), types);
 }
 
 DITypeAttr DebugImporter::translateImpl(llvm::DIType *node) {
   return cast<DITypeAttr>(translate(static_cast<llvm::DINode *>(node)));
 }
 
 DINodeAttr DebugImporter::translate(llvm::DINode *node) {
   if (!node)
     return nullptr;
 
   // Check for a cached instance.
   if (DINodeAttr attr = nodeToAttr.lookup(node))
     return attr;
 
+  // Return nullptr if a cyclic dependency is detected since the same node is
+  // being traversed twice. This check avoids infinite recursion if the debug
+  // metadata contains cycles.
+  if (!translationStack.insert(node))
+    return nullptr;
+  auto guard = llvm::make_scope_exit([&]() { translationStack.pop_back(); });
+
   // Convert the debug metadata if possible.
   auto translateNode = [this](llvm::DINode *node) -> DINodeAttr {
     if (auto *casted = dyn_cast<llvm::DIBasicType>(node))
       return translateImpl(casted);
     if (auto *casted = dyn_cast<llvm::DICompileUnit>(node))
       return translateImpl(casted);
     if (auto *casted = dyn_cast<llvm::DICompositeType>(node))
       return translateImpl(casted);

mlir/test/Target/LLVMIR/Import/debug-info.ll

 !6 = !DILocalVariable(scope: !3, name: "arg")
 !7 = !DILocation(line: 1, column: 2, scope: !3)
 !8 = !DILocation(line: 2, column: 2, scope: !3)
 !9 = !DILocation(line: 3, column: 2, scope: !3)
 
 ; // -----
 
 ; CHECK-LABEL: @class_method
-define void @class_method(ptr %arg1) {
+define void @class_method() {
   ; CHECK: llvm.return loc(#[[LOC:.+]])
-  ret void, !dbg !5
+  ret void, !dbg !9
 }
 
+; Verify the elements parameter is dropped due to the cyclic dependencies.
 ; CHECK: #[[COMP:.+]] = #llvm.di_composite_type<tag = DW_TAG_class_type, name = "class_name", file = #{{.*}}, line = 42, flags = "TypePassByReference|NonTrivial">
-; CHECK: #[[SP:.+]] = #llvm.di_subprogram<compileUnit = #{{.*}}, scope = #[[COMP]], name = "class_method", file = #{{.*}}, subprogramFlags = Definition>
+; CHECK: #[[COMP_PTR:.+]] = #llvm.di_derived_type<tag = DW_TAG_pointer_type, baseType = #[[COMP]], sizeInBits = 64>
+; CHECK: #[[SP_TYPE:.+]] = #llvm.di_subroutine_type<types = #{{.*}}, #[[COMP_PTR]]>
+; CHECK: #[[SP:.+]] = #llvm.di_subprogram<compileUnit = #{{.*}}, scope = #[[COMP]], name = "class_method", file = #{{.*}}, subprogramFlags = Definition, type = #[[SP_TYPE]]>
 ; CHECK: #[[LOC]] = loc(fused<#[[SP]]>
 
 !llvm.dbg.cu = !{!1}
 !llvm.module.flags = !{!0}
 !0 = !{i32 2, !"Debug Info Version", i32 3}
 !1 = distinct !DICompileUnit(language: DW_LANG_C, file: !2)
 !2 = !DIFile(filename: "debug-info.ll", directory: "/")
-!3 = !DICompositeType(tag: DW_TAG_class_type, name: "class_name", file: !2, line: 42, flags: DIFlagTypePassByReference | DIFlagNonTrivial)
-!4 = distinct !DISubprogram(name: "class_method", scope: !3, file: !2, spFlags: DISPFlagDefinition, unit: !1)
-!5 = !DILocation(line: 1, column: 2, scope: !4)
+!3 = !DICompositeType(tag: DW_TAG_class_type, name: "class_name", file: !2, line: 42, flags: DIFlagTypePassByReference | DIFlagNonTrivial, elements: !4)
+!4 = !{!5}
+!5 = distinct !DISubprogram(name: "class_method", scope: !3, file: !2, type: !6, spFlags: DISPFlagDefinition, unit: !1)
+!6 = !DISubroutineType(types: !7)
+!7 = !{null, !8}
+!8 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !3, size: 64, flags: DIFlagArtificial | DIFlagObjectPointer)
+!9 = !DILocation(line: 1, column: 2, scope: !5)
+
+; // -----
+
+; Verify the elements parameter is dropped due to the cyclic dependencies.
+; CHECK: #[[$COMP:.+]] = #llvm.di_composite_type<tag = DW_TAG_class_type, name = "class_field", file = #{{.*}}, line = 42, flags = "TypePassByReference|NonTrivial">
+; CHECK: #[[$COMP_PTR:.+]] = #llvm.di_derived_type<tag = DW_TAG_pointer_type, baseType = #[[$COMP]]>
+; CHECK: #[[$VAR0:.+]] = #llvm.di_local_variable<scope = #{{.*}}, name = "class_field", file = #{{.*}}, type = #[[$COMP_PTR]]>
+
+; CHECK-LABEL: @class_field
+; CHECK-SAME:  %[[ARG0:[a-zA-Z0-9]+]]
+define void @class_field(ptr %arg1) {
+  ; CHECK: llvm.intr.dbg.value #[[$VAR0]] = %[[ARG0]] : !llvm.ptr
+  call void @llvm.dbg.value(metadata ptr %arg1, metadata !7, metadata !DIExpression()), !dbg !9
+  ret void
+}
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!1}
+!llvm.module.flags = !{!0}
+!0 = !{i32 2, !"Debug Info Version", i32 3}
+!1 = distinct !DICompileUnit(language: DW_LANG_C, file: !2)
+!2 = !DIFile(filename: "debug-info.ll", directory: "/")
+!3 = !DICompositeType(tag: DW_TAG_class_type, name: "class_field", file: !2, line: 42, flags: DIFlagTypePassByReference | DIFlagNonTrivial, elements: !4)
+!4 = !{!6}
+!5 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !3, flags: DIFlagArtificial | DIFlagObjectPointer)
+!6 = !DIDerivedType(tag: DW_TAG_member, name: "call_field", file: !2, baseType: !5)
+!7 = !DILocalVariable(scope: !8, name: "class_field", file: !2, type: !5);
+!8 = distinct !DISubprogram(name: "class_field", scope: !2, file: !2, spFlags: DISPFlagDefinition, unit: !1)
+!9 = !DILocation(line: 1, column: 2, scope: !8)