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

[Abid Qadeer via llvm-commits]

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

Calling `BasicBlock::splice` in `spliceBB` 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 Ops to llvm intrinsics. Old is the BasicBlock where they were get inserted and it will have 2 llvm debug intrinsics by the time the implementation of `omp.teams`  starts. This implementation creates many BasicBlocks by calling `splitBB`. The `New` is the just created BasicBlock which is empty.

In the new scheme (using debug records), there will be no instruction in the `Old` BB after llvm.intr Ops get translated but just 2 trailing debug records. So both `Old` and `New` are empty. When control reaches `BasicBlock::splice`, it calls `spliceDebugInfoEmptyBlock`. This function 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(). The fix is to only call `BasicBlock::splice` when at least of `Old` or `New` is not empty.



>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] [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);