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

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-flang-openmp

Author: Abid Qadeer (abidh)

<details>
<summary>Changes</summary>

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.



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


1 Files Affected:

- (modified) llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp (+2-1) 


``````````diff
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);

``````````

</details>


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