Skip to content

[DebugInfo] Update DebugInfoFinder to take retainedNodes into account #140285

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 2 commits into from
May 20, 2025

Conversation

dzhidzhoev
Copy link
Member

Since https://reviews.llvm.org/D144004, DISubprogram's retainedNodes field is used to track DIImportedEntities, in addition to local variables and labels.

However, the corresponding update for DebugInfoFinder, to make it visit DISubprogram's retainedNodes, was missing.
This is the fix for it.

This change is separated from #119001 to simplify it.

Since https://reviews.llvm.org/D144004, DISubprogram's retainedNodes
field is used to track DIImportedEntities, in addition to local
variables and labels.

However, the corresponding update for DebugInfoFinder, to make it visit
DISubprogram's retainedNodes, was missing.

This is the fix for it.

This change is separated from
llvm#119001 to simplify it.
@llvmbot
Copy link
Member

llvmbot commented May 16, 2025

@llvm/pr-subscribers-debuginfo

@llvm/pr-subscribers-llvm-ir

Author: Vladislav Dzhidzhoev (dzhidzhoev)

Changes

Since https://reviews.llvm.org/D144004, DISubprogram's retainedNodes field is used to track DIImportedEntities, in addition to local variables and labels.

However, the corresponding update for DebugInfoFinder, to make it visit DISubprogram's retainedNodes, was missing.
This is the fix for it.

This change is separated from #119001 to simplify it.


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

3 Files Affected:

  • (modified) llvm/include/llvm/IR/DebugInfo.h (+2-1)
  • (modified) llvm/lib/IR/DebugInfo.cpp (+22-13)
  • (added) llvm/test/DebugInfo/Generic/debuginfofinder-retained-nodes.ll (+38)
diff --git a/llvm/include/llvm/IR/DebugInfo.h b/llvm/include/llvm/IR/DebugInfo.h
index 73f45c3769be4..c47738a6442e6 100644
--- a/llvm/include/llvm/IR/DebugInfo.h
+++ b/llvm/include/llvm/IR/DebugInfo.h
@@ -110,7 +110,7 @@ class DebugInfoFinder {
   void processInstruction(const Module &M, const Instruction &I);
 
   /// Process a DILocalVariable.
-  void processVariable(const Module &M, const DILocalVariable *DVI);
+  void processVariable(DILocalVariable *DVI);
   /// Process debug info location.
   void processLocation(const Module &M, const DILocation *Loc);
   /// Process a DbgRecord (e.g, treat a DbgVariableRecord like a
@@ -127,6 +127,7 @@ class DebugInfoFinder {
   void processCompileUnit(DICompileUnit *CU);
   void processScope(DIScope *Scope);
   void processType(DIType *DT);
+  void processImportedEntity(DIImportedEntity *Import);
   bool addCompileUnit(DICompileUnit *CU);
   bool addGlobalVariable(DIGlobalVariableExpression *DIG);
   bool addScope(DIScope *Scope);
diff --git a/llvm/lib/IR/DebugInfo.cpp b/llvm/lib/IR/DebugInfo.cpp
index dff9a08c2c8e0..9abde9023d057 100644
--- a/llvm/lib/IR/DebugInfo.cpp
+++ b/llvm/lib/IR/DebugInfo.cpp
@@ -242,22 +242,14 @@ void DebugInfoFinder::processCompileUnit(DICompileUnit *CU) {
     else
       processSubprogram(cast<DISubprogram>(RT));
   for (auto *Import : CU->getImportedEntities()) {
-    auto *Entity = Import->getEntity();
-    if (auto *T = dyn_cast<DIType>(Entity))
-      processType(T);
-    else if (auto *SP = dyn_cast<DISubprogram>(Entity))
-      processSubprogram(SP);
-    else if (auto *NS = dyn_cast<DINamespace>(Entity))
-      processScope(NS->getScope());
-    else if (auto *M = dyn_cast<DIModule>(Entity))
-      processScope(M->getScope());
+    processImportedEntity(Import);
   }
 }
 
 void DebugInfoFinder::processInstruction(const Module &M,
                                          const Instruction &I) {
   if (auto *DVI = dyn_cast<DbgVariableIntrinsic>(&I))
-    processVariable(M, DVI->getVariable());
+    processVariable(DVI->getVariable());
 
   if (auto DbgLoc = I.getDebugLoc())
     processLocation(M, DbgLoc.get());
@@ -275,7 +267,7 @@ void DebugInfoFinder::processLocation(const Module &M, const DILocation *Loc) {
 
 void DebugInfoFinder::processDbgRecord(const Module &M, const DbgRecord &DR) {
   if (const DbgVariableRecord *DVR = dyn_cast<const DbgVariableRecord>(&DR))
-    processVariable(M, DVR->getVariable());
+    processVariable(DVR->getVariable());
   processLocation(M, DR.getDebugLoc().get());
 }
 
@@ -303,6 +295,18 @@ void DebugInfoFinder::processType(DIType *DT) {
   }
 }
 
+void DebugInfoFinder::processImportedEntity(DIImportedEntity *Import) {
+  auto *Entity = Import->getEntity();
+  if (auto *T = dyn_cast<DIType>(Entity))
+    processType(T);
+  else if (auto *SP = dyn_cast<DISubprogram>(Entity))
+    processSubprogram(SP);
+  else if (auto *NS = dyn_cast<DINamespace>(Entity))
+    processScope(NS->getScope());
+  else if (auto *M = dyn_cast<DIModule>(Entity))
+    processScope(M->getScope());
+}
+
 void DebugInfoFinder::processScope(DIScope *Scope) {
   if (!Scope)
     return;
@@ -350,10 +354,15 @@ void DebugInfoFinder::processSubprogram(DISubprogram *SP) {
       processType(TVal->getType());
     }
   }
+
+  for (auto *N : SP->getRetainedNodes())
+    if (auto *Var = dyn_cast_or_null<DILocalVariable>(N))
+      processVariable(Var);
+    else if (auto *Import = dyn_cast_or_null<DIImportedEntity>(N))
+      processImportedEntity(Import);
 }
 
-void DebugInfoFinder::processVariable(const Module &M,
-                                      const DILocalVariable *DV) {
+void DebugInfoFinder::processVariable(DILocalVariable *DV) {
   if (!NodesSeen.insert(DV).second)
     return;
   processScope(DV->getScope());
diff --git a/llvm/test/DebugInfo/Generic/debuginfofinder-retained-nodes.ll b/llvm/test/DebugInfo/Generic/debuginfofinder-retained-nodes.ll
new file mode 100644
index 0000000000000..f8185c65639d6
--- /dev/null
+++ b/llvm/test/DebugInfo/Generic/debuginfofinder-retained-nodes.ll
@@ -0,0 +1,38 @@
+; RUN: opt -passes='print<module-debuginfo>' -disable-output 2>&1 < %s \
+; RUN:   | FileCheck %s
+
+; This is to track DebugInfoFinder's ability to find the debug info metadata,
+; in particular, properly visit DISubprogram's retainedNodes.
+
+; CHECK: Compile unit: DW_LANG_C_plus_plus from /somewhere/source.cpp
+; CHECK: Subprogram: foo from /somewhere/source.cpp:1 ('_Z3foov')
+; CHECK: Subprogram: bar from /somewhere/source.cpp:5
+; CHECK: Type: T from /somewhere/source.cpp:2 DW_TAG_structure_type
+
+%struct.T = type { i32 }
+
+; Function Attrs: mustprogress noinline nounwind optnone ssp uwtable(sync)
+define noundef i32 @_Z3foov() !dbg !7 {
+entry:
+  ret i32 0
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 21.0.0git", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug)
+!1 = !DIFile(filename: "source.cpp", directory: "/somewhere")
+!2 = !{i32 7, !"Dwarf Version", i32 5}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 4}
+!5 = !{i32 8, !"PIC Level", i32 2}
+!6 = !{!"clang version 21.0.0git"}
+!7 = distinct !DISubprogram(name: "foo", linkageName: "_Z3foov", scope: !1, file: !1, line: 1, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !8)
+!8 = !{!9}
+!9 = !DILocalVariable(name: "v", scope: !7, file: !1, line: 8, type: !10)
+!10 = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "T", scope: !7, file: !1, line: 2, size: 32, flags: DIFlagTypePassByValue, elements: !11)
+!11 = !{!12, !14}
+!12 = !DIDerivedType(tag: DW_TAG_member, name: "x", scope: !10, file: !1, line: 3, baseType: !13, size: 32)
+!13 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!14 = !DISubprogram(name: "bar", scope: !10, file: !1, line: 5, scopeLine: 5, flags: DIFlagPrototyped | DIFlagStaticMember, spFlags: DISPFlagLocalToUnit)
\ No newline at end of file

Copy link
Contributor

@SLTozer SLTozer left a comment

Choose a reason for hiding this comment

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

Change LGTM, but might be a good idea to add a DIImportedEntity to the test as well.

Comment on lines 358 to 362
for (auto *N : SP->getRetainedNodes())
if (auto *Var = dyn_cast_or_null<DILocalVariable>(N))
processVariable(Var);
else if (auto *Import = dyn_cast_or_null<DIImportedEntity>(N))
processImportedEntity(Import);
Copy link
Contributor

Choose a reason for hiding this comment

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

Requesting braces for the for block here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

@dzhidzhoev
Copy link
Member Author

Change LGTM, but might be a good idea to add a DIImportedEntity to the test as well.

The test case was added. Should I merge it? (Asking since the "approve" button wasn't pressed 🙂 ).

@dzhidzhoev dzhidzhoev merged commit f057a58 into llvm:main May 20, 2025
11 checks passed
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
…llvm#140285)

Since https://reviews.llvm.org/D144004, DISubprogram's retainedNodes
field is used to track DIImportedEntities, in addition to local
variables and labels.

However, the corresponding update for DebugInfoFinder, to make it visit
DISubprogram's retainedNodes, was missing.
This is the fix for it.

This change is separated from
llvm#119001 to simplify it.
ajaden-codes pushed a commit to Jaddyen/llvm-project that referenced this pull request Jun 6, 2025
…llvm#140285)

Since https://reviews.llvm.org/D144004, DISubprogram's retainedNodes
field is used to track DIImportedEntities, in addition to local
variables and labels.

However, the corresponding update for DebugInfoFinder, to make it visit
DISubprogram's retainedNodes, was missing.
This is the fix for it.

This change is separated from
llvm#119001 to simplify it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants