Skip to content

Commit

Permalink
[RemoveDIs] Fix spliceDebugInfo splice-to-end edge case (#105671)
Browse files Browse the repository at this point in the history
Fix #105571 which demonstrates an end() iterator dereference when
performing a non-empty splice to end() from a region that ends at
Src::end().

Rather than calling Instruction::adoptDbgRecords from Dest, create a marker
(which takes an iterator) and absorbDebugValues onto that. The "absorb" variant
doesn't clean up the source marker, which in this case we know is a trailing
marker, so we have to do that manually.

(cherry picked from commit 43661a1)
  • Loading branch information
OCHyams authored and tru committed Sep 10, 2024
1 parent 64015ee commit 11e2a15
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 2 deletions.
12 changes: 10 additions & 2 deletions llvm/lib/IR/BasicBlock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -975,8 +975,16 @@ void BasicBlock::spliceDebugInfoImpl(BasicBlock::iterator Dest, BasicBlock *Src,
if (ReadFromTail && Src->getMarker(Last)) {
DbgMarker *FromLast = Src->getMarker(Last);
if (LastIsEnd) {
Dest->adoptDbgRecords(Src, Last, true);
// adoptDbgRecords will release any trailers.
if (Dest == end()) {
// Abosrb the trailing markers from Src.
assert(FromLast == Src->getTrailingDbgRecords());
createMarker(Dest)->absorbDebugValues(*FromLast, true);
FromLast->eraseFromParent();
Src->deleteTrailingDbgRecords();
} else {
// adoptDbgRecords will release any trailers.
Dest->adoptDbgRecords(Src, Last, true);
}
assert(!Src->getTrailingDbgRecords());
} else {
// FIXME: can we use adoptDbgRecords here to reduce allocations?
Expand Down
52 changes: 52 additions & 0 deletions llvm/unittests/IR/BasicBlockDbgInfoTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1525,4 +1525,56 @@ TEST(BasicBlockDbgInfoTest, DbgMoveToEnd) {
EXPECT_FALSE(Ret->hasDbgRecords());
}

TEST(BasicBlockDbgInfoTest, CloneTrailingRecordsToEmptyBlock) {
LLVMContext C;
std::unique_ptr<Module> M = parseIR(C, R"(
define i16 @foo(i16 %a) !dbg !6 {
entry:
%b = add i16 %a, 0
#dbg_value(i16 %b, !9, !DIExpression(), !11)
ret i16 0, !dbg !11
}
!llvm.dbg.cu = !{!0}
!llvm.module.flags = !{!5}
!0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
!1 = !DIFile(filename: "t.ll", directory: "/")
!2 = !{}
!5 = !{i32 2, !"Debug Info Version", i32 3}
!6 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: null, file: !1, line: 1, type: !7, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !8)
!7 = !DISubroutineType(types: !2)
!8 = !{!9}
!9 = !DILocalVariable(name: "1", scope: !6, file: !1, line: 1, type: !10)
!10 = !DIBasicType(name: "ty16", size: 16, encoding: DW_ATE_unsigned)
!11 = !DILocation(line: 1, column: 1, scope: !6)
)");
ASSERT_TRUE(M);

Function *F = M->getFunction("foo");
BasicBlock &BB = F->getEntryBlock();
// Start with no trailing records.
ASSERT_FALSE(BB.getTrailingDbgRecords());

BasicBlock::iterator Ret = std::prev(BB.end());
BasicBlock::iterator B = std::prev(Ret);

// Delete terminator which has debug records: we now get trailing records.
Ret->eraseFromParent();
EXPECT_TRUE(BB.getTrailingDbgRecords());

BasicBlock *NewBB = BasicBlock::Create(C, "NewBB", F);
NewBB->splice(NewBB->end(), &BB, B, BB.end());

// The trailing records should've been absorbed into NewBB.
EXPECT_FALSE(BB.getTrailingDbgRecords());
EXPECT_TRUE(NewBB->getTrailingDbgRecords());
if (DbgMarker *Trailing = NewBB->getTrailingDbgRecords()) {
EXPECT_EQ(llvm::range_size(Trailing->getDbgRecordRange()), 1u);
// Drop the trailing records now, to prevent a cleanup assertion.
Trailing->eraseFromParent();
NewBB->deleteTrailingDbgRecords();
}
}

} // End anonymous namespace.

0 comments on commit 11e2a15

Please sign in to comment.