Skip to content
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

[flang][HLFIR] Fix use-after-free when rewriting users in canonicalize #84371

Merged
merged 2 commits into from
Mar 8, 2024

Conversation

kparzysz
Copy link
Contributor

@kparzysz kparzysz commented Mar 7, 2024

Rewriting an op can invalidate the operator range being iterated on. Store the users in a separate list, and iterate over the list instead.

This was detected by address sanitizer.

Rewriting an op can invalidate the operator range being iterated on. Store
the users in a separate list, and iterate over the list instead.

This was detected by address sanitizer.
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir labels Mar 7, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 7, 2024

@llvm/pr-subscribers-flang-fir-hlfir

Author: Krzysztof Parzyszek (kparzysz)

Changes

Rewriting an op can invalidate the operator range being iterated on. Store the users in a separate list, and iterate over the list instead.

This was detected by address sanitizer.


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

1 Files Affected:

  • (modified) flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp (+5-2)
diff --git a/flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp b/flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp
index 8bc92a991a69cf..74d94cd654b4a9 100644
--- a/flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp
+++ b/flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp
@@ -1152,7 +1152,9 @@ hlfir::MatmulOp::canonicalize(MatmulOp matmulOp,
 
       // but we do need to get rid of the hlfir.destroy for the hlfir.transpose
       // result (which is entirely removed)
-      for (mlir::Operation *user : transposeOp->getResult(0).getUsers())
+      llvm::SmallVector<mlir::Operation *> users(
+          transposeOp->getResult(0).getUsers());
+      for (mlir::Operation *user : users)
         if (auto destroyOp = mlir::dyn_cast_or_null<hlfir::DestroyOp>(user))
           rewriter.eraseOp(destroyOp);
       rewriter.eraseOp(transposeOp);
@@ -1864,7 +1866,8 @@ hlfir::ForallIndexOp::canonicalize(hlfir::ForallIndexOp indexOp,
       return mlir::failure();
 
   auto insertPt = rewriter.saveInsertionPoint();
-  for (mlir::Operation *user : indexOp->getResult(0).getUsers())
+  llvm::SmallVector<mlir::Operation*> users(indexOp->getResult(0).getUsers());
+  for (mlir::Operation *user : users)
     if (auto loadOp = mlir::dyn_cast<fir::LoadOp>(user)) {
       rewriter.setInsertionPoint(loadOp);
       rewriter.replaceOpWithNewOp<fir::ConvertOp>(

Copy link

github-actions bot commented Mar 7, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@jeanPerier jeanPerier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@kparzysz kparzysz merged commit 10b0156 into llvm:main Mar 8, 2024
4 checks passed
@kparzysz kparzysz deleted the users/kparzysz/uaf-02 branch March 8, 2024 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:fir-hlfir flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants