Skip to content

release/19.x: [DebugInfo][RemoveDIs] Find types hidden in DbgRecords (#106547) #107060

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 3, 2024

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Sep 3, 2024

Backport 25f87f2

Requested by: @jmorse

@llvmbot llvmbot added this to the LLVM 19.X Release milestone Sep 3, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Sep 3, 2024

@OCHyams What do you think about merging this PR to the release branch?

@llvmbot
Copy link
Member Author

llvmbot commented Sep 3, 2024

@llvm/pr-subscribers-debuginfo

@llvm/pr-subscribers-llvm-ir

Author: None (llvmbot)

Changes

Backport 25f87f2

Requested by: @jmorse


Full diff: https://github.com/llvm/llvm-project/pull/107060.diff

3 Files Affected:

  • (modified) llvm/lib/IR/DebugProgramInstruction.cpp (+3-2)
  • (modified) llvm/lib/IR/TypeFinder.cpp (+14)
  • (added) llvm/test/DebugInfo/type-finder-w-dbg-records.ll (+51)
diff --git a/llvm/lib/IR/DebugProgramInstruction.cpp b/llvm/lib/IR/DebugProgramInstruction.cpp
index 362d467beeb11b..5d2189b54204fb 100644
--- a/llvm/lib/IR/DebugProgramInstruction.cpp
+++ b/llvm/lib/IR/DebugProgramInstruction.cpp
@@ -473,11 +473,12 @@ DbgLabelRecord::createDebugIntrinsic(Module *M,
 
 Value *DbgVariableRecord::getAddress() const {
   auto *MD = getRawAddress();
-  if (auto *V = dyn_cast<ValueAsMetadata>(MD))
+  if (auto *V = dyn_cast_or_null<ValueAsMetadata>(MD))
     return V->getValue();
 
   // When the value goes to null, it gets replaced by an empty MDNode.
-  assert(!cast<MDNode>(MD)->getNumOperands() && "Expected an empty MDNode");
+  assert(!MD ||
+         !cast<MDNode>(MD)->getNumOperands() && "Expected an empty MDNode");
   return nullptr;
 }
 
diff --git a/llvm/lib/IR/TypeFinder.cpp b/llvm/lib/IR/TypeFinder.cpp
index 003155a4af4877..963f4b4806e1f9 100644
--- a/llvm/lib/IR/TypeFinder.cpp
+++ b/llvm/lib/IR/TypeFinder.cpp
@@ -88,6 +88,20 @@ void TypeFinder::run(const Module &M, bool onlyNamed) {
         for (const auto &MD : MDForInst)
           incorporateMDNode(MD.second);
         MDForInst.clear();
+
+        // Incorporate types hiding in variable-location information.
+        for (const auto &Dbg : I.getDbgRecordRange()) {
+          // Pick out records that have Values.
+          if (const DbgVariableRecord *DVI =
+                  dyn_cast<DbgVariableRecord>(&Dbg)) {
+            for (Value *V : DVI->location_ops())
+              incorporateValue(V);
+            if (DVI->isDbgAssign()) {
+              if (Value *Addr = DVI->getAddress())
+                incorporateValue(Addr);
+            }
+          }
+        }
       }
   }
 
diff --git a/llvm/test/DebugInfo/type-finder-w-dbg-records.ll b/llvm/test/DebugInfo/type-finder-w-dbg-records.ll
new file mode 100644
index 00000000000000..8259b4a9f1c3a5
--- /dev/null
+++ b/llvm/test/DebugInfo/type-finder-w-dbg-records.ll
@@ -0,0 +1,51 @@
+; RUN: opt --passes=verify %s -o - -S | FileCheck %s
+
+;; Test that the type definitions are discovered when serialising to LLVM-IR,
+;; even if they're only present inside a DbgRecord, and thus not normally
+;; visible.
+
+; CHECK: %union.anon = type { %struct.a }
+; CHECK: %struct.a = type { i32 }
+; CHECK: %union.anon2 = type { %struct.a2 }
+; CHECK: %struct.a2 = type { i32 }
+
+; ModuleID = 'bbi-98372.ll'
+source_filename = "bbi-98372.ll"
+
+%union.anon = type { %struct.a }
+%struct.a = type { i32 }
+%union.anon2 = type { %struct.a2 }
+%struct.a2 = type { i32 }
+
+@d = global [1 x { i16, i16 }] [{ i16, i16 } { i16 0, i16 undef }], align 1
+@e = global [1 x { i16, i16 }] [{ i16, i16 } { i16 0, i16 undef }], align 1
+
+define void @f() {
+entry:
+    #dbg_value(ptr getelementptr inbounds ([1 x %union.anon], ptr @d, i32 0, i32 3), !7, !DIExpression(), !14)
+    #dbg_assign(ptr null, !7, !DIExpression(), !16, ptr getelementptr inbounds ([1 x %union.anon2], ptr @e, i32 0, i32 3), !17, !14)
+  ret void, !dbg !15
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, producer: "clang", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "foo.c", directory: "/bar")
+!2 = !{i32 7, !"Dwarf Version", i32 4}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 1}
+!5 = !{i32 7, !"frame-pointer", i32 2}
+!6 = !{!"clang"}
+!7 = !DILocalVariable(name: "f", scope: !8, file: !1, line: 8, type: !12)
+!8 = distinct !DISubprogram(name: "e", scope: !1, file: !1, line: 8, type: !9, scopeLine: 8, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !11)
+!9 = !DISubroutineType(types: !10)
+!10 = !{null}
+!11 = !{!7}
+!12 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !13, size: 16)
+!13 = !DIBasicType(name: "int", size: 16, encoding: DW_ATE_signed)
+!14 = !DILocation(line: 0, scope: !8)
+!15 = !DILocation(line: 8, column: 28, scope: !8)
+!16 = distinct !DIAssignID()
+!17 = !DIExpression()

Copy link
Contributor

@OCHyams OCHyams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OCHyams What do you think about merging this PR to the release branch?

LGTM, this fixes a bug where invalid IR gets produced, so getting it backported sounds good.

For peace of mind: we don't need a null check in the DVI->location_ops() loop because location_ops() returns an empty range rather than a single-nullptr-location, and multiple locations (through DIArgList) should never be nullptr.

When serialising to textual IR, there can be constant Values referred to
by DbgRecords that don't appear anywhere else, and have types hidden
even deeper in side them. Enumerate these when enumerating all types.

Test by Mikael Holmén.

(cherry picked from commit 25f87f2)
@tru tru merged commit f3da9af into llvm:release/19.x Sep 3, 2024
8 of 10 checks passed
Copy link

github-actions bot commented Sep 3, 2024

@jmorse (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

4 participants