Skip to content

[DebugInfo][RemoveDIs] Find types hidden in DbgRecords #106547

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 10 commits into from
Sep 2, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions llvm/lib/IR/DebugProgramInstruction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
12 changes: 12 additions & 0 deletions llvm/lib/IR/TypeFinder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,18 @@ 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 (Value *Addr = DVI->getAddress(); Addr && DVI->isDbgAssign())
incorporateValue(Addr);
Copy link
Contributor

Choose a reason for hiding this comment

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

Regarding the latest change to check allow for DIArgList in getAddress: it's not permitted for dbg_declares or dbg_assigns address components to use DIArgList. getAddress returns the first DebugValue operand unless it's a dbg_assign - then it returns the second. The only reason we're seeing an DIArgList pop up in getAddress is because its being called before checking if DVI->isDbgAssign(): some dbg_value with a DIArgList is having getAddress called on it. I thought this would be fine when I wrote the suggestion above but clearly it breaks assumptions.

I think it's probably better to restructure this to:

if (DVI->isDbgAssign())) {
  if (Value *Addr = DVI->getAddress())
    incorporateValue(Addr);
}

And revert the most recent getAddress change- wdyt? Sorry for the additional hassle.

}
}
}
}

Expand Down
51 changes: 51 additions & 0 deletions llvm/test/DebugInfo/type-finder-w-dbg-records.ll
Original file line number Diff line number Diff line change
@@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

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

possibly worth adding dbg_assign and dbg_declare for completeness? (dbg_assign more important here imo, given the extra fields)

#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()
Loading