Skip to content

Commit 9bdf395

Browse files
authored
Merge pull request #62039 from eeckstein/fix-simplifycfg
SimplifyCFG: fix an argument use-after-erase in CheckedCastBrJumpThreading
2 parents 8c85936 + 6536739 commit 9bdf395

File tree

5 files changed

+108
-1
lines changed

5 files changed

+108
-1
lines changed

include/swift/SIL/SILArgument.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,14 @@ class SILArgument : public ValueBase {
101101

102102
SILBasicBlock *getParent() const { return parentBlock; }
103103

104+
/// Returns true if this argument is erased from a basic block.
105+
///
106+
/// Note that SILArguments which are erased from a SILBasicBlock are not
107+
/// destroyed and freed, but are kept in memory. So it's safe to keep a
108+
/// pointer to an erased argument and then at a later time check if its
109+
/// erased.
110+
bool isErased() const { return !parentBlock; }
111+
104112
SILFunction *getFunction();
105113
const SILFunction *getFunction() const;
106114

include/swift/SIL/SILBasicBlock.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ public SwiftObjectHeader {
308308
const ValueDecl *D = nullptr);
309309

310310
/// Remove all block arguments.
311-
void dropAllArguments() { ArgumentList.clear(); }
311+
void dropAllArguments();
312312

313313
//===--------------------------------------------------------------------===//
314314
// Successors

lib/SIL/IR/SILBasicBlock.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ SILFunctionArgument *SILBasicBlock::replaceFunctionArgument(
180180

181181
SILFunctionArgument *NewArg = new (M) SILFunctionArgument(Ty, Kind, D);
182182
NewArg->setParent(this);
183+
ArgumentList[i]->parentBlock = nullptr;
183184

184185
// TODO: When we switch to malloc/free allocation we'll be leaking memory
185186
// here.
@@ -203,6 +204,7 @@ SILPhiArgument *SILBasicBlock::replacePhiArgument(unsigned i, SILType Ty,
203204

204205
SILPhiArgument *NewArg = new (M) SILPhiArgument(Ty, Kind, D);
205206
NewArg->setParent(this);
207+
ArgumentList[i]->parentBlock = nullptr;
206208

207209
// TODO: When we switch to malloc/free allocation we'll be leaking memory
208210
// here.
@@ -257,9 +259,17 @@ SILPhiArgument *SILBasicBlock::insertPhiArgument(unsigned AtArgPos, SILType Ty,
257259
return arg;
258260
}
259261

262+
void SILBasicBlock::dropAllArguments() {
263+
for (SILArgument *arg : ArgumentList) {
264+
arg->parentBlock = nullptr;
265+
}
266+
ArgumentList.clear();
267+
}
268+
260269
void SILBasicBlock::eraseArgument(int Index) {
261270
assert(getArgument(Index)->use_empty() &&
262271
"Erasing block argument that has uses!");
272+
ArgumentList[Index]->parentBlock = nullptr;
263273
ArgumentList.erase(ArgumentList.begin() + Index);
264274
}
265275

lib/SILOptimizer/Utils/CheckedCastBrJumpThreading.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -771,6 +771,9 @@ void CheckedCastBrJumpThreading::optimizeFunction() {
771771
Fn->verifyCriticalEdges();
772772

773773
for (Edit *edit : Edits) {
774+
if (edit->SuccessArg->isErased())
775+
continue;
776+
774777
BasicBlockCloner Cloner(edit->CCBBlock, deBlocks);
775778
if (!Cloner.canCloneBlock())
776779
continue;

test/SILOptimizer/simplify_cfg.sil

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3233,3 +3233,89 @@ bb6:
32333233
%r = tuple()
32343234
return %r : $()
32353235
}
3236+
3237+
// CHECK-LABEL: sil @dont_crash_in_cast_jump_threading
3238+
// CHECK: checked_cast_br
3239+
// CHECK: } // end sil function 'dont_crash_in_cast_jump_threading'
3240+
sil @dont_crash_in_cast_jump_threading : $@convention(thin) (Optional<Base>) -> () {
3241+
bb0(%0 : $Optional<Base>):
3242+
br bb1
3243+
3244+
bb1:
3245+
switch_enum %0 : $Optional<Base>, case #Optional.some!enumelt: bb2, case #Optional.none!enumelt: bb21
3246+
3247+
3248+
bb2(%18 : $Base):
3249+
checked_cast_br %18 : $Base to Derived, bb3, bb4
3250+
3251+
bb3(%21 : $Derived):
3252+
%22 = integer_literal $Builtin.Int1, -1
3253+
br bb5(%22 : $Builtin.Int1)
3254+
3255+
bb4:
3256+
%24 = integer_literal $Builtin.Int1, 0
3257+
br bb5(%24 : $Builtin.Int1)
3258+
3259+
3260+
bb5(%26 : $Builtin.Int1):
3261+
cond_br %26, bb6, bb19
3262+
3263+
bb6:
3264+
checked_cast_br %18 : $Base to Derived, bb7, bb8
3265+
3266+
3267+
bb7(%29 : $Derived):
3268+
%30 = enum $Optional<Derived>, #Optional.some!enumelt, %29 : $Derived
3269+
br bb9(%30 : $Optional<Derived>)
3270+
3271+
bb8:
3272+
%32 = enum $Optional<Derived>, #Optional.none!enumelt
3273+
br bb9(%32 : $Optional<Derived>)
3274+
3275+
3276+
bb9(%34 : $Optional<Derived>):
3277+
switch_enum %34 : $Optional<Derived>, case #Optional.some!enumelt: bb10, case #Optional.none!enumelt: bb18
3278+
3279+
3280+
bb10(%36 : $Derived):
3281+
br bb11
3282+
3283+
bb11:
3284+
checked_cast_br %18 : $Base to Derived, bb12, bb13
3285+
3286+
3287+
bb12(%43 : $Derived):
3288+
%44 = enum $Optional<Derived>, #Optional.some!enumelt, %43 : $Derived
3289+
br bb14(%44 : $Optional<Derived>)
3290+
3291+
bb13:
3292+
%46 = enum $Optional<Derived>, #Optional.none!enumelt
3293+
br bb14(%46 : $Optional<Derived>)
3294+
3295+
3296+
bb14(%48 : $Optional<Derived>):
3297+
switch_enum %48 : $Optional<Derived>, case #Optional.some!enumelt: bb15, case #Optional.none!enumelt: bb17
3298+
3299+
3300+
bb15(%50 : $Derived):
3301+
br bb16
3302+
3303+
bb16:
3304+
br bb20
3305+
3306+
bb17:
3307+
br bb16
3308+
3309+
bb18:
3310+
br bb11
3311+
3312+
bb19:
3313+
br bb20
3314+
3315+
bb20:
3316+
br bb1
3317+
3318+
bb21:
3319+
%75 = tuple ()
3320+
return %75 : $()
3321+
}

0 commit comments

Comments
 (0)