[RemoveDIs] Fix SIGSEGV caused by splitBasicBlock

[FLZ101]

See llvm/unittests/IR/BasicBlockDbgInfoTest.cpp for a test case.

The SIGSEGV is like below:

 #0 0x0000000000d5142b llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/media/d/work/learn-riscv/llvm-project/build-r/unittests/IR/./IRTests+0xd5142b)
 #1 0x0000000000d4f10b SignalHandler(int) Signals.cpp:0:0
 #2 0x00007f2eb3a5c9a0 __restore_rt (/lib64/libc.so.6+0x3e9a0)
 #3 0x0000000000b863b8 llvm::DbgMarker::absorbDebugValues(llvm::DbgMarker&, bool) (/media/d/work/learn-riscv/llvm-project/build-r/unittests/IR/./IRTests+0xb863b8)
 #4 0x0000000000b031cc llvm::BasicBlock::spliceDebugInfoImpl(llvm::ilist_iterator_w_bits<llvm::ilist_detail::node_options<llvm::Instruction, false, false, void, true>, false, false>, llvm::BasicBlock*, llvm::ilist_iterator_w_bits<llvm::ilist_detail::node_options<llvm::Instruction, false, false, void, true>, false, false>, llvm::ilist_iterator_w_bits<llvm::ilist_detail::node_options<llvm::Instruction, false, false, void, true>, false, false>) (/media/d/work/learn-riscv/llvm-project/build-r/unittests/IR/./IRTests+0xb031cc)
 #5 0x0000000000b03e37 llvm::BasicBlock::splice(llvm::ilist_iterator_w_bits<llvm::ilist_detail::node_options<llvm::Instruction, false, false, void, true>, false, false>, llvm::BasicBlock*, llvm::ilist_iterator_w_bits<llvm::ilist_detail::node_options<llvm::Instruction, false, false, void, true>, false, false>, llvm::ilist_iterator_w_bits<llvm::ilist_detail::node_options<llvm::Instruction, false, false, void, true>, false, false>) (/media/d/work/learn-riscv/llvm-project/build-r/unittests/IR/./IRTests+0xb03e37)
 #6 0x0000000000b04495 llvm::BasicBlock::splitBasicBlockBefore(llvm::ilist_iterator_w_bits<llvm::ilist_detail::node_options<llvm::Instruction, false, false, void, true>, false, false>, llvm::Twine const&) (/media/d/work/learn-riscv/llvm-project/build-r/unittests/IR/./IRTests+0xb04495)
 #7 0x000000000050e257 (anonymous namespace)::BasicBlockDbgInfoTest_SplitBasicBlockBefore_Test::TestBody() BasicBlockDbgInfoTest.cpp:0:0
 #8 0x0000000001231c23 testing::Test::Run() (.part.0) gtest-all.cc:0:0
 #9 0x0000000001236a2a testing::TestInfo::Run() (/media/d/work/learn-riscv/llvm-project/build-r/unittests/IR/./IRTests+0x1236a2a)
#10 0x000000000123705c testing::TestSuite::Run() (.part.0) gtest-all.cc:0:0
#11 0x000000000123dff0 testing::internal::UnitTestImpl::RunAllTests() (/media/d/work/learn-riscv/llvm-project/build-r/unittests/IR/./IRTests+0x123dff0)
#12 0x000000000123136e testing::UnitTest::Run() (/media/d/work/learn-riscv/llvm-project/build-r/unittests/IR/./IRTests+0x123136e)
#13 0x000000000048bd76 main (/media/d/work/learn-riscv/llvm-project/build-r/unittests/IR/./IRTests+0x48bd76)
#14 0x00007f2eb3a4614a __libc_start_call_main (/lib64/libc.so.6+0x2814a)
#15 0x00007f2eb3a4620b __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x2820b)
#16 0x00000000004e9745 _start (/media/d/work/learn-riscv/llvm-project/build-r/unittests/IR/./IRTests+0x4e9745)

[llvmbot]

@llvm/pr-subscribers-llvm-ir

Author: Franklin Zhang (FLZ101)

Changes

---

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

2 Files Affected:

• (modified) llvm/lib/IR/BasicBlock.cpp (+1-1)
• (modified) llvm/unittests/IR/BasicBlockDbgInfoTest.cpp (+56)

diff --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp
index 6e62767c99e2a2..49d0e245c58676 100644
--- a/llvm/lib/IR/BasicBlock.cpp
+++ b/llvm/lib/IR/BasicBlock.cpp
@@ -1009,7 +1009,7 @@ void BasicBlock::spliceDebugInfoImpl(BasicBlock::iterator Dest, BasicBlock *Src,
     // generate the iterator with begin() / getFirstInsertionPt(), it means
     // any trailing debug-info at the end of the block would "normally" have
     // been pushed in front of "First". Move it there now.
-    DbgMarker *FirstMarker = getMarker(First);
+    DbgMarker *FirstMarker = createMarker(First);
     DbgMarker *TrailingDbgRecords = getTrailingDbgRecords();
     if (TrailingDbgRecords) {
       FirstMarker->absorbDebugValues(*TrailingDbgRecords, true);
diff --git a/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp b/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp
index 905928819dda80..f873bbd4293af5 100644
--- a/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp
+++ b/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp
@@ -109,6 +109,62 @@ TEST(BasicBlockDbgInfoTest, InsertAfterSelf) {
   UseNewDbgInfoFormat = false;
 }
 
+TEST(BasicBlockDbgInfoTest, SplitBasicBlockBefore) {
+  LLVMContext C;
+  UseNewDbgInfoFormat = true;
+
+  std::unique_ptr<Module> M = parseIR(C, R"---(
+    define dso_local void @func() #0 !dbg !10 {
+      %1 = alloca i32, align 4
+      tail call void @llvm.dbg.declare(metadata ptr %1, metadata !14, metadata !DIExpression()), !dbg !16
+      store i32 2, ptr %1, align 4, !dbg !16
+      ret void, !dbg !17
+    }
+
+    declare void @llvm.dbg.declare(metadata, metadata, metadata) #0
+
+    attributes #0 = { nocallback nofree nosync nounwind speculatable willreturn memory(none) }
+
+    !llvm.dbg.cu = !{!0}
+    !llvm.module.flags = !{!2, !3, !4, !5, !6, !7, !8}
+    !llvm.ident = !{!9}
+
+    !0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, producer: "dummy", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: None)
+    !1 = !DIFile(filename: "dummy", directory: "dummy")
+    !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 = !{i32 7, !"PIE Level", i32 2}
+    !7 = !{i32 7, !"uwtable", i32 2}
+    !8 = !{i32 7, !"frame-pointer", i32 2}
+    !9 = !{!"dummy"}
+    !10 = distinct !DISubprogram(name: "func", scope: !1, file: !1, line: 1, type: !11, scopeLine: 1, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !13)
+    !11 = !DISubroutineType(types: !12)
+    !12 = !{null}
+    !13 = !{}
+    !14 = !DILocalVariable(name: "a", scope: !10, file: !1, line: 2, type: !15)
+    !15 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+    !16 = !DILocation(line: 2, column: 6, scope: !10)
+    !17 = !DILocation(line: 3, column: 2, scope: !10)
+  )---");
+  ASSERT_TRUE(M);
+
+  M->convertToNewDbgValues();
+
+  Function *F = M->getFunction("func");
+
+  BasicBlock &BB = F->getEntryBlock();
+  auto I = std::prev(BB.end(), 2);
+  BB.splitBasicBlockBefore(I, "before");
+
+  BasicBlock &BBBefore = F->getEntryBlock();
+  auto I2 = std::prev(BBBefore.end(), 2);
+  ASSERT_TRUE(I2->hasDbgRecords());
+
+  UseNewDbgInfoFormat = false;
+}
+
 TEST(BasicBlockDbgInfoTest, MarkerOperations) {
   LLVMContext C;
   UseNewDbgInfoFormat = true;

[FLZ101]

@jmorse @SLTozer @OCHyams @dsandersllvm

[jmorse]

Looks correct at a surface level; I'm out-of-office for a while though, Stephen/Orlando should be able to confirm this is a good fix.

[OCHyams]

I was going to ask if the unittest could be changed to use the new textual format, but this (intrinsics) matches the other tests so that's ok for now, that can be updated with the others when we get to that.

Code change makes sense. LGTM once the inline comment is addressed. Thank you!

[OCHyams]

Can you drop this into the if (TrailingDbgRecords) body? We don't want to create a marker if we're not going to use it when TrailingDbgRecords is false.

[FLZ101]

done.

[OCHyams]

(Still LGTM, thanks)