https://github.com/steffenlarsen created
https://github.com/llvm/llvm-project/pull/230449
CIR may currently silently miscompile functions containing gotos if mem2reg is
run on them before the goto-solver. Until it lowers them to branches, cir.goto
and cir.indirect_goto are terminators without successors, so the block holding
the target cir.label does not list them as predecessors. mem2reg therefore
computes the value reaching the label from the fallthrough path only, and a
load after the label gets that value (or undef) instead of the value stored
before the goto.
For example, if mem2reg is run before the goto-solver, a C++ program like
```c++
int g(int a) {
int e = f(a);
if (e) goto out;
e = 7;
out:
return e;
}
```
would always return 7, even if f(a) returned a non-zero value.
To fix this, cir.alloca no longer reports a promotable slot while its enclosing
function still contains an unresolved goto. Once the goto-solver has run,
promotion proceeds as before.
Note that this is currently not reachable through the regular CIR pipeline, but
is reachable through pipelines that runs mem2reg before the goto-solver.
Assisted-by: Claude Opus 5.5
>From 25934bb26d63eed421f2e57d47a749255fd563fd Mon Sep 17 00:00:00 2001
From: Steffen Holst Larsen <[email protected]>
Date: Fri, 9 Oct 2026 04:32:29 -0500
Subject: [PATCH] [CIR] Avoid promoting memory slots if gotos still exist
CIR may currently silently miscompile functions containing gotos if
mem2reg is run on them before the goto-solver. Until it lowers them to
branches, cir.goto and cir.indirect_goto are terminators without
successors, so the block holding the target cir.label does not list them
as predecessors. mem2reg therefore computes the value reaching the label
from the fallthrough path only, and a load after the label gets that
value (or undef) instead of the value stored before the goto.
For example, if mem2reg is run before the goto-solver, a C++ program
like
```c++
int g(int a) {
int e = f(a);
if (e) goto out;
e = 7;
out:
return e;
}
```
would always return 7, even if f(a) returned a non-zero value.
To fix this, cir.alloca no longer reports a promotable slot while its
enclosing function still contains an unresolved goto. Once GotoSolver
has run, promotion proceeds as before.
Note that this is currently not reachable through the regular CIR
pipeline, but is reachable through pipelines that runs mem2reg before
the goto-solver.
Assisted-by: Claude Opus 5.5
Signed-off-by: Steffen Holst Larsen <[email protected]>
---
clang/lib/CIR/Dialect/IR/CIRMemorySlot.cpp | 22 +++++++++
clang/test/CIR/Transforms/mem2reg.cir | 56 ++++++++++++++++++++++
2 files changed, 78 insertions(+)
diff --git a/clang/lib/CIR/Dialect/IR/CIRMemorySlot.cpp
b/clang/lib/CIR/Dialect/IR/CIRMemorySlot.cpp
index 089303c0cbf2e4..002fafde72d52a 100644
--- a/clang/lib/CIR/Dialect/IR/CIRMemorySlot.cpp
+++ b/clang/lib/CIR/Dialect/IR/CIRMemorySlot.cpp
@@ -28,7 +28,29 @@ static bool forwardToUsers(Operation *op,
// Interfaces for AllocaOp
//===----------------------------------------------------------------------===//
+/// Returns true if the scope still contains a cir.goto or cir.indirect_goto.
+/// Until GotoSolver turns them into branches, these jumps are terminators
+/// without successors, so the block holding the target cir.label does not list
+/// them as predecessors.
+static bool hasUnresolvedGotos(Operation *scope) {
+ return scope
+ ->walk([](Block *block) {
+ return (!block->empty() &&
+ isa<cir::GotoOp, cir::IndirectGotoOp>(block->back()))
+ ? WalkResult::interrupt()
+ : WalkResult::advance();
+ })
+ .wasInterrupted();
+}
+
llvm::SmallVector<MemorySlot> cir::AllocaOp::getPromotableSlots() {
+ // Promotion computes reaching definitions from the CFG, which misses the
+ // edges of unresolved gotos, so a load after a cir.label would be given the
+ // value from the fallthrough path only. Leave the slot in memory until the
+ // gotos are lowered.
+ if (hasUnresolvedGotos(
+ getOperation()->getParentWithTrait<OpTrait::IsIsolatedFromAbove>()))
+ return {};
return {MemorySlot{getResult(), getAllocaType()}};
}
diff --git a/clang/test/CIR/Transforms/mem2reg.cir
b/clang/test/CIR/Transforms/mem2reg.cir
index 3162a3f5e09a5d..eb5561ebabf5f7 100644
--- a/clang/test/CIR/Transforms/mem2reg.cir
+++ b/clang/test/CIR/Transforms/mem2reg.cir
@@ -1,5 +1,7 @@
// RUN: cir-opt %s -cir-flatten-cfg -mem2reg -o - | FileCheck %s
+// RUN: cir-opt %s -cir-flatten-cfg -cir-goto-solver -mem2reg -o - | FileCheck
%s --check-prefix=SOLVED
+!void = !cir.void
!s32i = !cir.int<s, 32>
!u8i = !cir.int<u, 8>
!u16i = !cir.int<u, 16>
@@ -94,4 +96,58 @@ module {
%v = cir.load %slot : !cir.ptr<!rec_AllPad>, !rec_AllPad
cir.return %v : !rec_AllPad
}
+
+ // An unresolved cir.goto (direct or indirect) is not a predecessor of the
+ // block holding its cir.label, so promoting here would return the value
+ // stored on the dead fallthrough path (or undef) instead of %arg0. Once
+ // GotoSolver has turned the goto into a branch, the slot promotes.
+
+ // CHECK-LABEL: cir.func @do_not_promote_across_goto
+ // SOLVED-LABEL: cir.func @do_not_promote_across_goto
+ cir.func @do_not_promote_across_goto(%arg0: !s32i) -> !s32i {
+ // CHECK: %[[SLOT:.*]] = cir.alloca "e" align(4) : !cir.ptr<!s32i>
+ // CHECK: cir.goto "out"
+ // CHECK: cir.label "out"
+ // CHECK: %[[RET:.+]] = cir.load %[[SLOT]] : !cir.ptr<!s32i>, !s32i
+ // CHECK: cir.return %[[RET]] : !s32i
+ // SOLVED-NOT: cir.alloca
+ // SOLVED: cir.return %arg0 : !s32i
+ %0 = cir.alloca "e" align(4) : !cir.ptr<!s32i>
+ cir.store %arg0, %0 : !s32i, !cir.ptr<!s32i>
+ cir.goto "out"
+ ^bb1:
+ %1 = cir.const #cir.int<7> : !s32i
+ cir.store %1, %0 : !s32i, !cir.ptr<!s32i>
+ cir.br ^bb2
+ ^bb2:
+ cir.label "out"
+ %2 = cir.load %0 : !cir.ptr<!s32i>, !s32i
+ cir.return %2 : !s32i
+ }
+
+ // CHECK-LABEL: cir.func @do_not_promote_across_indirect_goto
+ // SOLVED-LABEL: cir.func @do_not_promote_across_indirect_goto
+ cir.func @do_not_promote_across_indirect_goto(%arg0: !s32i) -> !s32i {
+ // CHECK: %[[SLOT:.*]] = cir.alloca "e" align(4) : !cir.ptr<!s32i>
+ // CHECK: cir.indirect_goto
+ // CHECK: cir.label "out"
+ // CHECK: %[[RET:.+]] = cir.load %[[SLOT]] : !cir.ptr<!s32i>, !s32i
+ // CHECK: cir.return %[[RET]] : !s32i
+ // SOLVED-NOT: cir.alloca
+ // SOLVED: cir.label "out"
+ // SOLVED-NEXT: cir.return %arg0 : !s32i
+ // SOLVED: cir.indirect_br
+ %0 = cir.alloca "e" align(4) : !cir.ptr<!s32i>
+ cir.store %arg0, %0 : !s32i, !cir.ptr<!s32i>
+ %1 = cir.block_address <@do_not_promote_across_indirect_goto, "out"> :
!cir.ptr<!void>
+ cir.indirect_goto %1 : !cir.ptr<!void>
+ ^bb1:
+ %2 = cir.const #cir.int<7> : !s32i
+ cir.store %2, %0 : !s32i, !cir.ptr<!s32i>
+ cir.br ^bb2
+ ^bb2:
+ cir.label "out"
+ %3 = cir.load %0 : !cir.ptr<!s32i>, !s32i
+ cir.return %3 : !s32i
+ }
}
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
