Skip to content

[OMPIRBuilder] Prevent dangling InsertPt in IRBuilder. #148887

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

abidh
Copy link
Contributor

@abidh abidh commented Jul 15, 2025

There are places in PostOutlineCB callbacks where the instruction is deleted while InsertPt in the IRBuilder pointed to it causing a dangling InsertPt. This PR fixes that by ensuring that the InsertPt is always valid.

This fixes the sanitizer fail that was seen in #148284.

There are places in PostOutlineCB callbacks where the instruction is
deleted while InsertPt in the IRBuilder pointed to it causing dangling
InsertPt. This PR fixes that by ensuring that the InsertPt is always
valid.

This fixes the sanitizer fail that was seen in llvm#148284.
@abidh abidh requested review from skatrak, tblah and TIFitis July 15, 2025 16:43
@llvmbot llvmbot added flang:openmp clang:openmp OpenMP related changes to Clang labels Jul 15, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 15, 2025

@llvm/pr-subscribers-flang-openmp

Author: Abid Qadeer (abidh)

Changes

There are places in PostOutlineCB callbacks where the instruction is deleted while InsertPt in the IRBuilder pointed to it causing a dangling InsertPt. This PR fixes that by ensuring that the InsertPt is always valid.

This fixes the sanitizer fail that was seen in #148284.


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

1 Files Affected:

  • (modified) llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp (+6-2)
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 4f08ea97378c2..a3e0d7c89357b 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -2155,9 +2155,11 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::createTask(
       OutlinedFn.getArg(1)->replaceUsesWithIf(
           Shareds, [Shareds](Use &U) { return U.getUser() != Shareds; });
     }
-
-    for (Instruction *I : llvm::reverse(ToBeDeleted))
+    for (Instruction *I : llvm::reverse(ToBeDeleted)) {
+      if (I->getIterator() == Builder.GetInsertPoint())
+        Builder.SetInsertPoint(I->getParent());
       I->eraseFromParent();
+    }
   };
 
   addOutlineInfo(std::move(OI));
@@ -7714,6 +7716,7 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::emitTargetTask(
       Builder.CreateCall(TaskFn, {Ident, ThreadID, TaskData});
     }
 
+    Builder.SetInsertPoint(StaleCI->getParent());
     StaleCI->eraseFromParent();
     for (Instruction *I : llvm::reverse(ToBeDeleted))
       I->eraseFromParent();
@@ -9511,6 +9514,7 @@ OpenMPIRBuilder::createTeams(const LocationDescription &Loc,
                            omp::RuntimeFunction::OMPRTL___kmpc_fork_teams),
                        Args);
 
+    Builder.SetInsertPoint(StaleCI->getParent());
     for (Instruction *I : llvm::reverse(ToBeDeleted))
       I->eraseFromParent();
   };

@tblah
Copy link
Contributor

tblah commented Jul 21, 2025

Thanks for fixing this. Is it possible to set no insert point? Like a nullptr. The insert points added here aren't really meaningful to anything that tries to use them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:openmp OpenMP related changes to Clang flang:openmp
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants