llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Steffen Larsen (steffenlarsen)
<details>
<summary>Changes</summary>
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
---
Full diff: https://github.com/llvm/llvm-project/pull/230449.diff
2 Files Affected:
- (modified) clang/lib/CIR/Dialect/IR/CIRMemorySlot.cpp (+22)
- (modified) clang/test/CIR/Transforms/mem2reg.cir (+56)
``````````diff
diff --git a/clang/lib/CIR/Dialect/IR/CIRMemorySlot.cpp
b/clang/lib/CIR/Dialect/IR/CIRMemorySlot.cpp
index 089303c0cbf2e..002fafde72d52 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 3162a3f5e09a5..eb5561ebabf5f 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
+ }
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/230449
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
