Skip to content

Conversation

noclowns
Copy link
Contributor

Fix UB caused by accessing the top element of the stack via a dangling reference after a call to .pop() This is tripping static analysis.

No functional changes. Performance impact is negligible, but alt. implementation of the fix is possible if needed.

Testing: Both functional and unit tests are passing.

Fix UB caused by accessing the top element of the stack via a dangling reference after a call to .pop()
This is tripping static analysis.

No functional changes. Performance impact is negligible, but alt. implementation of the fix is possible if needed.

Testing: Both functional and unit tests are passing.
@llvmbot
Copy link
Member

llvmbot commented Oct 14, 2025

@llvm/pr-subscribers-bolt

Author: Slava Gurevich (noclowns)

Changes

Fix UB caused by accessing the top element of the stack via a dangling reference after a call to .pop() This is tripping static analysis.

No functional changes. Performance impact is negligible, but alt. implementation of the fix is possible if needed.

Testing: Both functional and unit tests are passing.


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

2 Files Affected:

  • (modified) bolt/lib/Passes/FrameAnalysis.cpp (+1-1)
  • (modified) bolt/lib/Passes/ShrinkWrapping.cpp (+1-1)
diff --git a/bolt/lib/Passes/FrameAnalysis.cpp b/bolt/lib/Passes/FrameAnalysis.cpp
index f568039bbf163..0b26da3371234 100644
--- a/bolt/lib/Passes/FrameAnalysis.cpp
+++ b/bolt/lib/Passes/FrameAnalysis.cpp
@@ -198,7 +198,7 @@ class FrameAccessAnalysis {
         if (CFIStack.empty())
           dbgs() << "Assertion is about to fail: " << BF.getPrintName() << "\n";
         assert(!CFIStack.empty() && "Corrupt CFI stack");
-        std::pair<int64_t, uint16_t> &Elem = CFIStack.top();
+        std::pair<int64_t, uint16_t> Elem = CFIStack.top();
         CFIStack.pop();
         CfaOffset = Elem.first;
         CfaReg = Elem.second;
diff --git a/bolt/lib/Passes/ShrinkWrapping.cpp b/bolt/lib/Passes/ShrinkWrapping.cpp
index 4ea60f388e2fa..fe342ccd38a67 100644
--- a/bolt/lib/Passes/ShrinkWrapping.cpp
+++ b/bolt/lib/Passes/ShrinkWrapping.cpp
@@ -402,7 +402,7 @@ void StackLayoutModifier::classifyCFIs() {
         break;
       case MCCFIInstruction::OpRestoreState: {
         assert(!CFIStack.empty() && "Corrupt CFI stack");
-        std::pair<int64_t, uint16_t> &Elem = CFIStack.top();
+        std::pair<int64_t, uint16_t> Elem = CFIStack.top();
         CFIStack.pop();
         CfaOffset = Elem.first;
         CfaReg = Elem.second;

@bgergely0 bgergely0 self-requested a review October 14, 2025 08:56
@bgergely0
Copy link
Contributor

LGTM!

@noclowns noclowns merged commit 4a8dd49 into llvm:main Oct 14, 2025
12 checks passed
akadutta pushed a commit to akadutta/llvm-project that referenced this pull request Oct 14, 2025
Fix UB caused by accessing the top element of the stack via a dangling
reference after a call to .pop() This is tripping static analysis.

No functional changes. Performance impact is negligible, but alt.
implementation of the fix is possible if needed.

Testing: Both functional and unit tests are passing.
@noclowns noclowns deleted the bolt-fix-uaf branch October 14, 2025 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants