[llvm] [OMPIRBuilder] Avoid crash in BasicBlock::splice. (PR #154987)

[Abid Qadeer via llvm-commits]

https://github.com/abidh updated https://github.com/llvm/llvm-project/pull/154987

>From 38c66254d75cde8d1c039eacc922cb44b2b4149e Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Fri, 22 Aug 2025 15:45:02 +0100
Subject: [PATCH 1/2] [OMPIRBuilder] Avoid crash in BasicBlock::splice.

Calling splice when both Old and New are empty is a nop currently but
it can cause a crash once debug records are used instead of debug
intrinsics. This PR makes the call conditional on at least one of
Old or New being non-empty.

Consider the following mlir:
omp.target map_entries() {
  llvm.intr.dbg.declare ...
  llvm.intr.dbg.declare ...
  omp.teams ...
  ...
}

Current code would translate llvm.intr to llvm intrinsics and we will
have 2 of them in the Old BB by the time omp.teams implementation starts.
This implementation creates many BasicBlocks by calling splitBB.

In the new scheme (using debug records), there will be no instruction
in the Old BB after llvm.intr get translated but just 2 trailing debug
records. So effectively both Old and New are empty. When control reaches
BasicBlock::splice, it calls spliceDebugInfoEmptyBlock. This funtion
expects that in this case (Src is empty but has trailing debug records),
the ToIt is valid and it can call adoptDbgRecords on it. This
assumption is not true in this case as New is empty and ToIt is pointing
to end().
---
 llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 27a4fcfb303c4..2ff6d3a8ddd21 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -308,7 +308,8 @@ void llvm::spliceBB(IRBuilderBase::InsertPoint IP, BasicBlock *New,
 
   // Move instructions to new block.
   BasicBlock *Old = IP.getBlock();
-  New->splice(New->begin(), Old, IP.getPoint(), Old->end());
+  if (!Old->empty() || !New->empty())
+    New->splice(New->begin(), Old, IP.getPoint(), Old->end());
 
   if (CreateBranch) {
     auto *NewBr = BranchInst::Create(New, Old);

>From f2d9c507cca64697ce3015f0b6c9e67aba1749e4 Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Tue, 26 Aug 2025 15:46:05 +0100
Subject: [PATCH 2/2] Add a unit test.

---
 .../Frontend/OpenMPIRBuilderTest.cpp          | 24 +++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
index b7a060bb3563d..c13570dc803b3 100644
--- a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -7852,4 +7852,28 @@ TEST_F(OpenMPIRBuilderTest, splitBB) {
     EXPECT_TRUE(DL == AllocaBB->getTerminator()->getStableDebugLoc());
 }
 
+TEST_F(OpenMPIRBuilderTest, spliceBBWithEmptyBB) {
+  OpenMPIRBuilder OMPBuilder(*M);
+  OMPBuilder.Config.IsTargetDevice = false;
+  OMPBuilder.initialize();
+  F->setName("func");
+  IRBuilder<> Builder(BB);
+
+  // Test calling spliceBB with an empty Block (but having trailing debug
+  // records).
+  DIBuilder DIB(*M);
+  DISubprogram *SP = F->getSubprogram();
+  DIType *VoidPtrTy =
+      DIB.createQualifiedType(dwarf::DW_TAG_pointer_type, nullptr);
+  DILocalVariable *Var = DIB.createParameterVariable(
+      SP, "test", /*ArgNo*/ 1, SP->getFile(), /*LineNo=*/0, VoidPtrTy);
+  DIB.insertDeclare(F->getArg(0), Var, DIB.createExpression(), DL,
+                    Builder.GetInsertPoint());
+  BasicBlock *New = BasicBlock::Create(Ctx, "", F);
+  spliceBB(Builder.saveIP(), New, true, DL);
+  Instruction *Terminator = BB->getTerminator();
+  EXPECT_NE(Terminator, nullptr);
+  EXPECT_FALSE(Terminator->getDbgRecordRange().empty());
+}
+
 } // namespace