Skip to content

Commit 8e2da8d

Browse files
committed
Outliner: Fix a use after release issue
If there is a release of the bridged value in between the bridge call and the objective-c call we need to account for that and can't just use a guaranteed convention. rdar://61911131
1 parent 081171c commit 8e2da8d

File tree

2 files changed

+88
-3
lines changed

2 files changed

+88
-3
lines changed

lib/SILOptimizer/Transforms/Outliner.cpp

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -742,6 +742,18 @@ void BridgedArgument::eraseFromParent() {
742742
BridgeFun->eraseFromParent();
743743
}
744744

745+
static ReleaseValueInst *findReleaseOf(SILValue releasedValue,
746+
SILBasicBlock::iterator from,
747+
SILBasicBlock::iterator to) {
748+
while (from != to) {
749+
auto release = dyn_cast<ReleaseValueInst>(&*from);
750+
if (release && release->getOperand() == releasedValue)
751+
return release;
752+
++from;
753+
}
754+
return nullptr;
755+
}
756+
745757
BridgedArgument BridgedArgument::match(unsigned ArgIdx, SILValue Arg,
746758
ApplyInst *AI) {
747759
// Match
@@ -782,8 +794,9 @@ BridgedArgument BridgedArgument::match(unsigned ArgIdx, SILValue Arg,
782794

783795
// Make sure that if we have a bridged value release that it is on the bridged
784796
// value.
785-
auto *BridgedValueRelease =
786-
dyn_cast<ReleaseValueInst>(std::next(SILBasicBlock::iterator(Enum)));
797+
auto *BridgedValueRelease = dyn_cast_or_null<ReleaseValueInst>(
798+
findReleaseOf(BridgedValue, std::next(SILBasicBlock::iterator(Enum)),
799+
SILBasicBlock::iterator(AI)));
787800
if (BridgedValueRelease && BridgedValueRelease->getOperand() != BridgedValue)
788801
return BridgedArgument();
789802

@@ -1232,6 +1245,7 @@ bool tryOutline(SILOptFunctionBuilder &FuncBuilder, SILFunction *Fun,
12321245
SmallPtrSet<SILBasicBlock *, 32> Visited;
12331246
SmallVector<SILBasicBlock *, 128> Worklist;
12341247
OutlinePatterns patterns(FuncBuilder);
1248+
bool changed = false;
12351249

12361250
// Traverse the function.
12371251
Worklist.push_back(&*Fun->begin());
@@ -1252,6 +1266,7 @@ bool tryOutline(SILOptFunctionBuilder &FuncBuilder, SILFunction *Fun,
12521266
FunctionsAdded.push_back(F);
12531267
CurInst = LastInst;
12541268
assert(LastInst->getParent() == CurBlock);
1269+
changed = true;
12551270
} else if (isa<TermInst>(CurInst)) {
12561271
std::copy(CurBlock->succ_begin(), CurBlock->succ_end(),
12571272
std::back_inserter(Worklist));
@@ -1261,7 +1276,7 @@ bool tryOutline(SILOptFunctionBuilder &FuncBuilder, SILFunction *Fun,
12611276
}
12621277
}
12631278
}
1264-
return false;
1279+
return changed;
12651280
}
12661281

12671282
namespace {

test/SILOptimizer/outliner.sil

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
// RUN: %target-sil-opt -outliner %s -enable-sil-verify-all | %FileCheck %s
2+
3+
// REQUIRES: OS=macosx
4+
5+
sil_stage canonical
6+
7+
import Builtin
8+
import Swift
9+
import Foundation
10+
11+
12+
@objc class MyObject {
13+
@objc static func take(arg: Data?) -> Data?
14+
}
15+
16+
sil @getData : $@convention(thin) () -> @owned Data
17+
sil @$s10Foundation4DataV19_bridgeToObjectiveCSo6NSDataCyF : $@convention(method) (@guaranteed Data) -> @owned NSData
18+
sil @$s10Foundation4DataV36_unconditionallyBridgeFromObjectiveCyACSo6NSDataCSgFZ : $@convention(method) (@guaranteed Optional<NSData>, @thin Data.Type) -> @owned Data
19+
20+
// We used to have a use-after release failure.
21+
22+
// CHECK-LABEL: sil [Osize] @test : $@convention(thin) (@owned MyObject) -> () {
23+
// CHECK: bb0([[ARG:%.*]] : $MyObject):
24+
// CHECK: [[META:%.*]] = metatype $@objc_metatype MyObject.Type
25+
// CHECK: [[FUN1:%.*]] = function_ref @getData : $@convention(thin) () -> @owned Data
26+
// CHECK: [[DATA:%.*]] = apply [[FUN1]]() : $@convention(thin) () -> @owned Data
27+
// CHECK: release_value [[ARG]] : $MyObject
28+
// CHECK: [[OUTLINED:%.*]] = function_ref @$s4main8MyObjectC4take3arg10Foundation4DataVSgAI_tFZToTembnb_ : $@convention(thin) (@owned Data, @objc_metatype MyObject.Type) -> @owned Optional<Data>
29+
// CHECK: [[RES:%.*]] = apply [[OUTLINED]]([[DATA]], [[META]]) : $@convention(thin) (@owned Data, @objc_metatype MyObject.Type) -> @owned Optional<Data>
30+
// CHECK: br bb1
31+
32+
// CHECK: bb1:
33+
// CHECK: release_value [[RES]]
34+
// CHECK: [[T:%.*]] = tuple ()
35+
// CHECK: return [[T]] : $()
36+
// CHECK: } // end sil function 'test'
37+
38+
sil [Osize] @test : $@convention(thin) (@owned MyObject) -> () {
39+
bb0(%0: $MyObject):
40+
%35 = metatype $@objc_metatype MyObject.Type
41+
%41 = function_ref @getData : $@convention(thin) () -> @owned Data
42+
%43 = apply %41() : $@convention(thin) () -> @owned Data
43+
%44 = function_ref @$s10Foundation4DataV19_bridgeToObjectiveCSo6NSDataCyF : $@convention(method) (@guaranteed Data) -> @owned NSData
44+
%45 = apply %44(%43) : $@convention(method) (@guaranteed Data) -> @owned NSData
45+
%46 = enum $Optional<NSData>, #Optional.some!enumelt, %45 : $NSData
46+
release_value %0 : $MyObject
47+
release_value %43 : $Data
48+
%50 = objc_method %35 : $@objc_metatype MyObject.Type, #MyObject.take!foreign : (MyObject.Type) -> (Data?) -> Data?, $@convention(objc_method) (Optional<NSData>, @objc_metatype MyObject.Type) -> @autoreleased Optional<NSData>
49+
%51 = apply %50(%46, %35) : $@convention(objc_method) (Optional<NSData>, @objc_metatype MyObject.Type) -> @autoreleased Optional<NSData>
50+
release_value %46 : $Optional<NSData>
51+
switch_enum %51 : $Optional<NSData>, case #Optional.some!enumelt: bb5, case #Optional.none!enumelt: bb6
52+
53+
bb5(%54 : $NSData):
54+
%55 = function_ref @$s10Foundation4DataV36_unconditionallyBridgeFromObjectiveCyACSo6NSDataCSgFZ : $@convention(method) (@guaranteed Optional<NSData>, @thin Data.Type) -> @owned Data
55+
%56 = enum $Optional<NSData>, #Optional.some!enumelt, %54 : $NSData
56+
%57 = metatype $@thin Data.Type
57+
%58 = apply %55(%56, %57) : $@convention(method) (@guaranteed Optional<NSData>, @thin Data.Type) -> @owned Data
58+
%59 = enum $Optional<Data>, #Optional.some!enumelt, %58 : $Data
59+
release_value %56 : $Optional<NSData>
60+
br bb7(%59 : $Optional<Data>)
61+
62+
bb6:
63+
%62 = enum $Optional<Data>, #Optional.none!enumelt
64+
br bb7(%62 : $Optional<Data>)
65+
66+
bb7(%64 : $Optional<Data>):
67+
release_value %64 : $Optional<Data>
68+
%102 = tuple ()
69+
return %102 : $()
70+
}

0 commit comments

Comments
 (0)