Skip to content

Commit 9e39793

Browse files
jckarterjckarter
authored
Merge pull request #67313 from jckarter/switch-expr-initialization-fix
SILGen: Don't reuse the `Initialization` across branches of an `if` or `switch` expression.
2 parents 534a078 + c52ae08 commit 9e39793

8 files changed

+92
-45
lines changed

‎lib/SILGen/SILGenExpr.cpp

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2200,17 +2200,9 @@ RValue RValueEmitter::visitSingleValueStmtExpr(SingleValueStmtExpr *E,
22002200
auto &lowering = SGF.getTypeLowering(E->getType());
22012201
auto resultAddr = SGF.emitTemporaryAllocation(E, lowering.getLoweredType());
22022202

2203-
// This won't give us a useful diagnostic if the result doesn't end up
2204-
// initialized ("variable '<unknown>' used before being initialized"), but it
2205-
// will at least catch a potential miscompile when the SIL verifier is
2206-
// disabled.
2207-
resultAddr = SGF.B.createMarkUninitialized(
2208-
E, resultAddr, MarkUninitializedInst::Kind::Var);
2209-
KnownAddressInitialization init(resultAddr);
2210-
22112203
// Collect the target exprs that will be used for initialization.
22122204
SmallVector<Expr *, 4> scratch;
2213-
SILGenFunction::SingleValueStmtInitialization initInfo(&init);
2205+
SILGenFunction::SingleValueStmtInitialization initInfo(resultAddr);
22142206
for (auto *E : E->getSingleExprBranches(scratch))
22152207
initInfo.Exprs.insert(E);
22162208

‎lib/SILGen/SILGenFunction.cpp

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1650,7 +1650,7 @@ void SILGenFunction::emitGeneratorFunction(
16501650
mergeCleanupBlocks();
16511651
}
16521652

1653-
Initialization *SILGenFunction::getSingleValueStmtInit(Expr *E) {
1653+
std::unique_ptr<Initialization> SILGenFunction::getSingleValueStmtInit(Expr *E) {
16541654
if (SingleValueStmtInitStack.empty())
16551655
return nullptr;
16561656

@@ -1659,7 +1659,15 @@ Initialization *SILGenFunction::getSingleValueStmtInit(Expr *E) {
16591659
if (!SingleValueStmtInitStack.back().Exprs.contains(E))
16601660
return nullptr;
16611661

1662-
return SingleValueStmtInitStack.back().Init;
1662+
// This won't give us a useful diagnostic if the result doesn't end up
1663+
// initialized ("variable '<unknown>' used before being initialized"), but it
1664+
// will at least catch a potential miscompile when the SIL verifier is
1665+
// disabled.
1666+
auto resultAddr = SingleValueStmtInitStack.back().InitializationBuffer;
1667+
resultAddr = B.createMarkUninitialized(
1668+
E, resultAddr, MarkUninitializedInst::Kind::Var);
1669+
1670+
return std::make_unique<KnownAddressInitialization>(resultAddr);
16631671
}
16641672

16651673
void SILGenFunction::emitProfilerIncrement(ASTNode Node) {

‎lib/SILGen/SILGenFunction.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -353,9 +353,10 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
353353
struct SingleValueStmtInitialization {
354354
/// The target expressions to be used for initialization.
355355
SmallPtrSet<Expr *, 4> Exprs;
356-
Initialization *Init;
356+
SILValue InitializationBuffer;
357357

358-
SingleValueStmtInitialization(Initialization *init) : Init(init) {}
358+
SingleValueStmtInitialization(SILValue buffer)
359+
: InitializationBuffer(buffer) {}
359360
};
360361

361362
/// A stack of active SingleValueStmtExpr initializations that may be
@@ -741,7 +742,7 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
741742
/// Check to see if an initalization for a SingleValueStmtExpr is active, and
742743
/// if the provided expression is for one of its branches. If so, returns the
743744
/// initialization to use for the expression. Otherwise returns \c nullptr.
744-
Initialization *getSingleValueStmtInit(Expr *E);
745+
std::unique_ptr<Initialization> getSingleValueStmtInit(Expr *E);
745746

746747
//===--------------------------------------------------------------------===//
747748
// Entry points for codegen

‎lib/SILGen/SILGenLValue.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2981,6 +2981,7 @@ LValue SILGenLValue::visitRec(Expr *e, SGFAccessKind accessKind,
29812981
// a `borrow x` operator, the operator is used on the base here), we want to
29822982
// apply the lvalue within a formal access to the original value instead of
29832983
// an actual loaded copy.
2984+
29842985
if (e->getType()->isPureMoveOnly()) {
29852986
if (auto load = dyn_cast<LoadExpr>(e)) {
29862987
LValue lv = visitRec(load->getSubExpr(), SGFAccessKind::BorrowedAddressRead,
@@ -2992,7 +2993,7 @@ LValue SILGenLValue::visitRec(Expr *e, SGFAccessKind accessKind,
29922993
return lv;
29932994
}
29942995
}
2995-
2996+
29962997
// Otherwise we have a non-lvalue type (references, values, metatypes,
29972998
// etc). These act as the root of a logical lvalue. Compute the root value,
29982999
// wrap it in a ValueComponent, and return it for our caller.

‎lib/SILGen/SILGenStmt.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -423,9 +423,9 @@ void StmtEmitter::visitBraceStmt(BraceStmt *S) {
423423
// active, and if so, use it for this expression branch. If the expression
424424
// is uninhabited, we can skip this, and let unreachability checking
425425
// handle it.
426-
auto *init = SGF.getSingleValueStmtInit(E);
426+
auto init = SGF.getSingleValueStmtInit(E);
427427
if (init && !E->getType()->isStructurallyUninhabited()) {
428-
SGF.emitExprInto(E, init);
428+
SGF.emitExprInto(E, init.get());
429429
} else {
430430
SGF.emitIgnoredExpr(E);
431431
}

‎test/SILGen/if_expr.swift

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,23 +7,24 @@ func foo() -> Int {
77

88
// CHECK-LABEL: sil hidden [ossa] @$s7if_expr3fooSiyF : $@convention(thin) () -> Int
99
// CHECK: [[RESULT_STORAGE:%[0-9]+]] = alloc_stack $Int
10-
// CHECK: [[RESULT:%[0-9]+]] = mark_uninitialized [var] [[RESULT_STORAGE]] : $*Int
1110
// CHECK: cond_br {{%[0-9]+}}, [[TRUEBB:bb[0-9]+]], [[FALSEBB:bb[0-9]+]]
1211
//
1312
// CHECK: [[TRUEBB]]:
13+
// CHECK: [[RESULT:%[0-9]+]] = mark_uninitialized [var] [[RESULT_STORAGE]] : $*Int
1414
// CHECK: [[ONE_BUILTIN:%[0-9]+]] = integer_literal $Builtin.IntLiteral, 1
1515
// CHECK: [[ONE:%[0-9]+]] = apply {{%[0-9]+}}([[ONE_BUILTIN]], {{%[0-9]+}}) : $@convention(method) (Builtin.IntLiteral, @thin Int.Type) -> Int
1616
// CHECK: store [[ONE]] to [trivial] [[RESULT]] : $*Int
1717
// CHECK: br [[EXITBB:bb[0-9]+]]
1818
//
1919
// CHECK: [[FALSEBB]]:
20+
// CHECK: [[RESULT:%[0-9]+]] = mark_uninitialized [var] [[RESULT_STORAGE]] : $*Int
2021
// CHECK: [[TWO_BUILTIN:%[0-9]+]] = integer_literal $Builtin.IntLiteral, 2
2122
// CHECK: [[TWO:%[0-9]+]] = apply {{%[0-9]+}}([[TWO_BUILTIN]], {{%[0-9]+}}) : $@convention(method) (Builtin.IntLiteral, @thin Int.Type) -> Int
2223
// CHECK: store [[TWO]] to [trivial] [[RESULT]] : $*Int
2324
// CHECK: br [[EXITBB]]
2425
//
2526
// CHECK: [[EXITBB]]:
26-
// CHECK: [[VAL:%[0-9]+]] = load [trivial] [[RESULT]] : $*Int
27+
// CHECK: [[VAL:%[0-9]+]] = load [trivial] [[RESULT_STORAGE]] : $*Int
2728
// CHECK: dealloc_stack [[RESULT_STORAGE]] : $*Int
2829
// CHECK: return [[VAL]] : $Int
2930

@@ -36,22 +37,23 @@ func bar(_ x: C) -> C {
3637
// CHECK-LABEL: sil hidden [ossa] @$s7if_expr3baryAA1CCADF : $@convention(thin) (@guaranteed C) -> @owned C
3738
// CHECK: bb0([[CPARAM:%[0-9]+]] : @guaranteed $C):
3839
// CHECK: [[RESULT_STORAGE:%[0-9]+]] = alloc_stack $C
39-
// CHECK: [[RESULT:%[0-9]+]] = mark_uninitialized [var] [[RESULT_STORAGE]] : $*C
4040
// CHECK: cond_br {{%[0-9]+}}, [[TRUEBB:bb[0-9]+]], [[FALSEBB:bb[0-9]+]]
4141
//
4242
// CHECK: [[TRUEBB]]:
43+
// CHECK: [[RESULT:%[0-9]+]] = mark_uninitialized [var] [[RESULT_STORAGE]] : $*C
4344
// CHECK: [[C:%[0-9]+]] = copy_value [[CPARAM]] : $C
4445
// CHECK: store [[C]] to [init] [[RESULT]] : $*C
4546
// CHECK: br [[EXITBB:bb[0-9]+]]
4647
//
4748
// CHECK: [[FALSEBB]]:
49+
// CHECK: [[RESULT:%[0-9]+]] = mark_uninitialized [var] [[RESULT_STORAGE]] : $*C
4850
// CHECK: [[CTOR:%[0-9]+]] = function_ref @$s7if_expr1CCACycfC : $@convention(method) (@thick C.Type) -> @owned C
4951
// CHECK: [[C:%[0-9]+]] = apply [[CTOR]]({{%[0-9]+}}) : $@convention(method) (@thick C.Type) -> @owned C
5052
// CHECK: store [[C]] to [init] [[RESULT]] : $*C
5153
// CHECK: br [[EXITBB]]
5254
//
5355
// CHECK: [[EXITBB]]:
54-
// CHECK: [[VAL:%[0-9]+]] = load [take] [[RESULT]] : $*C
56+
// CHECK: [[VAL:%[0-9]+]] = load [take] [[RESULT_STORAGE]] : $*C
5557
// CHECK: dealloc_stack [[RESULT_STORAGE]] : $*C
5658
// CHECK: return [[VAL]] : $C
5759

@@ -69,7 +71,6 @@ func baz() throws -> Int {
6971

7072
// CHECK-LABEL: sil hidden [ossa] @$s7if_expr3bazSiyKF : $@convention(thin) () -> (Int, @error any Error)
7173
// CHECK: [[RESULT_STORAGE:%[0-9]+]] = alloc_stack $Int
72-
// CHECK: [[RESULT:%[0-9]+]] = mark_uninitialized [var] [[RESULT_STORAGE]] : $*Int
7374
// CHECK: cond_br {{%[0-9]+}}, [[TRUEBB:bb[0-9]+]], [[FALSEBB:bb[0-9]+]]
7475
//
7576
// CHECK: [[FALSEBB]]:
@@ -82,7 +83,7 @@ func baz() throws -> Int {
8283
// CHECK: br [[EXITBB:bb[0-9]+]]
8384
//
8485
// CHECK: [[EXITBB]]:
85-
// CHECK: [[VAL:%[0-9]+]] = load [trivial] [[RESULT]] : $*Int
86+
// CHECK: [[VAL:%[0-9]+]] = load [trivial] [[RESULT_STORAGE]] : $*Int
8687
// CHECK: dealloc_stack [[RESULT_STORAGE]] : $*Int
8788
// CHECK: return [[VAL]] : $Int
8889

@@ -92,18 +93,18 @@ func qux() throws -> Int {
9293

9394
// CHECK-LABEL: sil hidden [ossa] @$s7if_expr3quxSiyKF : $@convention(thin) () -> (Int, @error any Error)
9495
// CHECK: [[RESULT_STORAGE:%[0-9]+]] = alloc_stack $Int
95-
// CHECK: [[RESULT:%[0-9]+]] = mark_uninitialized [var] [[RESULT_STORAGE]] : $*Int
9696
// CHECK: cond_br {{%[0-9]+}}, [[TRUEBB:bb[0-9]+]], [[FALSEBB:bb[0-9]+]]
9797
//
9898
// CHECK: [[FALSEBB]]:
99+
// CHECK: [[RESULT:%[0-9]+]] = mark_uninitialized [var] [[RESULT_STORAGE]] : $*Int
99100
// CHECK: try_apply {{%[0-9]+}}() : $@convention(thin) () -> (Int, @error any Error), normal [[NORMALBB:bb[0-9]+]], error [[ERRORBB:bb[0-9]+]]
100101
//
101102
// CHECK: [[NORMALBB]]([[BAZVAL:%[0-9]+]] : $Int):
102103
// CHECK: store [[BAZVAL]] to [trivial] [[RESULT]] : $*Int
103104
// CHECK: br [[EXITBB:bb[0-9]+]]
104105
//
105106
// CHECK: [[EXITBB]]:
106-
// CHECK: [[VAL:%[0-9]+]] = load [trivial] [[RESULT]] : $*Int
107+
// CHECK: [[VAL:%[0-9]+]] = load [trivial] [[RESULT_STORAGE]] : $*Int
107108
// CHECK: dealloc_stack [[RESULT_STORAGE]] : $*Int
108109
// CHECK: return [[VAL]] : $Int
109110
//
@@ -141,18 +142,18 @@ func testClosure() throws -> Int {
141142

142143
// CHECK-LABEL: sil private [ossa] @$s7if_expr11testClosureSiyKFSiyKcfU_ : $@convention(thin) () -> (Int, @error any Error)
143144
// CHECK: [[RESULT_STORAGE:%[0-9]+]] = alloc_stack $Int
144-
// CHECK: [[RESULT:%[0-9]+]] = mark_uninitialized [var] [[RESULT_STORAGE]] : $*Int
145145
// CHECK: cond_br {{%[0-9]+}}, [[TRUEBB:bb[0-9]+]], [[FALSEBB:bb[0-9]+]]
146146
//
147147
// CHECK: [[FALSEBB]]:
148+
// CHECK: [[RESULT:%[0-9]+]] = mark_uninitialized [var] [[RESULT_STORAGE]] : $*Int
148149
// CHECK: try_apply {{%[0-9]+}}() : $@convention(thin) () -> (Int, @error any Error), normal [[NORMALBB:bb[0-9]+]], error [[ERRORBB:bb[0-9]+]]
149150
//
150151
// CHECK: [[NORMALBB]]([[BAZVAL:%[0-9]+]] : $Int):
151152
// CHECK: store [[BAZVAL]] to [trivial] [[RESULT]] : $*Int
152153
// CHECK: br [[EXITBB:bb[0-9]+]]
153154
//
154155
// CHECK: [[EXITBB]]:
155-
// CHECK: [[VAL:%[0-9]+]] = load [trivial] [[RESULT]] : $*Int
156+
// CHECK: [[VAL:%[0-9]+]] = load [trivial] [[RESULT_STORAGE]] : $*Int
156157
// CHECK: dealloc_stack [[RESULT_STORAGE]] : $*Int
157158
// CHECK: return [[VAL]] : $Int
158159
//
@@ -174,7 +175,6 @@ func testNested() throws -> Int {
174175

175176
// CHECK-LABEL: sil hidden [ossa] @$s7if_expr10testNestedSiyKF : $@convention(thin) () -> (Int, @error any Error)
176177
// CHECK: [[RESULT_STORAGE:%[0-9]+]] = alloc_stack $Int
177-
// CHECK: [[RESULT:%[0-9]+]] = mark_uninitialized [var] [[RESULT_STORAGE]] : $*Int
178178
// CHECK: cond_br {{%[0-9]+}}, [[TRUEBB:bb[0-9]+]], [[FALSEBB:bb[0-9]+]]
179179
//
180180
// CHECK: [[FALSEBB]]:
@@ -187,7 +187,7 @@ func testNested() throws -> Int {
187187
// CHECK: br [[EXITBB:bb[0-9]+]]
188188
//
189189
// CHECK: [[EXITBB]]:
190-
// CHECK: [[VAL:%[0-9]+]] = load [trivial] [[RESULT]] : $*Int
190+
// CHECK: [[VAL:%[0-9]+]] = load [trivial] [[RESULT_STORAGE]] : $*Int
191191
// CHECK: dealloc_stack [[RESULT_STORAGE]] : $*Int
192192
// CHECK: return [[VAL]] : $Int
193193

0 commit comments

Comments
 (0)