Skip to content

[DebugInfo][RemoveDIs] Use autoupgrader to convert old debug-info #143452

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.

Sign up for GitHub

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 7 commits into from
Jun 11, 2025
Merged

[DebugInfo][RemoveDIs] Use autoupgrader to convert old debug-info #143452

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8525072
[DebugInfo][RemoveDIs] Use autoupgrader to convert old debug-info
jmorse Jun 3, 2025
3021087
Formatting, fix a comment, remove cruft from test
jmorse Jun 10, 2025
8506550
Pass autoupgraded DbgRecords through pre-verifier constructor
jmorse Jun 10, 2025
d41ece6
Test for some more localisation of debug-info errors
jmorse Jun 10, 2025
add20db
Hark the herald clang-format sings...
jmorse Jun 10, 2025
4c75936
Strip cruft from another test
jmorse Jun 11, 2025
bf1e168
Spelling, and avoid an un-necessary reinterpret cast
jmorse Jun 11, 2025
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
2 changes: 0 additions & 2 deletions llvm/lib/AsmParser/LLParser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8336,8 +8336,6 @@ bool LLParser::parseCall(Instruction *&Inst, PerFunctionState &PFS,
return error(CallLoc, "llvm.dbg intrinsic should not appear in a module "
"using non-intrinsic debug info");
}
if (!SeenOldDbgInfoFormat)
M->setNewDbgInfoFormatFlag(false);
SeenOldDbgInfoFormat = true;
}
CI->setAttributes(PAL);
Expand Down
39 changes: 27 additions & 12 deletions llvm/lib/IR/AsmWriter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1204,25 +1204,32 @@ void SlotTracker::processFunctionMetadata(const Function &F) {
}

void SlotTracker::processDbgRecordMetadata(const DbgRecord &DR) {
// Tolerate null metadata pointers: it's a completely illegal debug record,
// but we can have faulty metadata from debug-intrinsic days being
// autoupgraded into debug records. This gets caught by the verifier, which
// then will print the faulty IR, hitting this code path.
if (const DbgVariableRecord *DVR = dyn_cast<const DbgVariableRecord>(&DR)) {
// Process metadata used by DbgRecords; we only specifically care about the
// DILocalVariable, DILocation, and DIAssignID fields, as the Value and
// Expression fields should only be printed inline and so do not use a slot.
// Note: The above doesn't apply for empty-metadata operands.
if (auto *Empty = dyn_cast<MDNode>(DVR->getRawLocation()))
if (auto *Empty = dyn_cast_if_present<MDNode>(DVR->getRawLocation()))
CreateMetadataSlot(Empty);
CreateMetadataSlot(DVR->getRawVariable());
if (DVR->getRawVariable())
CreateMetadataSlot(DVR->getRawVariable());
if (DVR->isDbgAssign()) {
CreateMetadataSlot(cast<MDNode>(DVR->getRawAssignID()));
if (auto *Empty = dyn_cast<MDNode>(DVR->getRawAddress()))
if (auto *AssignID = DVR->getRawAssignID())
CreateMetadataSlot(cast<MDNode>(AssignID));
if (auto *Empty = dyn_cast_if_present<MDNode>(DVR->getRawAddress()))
CreateMetadataSlot(Empty);
}
} else if (const DbgLabelRecord *DLR = dyn_cast<const DbgLabelRecord>(&DR)) {
CreateMetadataSlot(DLR->getRawLabel());
} else {
llvm_unreachable("unsupported DbgRecord kind");
}
CreateMetadataSlot(DR.getDebugLoc().getAsMDNode());
if (DR.getDebugLoc())
CreateMetadataSlot(DR.getDebugLoc().getAsMDNode());
}

void SlotTracker::processInstructionMetadata(const Instruction &I) {
Expand Down Expand Up @@ -4867,22 +4874,30 @@ void AssemblyWriter::printDbgVariableRecord(const DbgVariableRecord &DVR) {
llvm_unreachable(
"Tried to print a DbgVariableRecord with an invalid LocationType!");
}

auto PrintOrNull = [&](Metadata *M) {
if (!M)
Out << "(null)";
else
WriteAsOperandInternal(Out, M, WriterCtx, true);
};

Out << "(";
WriteAsOperandInternal(Out, DVR.getRawLocation(), WriterCtx, true);
PrintOrNull(DVR.getRawLocation());
Out << ", ";
WriteAsOperandInternal(Out, DVR.getRawVariable(), WriterCtx, true);
PrintOrNull(DVR.getRawVariable());
Out << ", ";
WriteAsOperandInternal(Out, DVR.getRawExpression(), WriterCtx, true);
PrintOrNull(DVR.getRawExpression());
Out << ", ";
if (DVR.isDbgAssign()) {
WriteAsOperandInternal(Out, DVR.getRawAssignID(), WriterCtx, true);
PrintOrNull(DVR.getRawAssignID());
Out << ", ";
WriteAsOperandInternal(Out, DVR.getRawAddress(), WriterCtx, true);
PrintOrNull(DVR.getRawAddress());
Out << ", ";
WriteAsOperandInternal(Out, DVR.getRawAddressExpression(), WriterCtx, true);
PrintOrNull(DVR.getRawAddressExpression());
Out << ", ";
}
WriteAsOperandInternal(Out, DVR.getDebugLoc().getAsMDNode(), WriterCtx, true);
PrintOrNull(DVR.getDebugLoc().getAsMDNode());
Out << ")";
}

Expand Down
78 changes: 53 additions & 25 deletions llvm/lib/IR/AutoUpgrade.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1155,8 +1155,7 @@ static bool upgradeIntrinsicFunction1(Function *F, Function *&NewFn,
case 'd':
if (Name.consume_front("dbg.")) {
// Mark debug intrinsics for upgrade to new debug format.
if (CanUpgradeDebugIntrinsicsToRecords &&
F->getParent()->IsNewDbgInfoFormat) {
if (CanUpgradeDebugIntrinsicsToRecords) {
if (Name == "addr" || Name == "value" || Name == "assign" ||
Name == "declare" || Name == "label") {
// There's no function to replace these with.
Expand Down Expand Up @@ -4395,39 +4394,67 @@ static Value *upgradeAMDGCNIntrinsicCall(StringRef Name, CallBase *CI,
return Builder.CreateBitCast(RMW, RetTy);
}

/// Helper to unwrap intrinsic call MetadataAsValue operands.
template <typename MDType>
static MDType *unwrapMAVOp(CallBase *CI, unsigned Op) {
if (MetadataAsValue *MAV = dyn_cast<MetadataAsValue>(CI->getArgOperand(Op)))
return dyn_cast<MDType>(MAV->getMetadata());
/// Helper to unwrap intrinsic call MetadataAsValue operands. Return as a
/// plain MDNode, as it's the verifiers job to check these are the correct
/// types later.
static MDNode *unwrapMAVOp(CallBase *CI, unsigned Op) {
if (Op < CI->arg_size()) {
if (MetadataAsValue *MAV =
dyn_cast<MetadataAsValue>(CI->getArgOperand(Op))) {
Metadata *MD = MAV->getMetadata();
if (isa<MDNode>(MD))
return reinterpret_cast<MDNode *>(MD);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (MetadataAsValue *MAV =
dyn_cast<MetadataAsValue>(CI->getArgOperand(Op))) {
Metadata *MD = MAV->getMetadata();
if (isa<MDNode>(MD))
return reinterpret_cast<MDNode *>(MD);
}
if (MetadataAsValue *MAV =
dyn_cast<MetadataAsValue>(CI->getArgOperand(Op))) {
return dyn_cast<MDNode>(MAV->getMetadata());

Do we still need the reinterpret_cast now given you're doing an isa check?

Copy link
Member Author

Choose a reason for hiding this comment

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

Switched to using dyn_cast overall.

}
return nullptr;
}

/// Helper to unwrap Metadata MetadataAsValue operands, such as the Value field.
static Metadata *unwrapMAVMetadataOp(CallBase *CI, unsigned Op) {
if (Op < CI->arg_size())
if (MetadataAsValue *MAV = dyn_cast<MetadataAsValue>(CI->getArgOperand(Op)))
return MAV->getMetadata();
return nullptr;
}

static MDNode *getDebugLocSafe(const Instruction *I) {
// The MDNode attached to this instruction might not be the correct type,
// as the verifier has not yet be run. Fetch it as a bare MDNode.
return I->getDebugLoc().getAsMDNode();
}

/// Convert debug intrinsic calls to non-instruction debug records.
/// \p Name - Final part of the intrinsic name, e.g. 'value' in llvm.dbg.value.
/// \p CI - The debug intrinsic call.
static void upgradeDbgIntrinsicToDbgRecord(StringRef Name, CallBase *CI) {
DbgRecord *DR = nullptr;
if (Name == "label") {
DR = new DbgLabelRecord(unwrapMAVOp<DILabel>(CI, 0), CI->getDebugLoc());
DR = DbgLabelRecord::createUnresolvedDbgLabelRecord(unwrapMAVOp(CI, 0),
CI->getDebugLoc());
} else if (Name == "assign") {
DR = new DbgVariableRecord(
unwrapMAVOp<Metadata>(CI, 0), unwrapMAVOp<DILocalVariable>(CI, 1),
unwrapMAVOp<DIExpression>(CI, 2), unwrapMAVOp<DIAssignID>(CI, 3),
unwrapMAVOp<Metadata>(CI, 4), unwrapMAVOp<DIExpression>(CI, 5),
CI->getDebugLoc());
DR = DbgVariableRecord::createUnresolvedDbgVariableRecord(
DbgVariableRecord::LocationType::Assign, unwrapMAVMetadataOp(CI, 0),
unwrapMAVOp(CI, 1), unwrapMAVOp(CI, 2), unwrapMAVOp(CI, 3),
unwrapMAVMetadataOp(CI, 4),
/*The address is a Value ref, it will be stored as a Metadata */
unwrapMAVOp(CI, 5), getDebugLocSafe(CI));
} else if (Name == "declare") {
DR = new DbgVariableRecord(
unwrapMAVOp<Metadata>(CI, 0), unwrapMAVOp<DILocalVariable>(CI, 1),
unwrapMAVOp<DIExpression>(CI, 2), CI->getDebugLoc(),
DbgVariableRecord::LocationType::Declare);
DR = DbgVariableRecord::createUnresolvedDbgVariableRecord(
DbgVariableRecord::LocationType::Declare, unwrapMAVMetadataOp(CI, 0),
unwrapMAVOp(CI, 1), unwrapMAVOp(CI, 2), nullptr, nullptr, nullptr,
getDebugLocSafe(CI));
} else if (Name == "addr") {
// Upgrade dbg.addr to dbg.value with DW_OP_deref.
DIExpression *Expr = unwrapMAVOp<DIExpression>(CI, 2);
Expr = DIExpression::append(Expr, dwarf::DW_OP_deref);
DR = new DbgVariableRecord(unwrapMAVOp<Metadata>(CI, 0),
unwrapMAVOp<DILocalVariable>(CI, 1), Expr,
CI->getDebugLoc());
MDNode *ExprNode = unwrapMAVOp(CI, 2);
// Don't try to add something to the expression if it's not an expression.
// Instead, allow the verifier to fail later.
if (DIExpression *Expr = dyn_cast<DIExpression>(ExprNode)) {
ExprNode = DIExpression::append(Expr, dwarf::DW_OP_deref);
}
DR = DbgVariableRecord::createUnresolvedDbgVariableRecord(
DbgVariableRecord::LocationType::Value, unwrapMAVMetadataOp(CI, 0),
unwrapMAVOp(CI, 1), ExprNode, nullptr, nullptr, nullptr,
getDebugLocSafe(CI));
} else if (Name == "value") {
// An old version of dbg.value had an extra offset argument.
unsigned VarOp = 1;
Expand All @@ -4440,9 +4467,10 @@ static void upgradeDbgIntrinsicToDbgRecord(StringRef Name, CallBase *CI) {
VarOp = 2;
ExprOp = 3;
}
DR = new DbgVariableRecord(
unwrapMAVOp<Metadata>(CI, 0), unwrapMAVOp<DILocalVariable>(CI, VarOp),
unwrapMAVOp<DIExpression>(CI, ExprOp), CI->getDebugLoc());
DR = DbgVariableRecord::createUnresolvedDbgVariableRecord(
DbgVariableRecord::LocationType::Value, unwrapMAVMetadataOp(CI, 0),
unwrapMAVOp(CI, VarOp), unwrapMAVOp(CI, ExprOp), nullptr, nullptr,
nullptr, getDebugLocSafe(CI));
}
assert(DR && "Unhandled intrinsic kind in upgrade to DbgRecord");
CI->getParent()->insertDbgRecordBefore(DR, CI->getIterator());
Expand Down
4 changes: 0 additions & 4 deletions llvm/lib/IR/BasicBlock.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ using namespace llvm;
STATISTIC(NumInstrRenumberings, "Number of renumberings across all blocks");

DbgMarker *BasicBlock::createMarker(Instruction *I) {
assert(IsNewDbgInfoFormat &&
"Tried to create a marker in a non new debug-info block!");
if (I->DebugMarker)
return I->DebugMarker;
DbgMarker *Marker = new DbgMarker();
Expand All @@ -43,8 +41,6 @@ DbgMarker *BasicBlock::createMarker(Instruction *I) {
}

DbgMarker *BasicBlock::createMarker(InstListType::iterator It) {
assert(IsNewDbgInfoFormat &&
"Tried to create a marker in a non new debug-info block!");
Copy link
Contributor

Choose a reason for hiding this comment

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

Are the IsNewDbgInfoFormat deletions (here and in the unittest) from a different patch in the series?

Copy link
Member Author

Choose a reason for hiding this comment

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

I did actually hit this while developing this patch; LLParser was setting the flag to false, then Autoupgrade, upon upgrading, would create a marker and hit this assertion. Deleting the assertion (as the flag should be true everywhere anyway) was part of that process.

(Technically this could be taken out of this patch now as there's nothing setting the flag to false, anywhere, but it's got to be deleted at some point).

if (It != end())
return createMarker(&*It);
DbgMarker *DM = getTrailingDbgRecords();
Expand Down
29 changes: 16 additions & 13 deletions llvm/lib/IR/Verifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6710,38 +6710,41 @@ void Verifier::visit(DbgVariableRecord &DVR) {
CheckDI(DVR.getType() == DbgVariableRecord::LocationType::Value ||
DVR.getType() == DbgVariableRecord::LocationType::Declare ||
DVR.getType() == DbgVariableRecord::LocationType::Assign,
"invalid #dbg record type", &DVR, DVR.getType());
"invalid #dbg record type", &DVR, DVR.getType(), BB, F);

// The location for a DbgVariableRecord must be either a ValueAsMetadata,
// DIArgList, or an empty MDNode (which is a legacy representation for an
// "undef" location).
auto *MD = DVR.getRawLocation();
CheckDI(MD && (isa<ValueAsMetadata>(MD) || isa<DIArgList>(MD) ||
(isa<MDNode>(MD) && !cast<MDNode>(MD)->getNumOperands())),
"invalid #dbg record address/value", &DVR, MD);
"invalid #dbg record address/value", &DVR, MD, BB, F);
if (auto *VAM = dyn_cast<ValueAsMetadata>(MD)) {
visitValueAsMetadata(*VAM, F);
if (DVR.isDbgDeclare()) {
// Allow integers here to support inttoptr salvage.
Type *Ty = VAM->getValue()->getType();
CheckDI(Ty->isPointerTy() || Ty->isIntegerTy(),
"location of #dbg_declare must be a pointer or int", &DVR, MD);
"location of #dbg_declare must be a pointer or int", &DVR, MD, BB,
F);
}
} else if (auto *AL = dyn_cast<DIArgList>(MD)) {
visitDIArgList(*AL, F);
}

CheckDI(isa_and_nonnull<DILocalVariable>(DVR.getRawVariable()),
"invalid #dbg record variable", &DVR, DVR.getRawVariable());
"invalid #dbg record variable", &DVR, DVR.getRawVariable(), BB, F);
visitMDNode(*DVR.getRawVariable(), AreDebugLocsAllowed::No);

CheckDI(isa_and_nonnull<DIExpression>(DVR.getRawExpression()),
"invalid #dbg record expression", &DVR, DVR.getRawExpression());
"invalid #dbg record expression", &DVR, DVR.getRawExpression(), BB,
F);
visitMDNode(*DVR.getExpression(), AreDebugLocsAllowed::No);

if (DVR.isDbgAssign()) {
CheckDI(isa_and_nonnull<DIAssignID>(DVR.getRawAssignID()),
"invalid #dbg_assign DIAssignID", &DVR, DVR.getRawAssignID());
"invalid #dbg_assign DIAssignID", &DVR, DVR.getRawAssignID(), BB,
F);
visitMDNode(*cast<DIAssignID>(DVR.getRawAssignID()),
AreDebugLocsAllowed::No);

Expand All @@ -6752,29 +6755,29 @@ void Verifier::visit(DbgVariableRecord &DVR) {
CheckDI(
isa<ValueAsMetadata>(RawAddr) ||
(isa<MDNode>(RawAddr) && !cast<MDNode>(RawAddr)->getNumOperands()),
"invalid #dbg_assign address", &DVR, DVR.getRawAddress());
"invalid #dbg_assign address", &DVR, DVR.getRawAddress(), BB, F);
if (auto *VAM = dyn_cast<ValueAsMetadata>(RawAddr))
visitValueAsMetadata(*VAM, F);

CheckDI(isa_and_nonnull<DIExpression>(DVR.getRawAddressExpression()),
"invalid #dbg_assign address expression", &DVR,
DVR.getRawAddressExpression());
DVR.getRawAddressExpression(), BB, F);
visitMDNode(*DVR.getAddressExpression(), AreDebugLocsAllowed::No);

// All of the linked instructions should be in the same function as DVR.
for (Instruction *I : at::getAssignmentInsts(&DVR))
CheckDI(DVR.getFunction() == I->getFunction(),
"inst not in same function as #dbg_assign", I, &DVR);
"inst not in same function as #dbg_assign", I, &DVR, BB, F);
}

// This check is redundant with one in visitLocalVariable().
DILocalVariable *Var = DVR.getVariable();
CheckDI(isType(Var->getRawType()), "invalid type ref", Var,
Var->getRawType());
CheckDI(isType(Var->getRawType()), "invalid type ref", Var, Var->getRawType(),
BB, F);

auto *DLNode = DVR.getDebugLoc().getAsMDNode();
CheckDI(isa_and_nonnull<DILocation>(DLNode), "invalid #dbg record DILocation",
&DVR, DLNode);
&DVR, DLNode, BB, F);
DILocation *Loc = DVR.getDebugLoc();

// The scopes for variables and !dbg attachments must agree.
Expand All @@ -6786,7 +6789,7 @@ void Verifier::visit(DbgVariableRecord &DVR) {
CheckDI(VarSP == LocSP,
"mismatched subprogram between #dbg record variable and DILocation",
&DVR, BB, F, Var, Var->getScope()->getSubprogram(), Loc,
Loc->getScope()->getSubprogram());
Loc->getScope()->getSubprogram(), BB, F);

verifyFnArgs(DVR);
}
Expand Down
6 changes: 5 additions & 1 deletion llvm/test/Assembler/drop-debug-info-nonzero-alloca.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,12 @@ entry:
metadata ptr undef,
metadata !DILocalVariable(scope: !1),
metadata !DIExpression())
; AS: llvm.dbg.value intrinsic requires a !dbg attachment
; AS: invalid #dbg record DILocation
; AS: #dbg_value(ptr undef, !{{[0-9]+}}, !DIExpression(), (null))
; AS: label %entry
; AS: ptr @foo
; AS: warning: ignoring invalid debug info in <stdin>

ret void
}

Expand Down
18 changes: 9 additions & 9 deletions llvm/test/DebugInfo/Generic/assignment-tracking/parse-and-verify/verify.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

define dso_local void @fun2() !dbg !15 {
;; DIAssignID copied here from @fun() where it is used by intrinsics.
; CHECK: dbg.assign not in same function as inst
; CHECK: DVRAssign not in same function as inst
%x = alloca i32, align 4, !DIAssignID !14
ret void
}
Expand All @@ -17,24 +17,24 @@ define dso_local void @fun() !dbg !7 {
entry:
%a = alloca i32, align 4, !DIAssignID !14
;; Here something other than a dbg.assign intrinsic is using a DIAssignID.
; CHECK: !DIAssignID should only be used by llvm.dbg.assign intrinsics
; CHECK: !DIAssignID should only be used by Assign DVRs
call void @llvm.dbg.value(metadata !14, metadata !10, metadata !DIExpression()), !dbg !13

;; Each following dbg.assign has an argument of the incorrect type.
; CHECK: invalid llvm.dbg.assign intrinsic address/value
; CHECK: invalid #dbg record address/value
call void @llvm.dbg.assign(metadata !3, metadata !10, metadata !DIExpression(), metadata !14, metadata ptr undef, metadata !DIExpression()), !dbg !13
; CHECK: invalid llvm.dbg.assign intrinsic variable
; CHECK: invalid #dbg record variable
call void @llvm.dbg.assign(metadata i32 0, metadata !2, metadata !DIExpression(), metadata !14, metadata ptr undef, metadata !DIExpression()), !dbg !13
; CHECK: invalid llvm.dbg.assign intrinsic expression
; CHECK: invalid #dbg record expression
call void @llvm.dbg.assign(metadata !14, metadata !10, metadata !2, metadata !14, metadata ptr undef, metadata !DIExpression()), !dbg !13
; CHECK: invalid llvm.dbg.assign intrinsic DIAssignID
; CHECK: invalid #dbg_assign DIAssignID
call void @llvm.dbg.assign(metadata !14, metadata !10, metadata !DIExpression(), metadata !2, metadata ptr undef, metadata !DIExpression()), !dbg !13
; CHECK: invalid llvm.dbg.assign intrinsic address
; CHECK: invalid #dbg_assign address
call void @llvm.dbg.assign(metadata !14, metadata !10, metadata !DIExpression(), metadata !14, metadata !3, metadata !DIExpression()), !dbg !13
;; Empty metadata debug operands are allowed.
; CHECK-NOT: invalid llvm.dbg.assign
; CHECK-NOT: invalid #dbg record
call void @llvm.dbg.assign(metadata !14, metadata !10, metadata !DIExpression(), metadata !14, metadata !2, metadata !DIExpression()), !dbg !13
; CHECK: invalid llvm.dbg.assign intrinsic address expression
; CHECK: invalid #dbg_assign address expression
call void @llvm.dbg.assign(metadata !14, metadata !10, metadata !DIExpression(), metadata !14, metadata ptr undef, metadata !2), !dbg !13
ret void
}
Expand Down
Loading
Loading