Skip to content

[Arm] Fix UAF in ARMConstantIslandPass #146232

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

Conversation

qinkunbao
Copy link
Member

@qinkunbao qinkunbao commented Jun 28, 2025

#146198 changes

    for (unsigned i = 0, e = ImmBranches.size(); i != e; ++i)
      BRChange |= fixupImmediateBr(ImmBranches[i]);

to

    for (ImmBranch &Br : ImmBranches)
      BRChange |= fixupImmediateBr(Br);

Unfortunately, they are not NFC and cause the buildbot error. e.g.,
https://lab.llvm.org/buildbot/#/builders/24/builds/9943
https://lab.llvm.org/buildbot/#/builders/169/builds/12570
Use make_early_inc_range to fix the issue

Created using spr 1.3.6
@llvmbot
Copy link
Member

llvmbot commented Jun 28, 2025

@llvm/pr-subscribers-backend-arm

Author: Qinkun Bao (qinkunbao)

Changes

#146198 changes

    for (unsigned i = 0, e = ImmBranches.size(); i != e; ++i)
      BRChange |= fixupImmediateBr(ImmBranches[i]);

to

    for (ImmBranch &Br : ImmBranches)
      BRChange |= fixupImmediateBr(Br);

Unfortunately, they are not NFC and causes the buildbot error. e.g.,
https://lab.llvm.org/buildbot/#/builders/24/builds/9943
https://lab.llvm.org/buildbot/#/builders/169/builds/12570
Revoke the change to fix the bot.


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

1 Files Affected:

  • (modified) llvm/lib/Target/ARM/ARMConstantIslandPass.cpp (+2-2)
diff --git a/llvm/lib/Target/ARM/ARMConstantIslandPass.cpp b/llvm/lib/Target/ARM/ARMConstantIslandPass.cpp
index e72aa8ef051cd..ca3dc15ff3ad6 100644
--- a/llvm/lib/Target/ARM/ARMConstantIslandPass.cpp
+++ b/llvm/lib/Target/ARM/ARMConstantIslandPass.cpp
@@ -476,8 +476,8 @@ bool ARMConstantIslands::runOnMachineFunction(MachineFunction &mf) {
 
     LLVM_DEBUG(dbgs() << "Beginning BR iteration #" << NoBRIters << '\n');
     bool BRChange = false;
-    for (ImmBranch &Br : ImmBranches)
-      BRChange |= fixupImmediateBr(Br);
+    for (unsigned i = 0, e = ImmBranches.size(); i != e; ++i)
+      BRChange |= fixupImmediateBr(ImmBranches[i]);
     if (BRChange && ++NoBRIters > 30)
       report_fatal_error("Branch Fix Up pass failed to converge!");
     LLVM_DEBUG(dumpBBs());

@qinkunbao qinkunbao changed the title [Arm] Fix UAF in https://github.com/llvm/llvm-project/pull/146198 [Arm] Fix UAF in ARMConstantIslandPass Jun 28, 2025
Copy link
Contributor

@kazutakahirata kazutakahirata 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!

@arsenm
Copy link
Contributor

arsenm commented Jun 29, 2025

I'm assuming this is fixupImmediateBr resizing the vector, can you use make_early_inc_range

Created using spr 1.3.6
@qinkunbao qinkunbao merged commit 2248cdf into main Jun 29, 2025
7 checks passed
@qinkunbao qinkunbao deleted the users/qinkunbao/spr/arm-fix-uaf-in-httpsgithubcomllvmllvm-projectpull146198 branch June 29, 2025 04:20
@qinkunbao
Copy link
Member Author

Looks like it does not work.
@thurstond FYI

rlavaee pushed a commit to rlavaee/llvm-project that referenced this pull request Jul 1, 2025
llvm#146198 changes
```
    for (unsigned i = 0, e = ImmBranches.size(); i != e; ++i)
      BRChange |= fixupImmediateBr(ImmBranches[i]);
```
to
```
    for (ImmBranch &Br : ImmBranches)
      BRChange |= fixupImmediateBr(Br);
```
Unfortunately, they are not NFC and cause the buildbot error. e.g.,
https://lab.llvm.org/buildbot/#/builders/24/builds/9943
https://lab.llvm.org/buildbot/#/builders/169/builds/12570
Use make_early_inc_range to fix the issue
rlavaee pushed a commit to rlavaee/llvm-project that referenced this pull request Jul 1, 2025
llvm#146198 changes
```
    for (unsigned i = 0, e = ImmBranches.size(); i != e; ++i)
      BRChange |= fixupImmediateBr(ImmBranches[i]);
```
to
```
    for (ImmBranch &Br : ImmBranches)
      BRChange |= fixupImmediateBr(Br);
```
Unfortunately, they are not NFC and cause the buildbot error. e.g.,
https://lab.llvm.org/buildbot/#/builders/24/builds/9943
https://lab.llvm.org/buildbot/#/builders/169/builds/12570
Use make_early_inc_range to fix the issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants