Skip to content

Commit bf87de3

Browse files
eecksteineeckstein
committed
Fix a memory leak caused by the ReleaseDevirtualizer.
This occured if a stack-promoted object with a devirtualized final release is not actually allocated on the stack. Now the ReleaseDevirtualizer models the procedure of a final release more accurately. It inserts a set_deallocating instruction and calles the deallocator (instead of just the deinit). This changes also includes two peephole optimizations in IRGen and LLVMStackPromotion which get rid of unused runtime calls in case the stack promoted object is really allocated on the stack. This fixes rdar://problem/25068118
1 parent c1bcb0b commit bf87de3

File tree

6 files changed

+256
-35
lines changed

6 files changed

+256
-35
lines changed

‎lib/IRGen/IRGenSIL.cpp

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3614,15 +3614,27 @@ void IRGenSILFunction::visitDeallocRefInst(swift::DeallocRefInst *i) {
36143614
// Lower the operand.
36153615
Explosion self = getLoweredExplosion(i->getOperand());
36163616
auto selfValue = self.claimNext();
3617+
auto *ARI = dyn_cast<AllocRefInst>(i->getOperand());
36173618
if (!i->canAllocOnStack()) {
3619+
if (ARI && StackAllocs.count(ARI)) {
3620+
// We can ignore dealloc_refs (without [stack]) for stack allocated
3621+
// objects.
3622+
//
3623+
// %0 = alloc_ref [stack]
3624+
// ...
3625+
// dealloc_ref %0 // not needed (stems from the inlined deallocator)
3626+
// ...
3627+
// dealloc_ref [stack] %0
3628+
return;
3629+
}
3630+
36183631
auto classType = i->getOperand()->getType();
36193632
emitClassDeallocation(*this, classType, selfValue);
36203633
return;
36213634
}
36223635
// It's a dealloc_ref [stack]. Even if the alloc_ref did not allocate the
36233636
// object on the stack, we don't have to deallocate it, because it is
36243637
// deallocated in the final release.
3625-
auto *ARI = cast<AllocRefInst>(i->getOperand());
36263638
assert(ARI->canAllocOnStack());
36273639
if (StackAllocs.count(ARI)) {
36283640
if (IGM.Opts.EmitStackPromotionChecks) {

‎lib/LLVMPasses/LLVMStackPromotion.cpp

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,74 @@ int canPromote(CallInst *CI, unsigned &align, int maxSize) {
8484
return size;
8585
}
8686

87+
/// Remove redundant runtime calls for stack allocated buffers.
88+
/// If a buffer is allocated on the stack it's not needed to explicitly set
89+
/// the RC_DEALLOCATING_FLAG flag (except there is code which may depend on it).
90+
/// Also the a call to swift_deallocClassInstance (which stems from an inlined
91+
/// deallocator) is not needed.
92+
///
93+
/// %0 = alloca
94+
/// ...
95+
/// call @swift_setDeallocating(%0) // not needed
96+
/// // code which does not depend on the RC_DEALLOCATING_FLAG flag.
97+
/// call @swift_deallocClassInstance(%0) // not needed
98+
/// call @llvm.lifetime.end(%0)
99+
///
100+
static void removeRedundantRTCalls(CallInst *DeallocCall) {
101+
BasicBlock::iterator Iter(DeallocCall);
102+
BasicBlock::iterator Begin = DeallocCall->getParent()->begin();
103+
Value *Buffer = DeallocCall->getArgOperand(0);
104+
CallInst *RedundantDealloc = nullptr;
105+
CallInst *RedundantSetFlag = nullptr;
106+
SmallVector<Instruction *, 2> ToDelete;
107+
while (Iter != Begin) {
108+
--Iter;
109+
Instruction *I = &*Iter;
110+
if (auto *CI = dyn_cast<CallInst>(I)) {
111+
112+
// Check if we have a runtime function with the buffer as argument.
113+
if (CI->getNumArgOperands() < 1)
114+
break;
115+
if (CI->getArgOperand(0)->stripPointerCasts() != Buffer)
116+
break;
117+
auto *Callee = dyn_cast<Constant>(CI->getCalledValue());
118+
if (!Callee)
119+
break;
120+
121+
// The callee function my be a bitcast constant expression.
122+
if (auto *U = dyn_cast<ConstantExpr>(Callee)) {
123+
if (U->getOpcode() == Instruction::BitCast)
124+
Callee = U->getOperand(0);
125+
}
126+
auto *RTFunc = dyn_cast<Function>(Callee);
127+
if (!RTFunc)
128+
break;
129+
130+
if (RTFunc->getName() == "swift_setDeallocating") {
131+
assert(RedundantDealloc && "dealloc call must follow setDeallocating");
132+
assert(!RedundantSetFlag && "multiple setDeallocating calls");
133+
RedundantSetFlag = CI;
134+
continue;
135+
}
136+
if (RTFunc->getName() == "swift_deallocClassInstance") {
137+
assert(!RedundantSetFlag && "dealloc call must follow setDeallocating");
138+
assert(!RedundantDealloc && "multiple deallocClassInstance calls");
139+
RedundantDealloc = CI;
140+
continue;
141+
}
142+
break;
143+
}
144+
// Bail if we have an instruction which may read the RC_DEALLOCATING_FLAG
145+
// flag.
146+
if (I->mayReadFromMemory())
147+
break;
148+
}
149+
if (RedundantDealloc)
150+
RedundantDealloc->eraseFromParent();
151+
if (RedundantSetFlag)
152+
RedundantSetFlag->eraseFromParent();
153+
}
154+
87155
bool SwiftStackPromotion::runOnFunction(Function &F) {
88156

89157
bool Changed = false;
@@ -179,6 +247,9 @@ bool SwiftStackPromotion::runOnFunction(Function &F) {
179247
CallInst *Alloc = dyn_cast<CallInst>(CI->getArgOperand(0));
180248
assert(Alloc && "alloc buffer obfuscated");
181249
if (PromotedAllocs.count(Alloc)) {
250+
251+
removeRedundantRTCalls(CI);
252+
182253
IRBuilder<> B(CI);
183254
// This has two purposes:
184255
// 1. Tell LLVM the lifetime of the allocated stack memory.

‎lib/SILOptimizer/Transforms/ReleaseDevirtualizer.cpp

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ namespace {
3232
/// with
3333
/// %x = alloc_ref [stack] $X
3434
/// ...
35-
/// %d = function_ref @deinit_of_X
35+
/// set_deallocating %x
36+
/// %d = function_ref @dealloc_of_X
3637
/// %a = apply %d(%x)
3738
/// dealloc_ref [stack] %x
3839
///
@@ -61,9 +62,9 @@ class ReleaseDevirtualizer : public SILFunctionTransform {
6162
ApplyInst *DeallocCall);
6263

6364
/// Replace the release-instruction \p ReleaseInst with an explicit call to
64-
/// the destructor of \p AllocType for \p object.
65-
bool createDeinitCall(SILType AllocType, SILInstruction *ReleaseInst,
66-
SILValue object);
65+
/// the deallocating destructor of \p AllocType for \p object.
66+
bool createDeallocCall(SILType AllocType, SILInstruction *ReleaseInst,
67+
SILValue object);
6768

6869
StringRef getName() override { return "Release Devirtualizer"; }
6970

@@ -135,7 +136,7 @@ devirtualizeReleaseOfObject(SILInstruction *ReleaseInst,
135136
return false;
136137

137138
SILType AllocType = ARI->getType();
138-
return createDeinitCall(AllocType, ReleaseInst, ARI);
139+
return createDeallocCall(AllocType, ReleaseInst, ARI);
139140
}
140141

141142
bool ReleaseDevirtualizer::
@@ -180,46 +181,50 @@ devirtualizeReleaseOfBuffer(SILInstruction *ReleaseInst,
180181
return false;
181182

182183
SILType SILClType = SILType::getPrimitiveObjectType(CanType(ClType));
183-
return createDeinitCall(SILClType, ReleaseInst, AllocAI);
184+
return createDeallocCall(SILClType, ReleaseInst, AllocAI);
184185
}
185186

186-
bool ReleaseDevirtualizer::createDeinitCall(SILType AllocType,
187+
bool ReleaseDevirtualizer::createDeallocCall(SILType AllocType,
187188
SILInstruction *ReleaseInst,
188189
SILValue object) {
189-
DEBUG(llvm::dbgs() << " create deinit call\n");
190+
DEBUG(llvm::dbgs() << " create dealloc call\n");
190191

191192
ClassDecl *Cl = AllocType.getClassOrBoundGenericClass();
192193
assert(Cl && "no class type allocated with alloc_ref");
193194

194195
// Find the destructor of the type.
195196
DestructorDecl *Destructor = Cl->getDestructor();
196-
SILDeclRef DeinitRef(Destructor, SILDeclRef::Kind::Destroyer);
197+
SILDeclRef DeallocRef(Destructor, SILDeclRef::Kind::Deallocator);
197198
SILModule &M = ReleaseInst->getFunction()->getModule();
198-
SILFunction *Deinit = M.lookUpFunction(DeinitRef);
199-
if (!Deinit)
199+
SILFunction *Dealloc = M.lookUpFunction(DeallocRef);
200+
if (!Dealloc)
200201
return false;
201202

202-
CanSILFunctionType DeinitType = Deinit->getLoweredFunctionType();
203+
CanSILFunctionType DeallocType = Dealloc->getLoweredFunctionType();
203204
ArrayRef<Substitution> AllocSubsts = AllocType.gatherAllSubstitutions(M);
204205

205-
assert(!AllocSubsts.empty() == DeinitType->isPolymorphic() &&
206-
"deinit of generic class is not polymorphic or vice versa");
206+
assert(!AllocSubsts.empty() == DeallocType->isPolymorphic() &&
207+
"dealloc of generic class is not polymorphic or vice versa");
207208

208-
if (DeinitType->isPolymorphic())
209-
DeinitType = DeinitType->substGenericArgs(M, M.getSwiftModule(),
209+
if (DeallocType->isPolymorphic())
210+
DeallocType = DeallocType->substGenericArgs(M, M.getSwiftModule(),
210211
AllocSubsts);
211212

212-
SILType ReturnType = DeinitType->getSILResult();
213-
SILType DeinitSILType = SILType::getPrimitiveObjectType(DeinitType);
213+
SILType ReturnType = DeallocType->getSILResult();
214+
SILType DeallocSILType = SILType::getPrimitiveObjectType(DeallocType);
214215

215216
SILBuilder B(ReleaseInst);
216217
if (object->getType() != AllocType)
217218
object = B.createUncheckedRefCast(ReleaseInst->getLoc(), object, AllocType);
218219

220+
// Do what a release would do before calling the deallocator: set the object
221+
// in deallocating state, which means set the RC_DEALLOCATING_FLAG flag.
222+
B.createSetDeallocating(ReleaseInst->getLoc(), object);
223+
219224
// Create the call to the destructor with the allocated object as self
220225
// argument.
221-
auto *MI = B.createFunctionRef(ReleaseInst->getLoc(), Deinit);
222-
B.createApply(ReleaseInst->getLoc(), MI, DeinitSILType, ReturnType,
226+
auto *MI = B.createFunctionRef(ReleaseInst->getLoc(), Dealloc);
227+
B.createApply(ReleaseInst->getLoc(), MI, DeallocSILType, ReturnType,
223228
AllocSubsts, { object }, false);
224229

225230
NumReleasesDevirtualized++;

‎test/IRGen/class_stack_alloc.sil

Lines changed: 85 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-frontend -emit-stack-promotion-checks -stack-promotion-limit 48 -Onone -emit-ir %s | FileCheck %s
1+
// RUN: %target-swift-frontend -stack-promotion-limit 48 -Onone -emit-ir %s | FileCheck %s
22

33
import Builtin
44
import Swift
@@ -14,18 +14,30 @@ struct TestStruct {
1414
@sil_stored var c : Int64
1515
}
1616

17+
class BigClass {
18+
@sil_stored var a : Int64
19+
@sil_stored var b : Int64
20+
@sil_stored var c : Int64
21+
@sil_stored var d : Int64
22+
@sil_stored var e : Int64
23+
@sil_stored var f : Int64
24+
@sil_stored var g : Int64
25+
init()
26+
}
27+
1728
sil_vtable TestClass {}
29+
sil_vtable BigClass {}
1830

1931
// CHECK-LABEL: define{{( protected)?}} void @simple_promote
2032
// CHECK: %reference.raw = alloca %[[C:[a-zA-Z0-9_]+]], align 8
21-
// CHECK: [[M:%[0-9]+]] = call %swift.type* @_TMa[[C]]()
22-
// CHECK: [[O:%[0-9]+]] = bitcast %[[C]]* %reference.raw to %swift.refcounted*
23-
// CHECK: %reference.new = call %swift.refcounted* @swift_initStackObject(%swift.type* [[M]], %swift.refcounted* [[O]])
24-
// CHECK: [[R:%[0-9]+]] = bitcast %swift.refcounted* %reference.new to %[[C]]*
25-
// CHECK: call {{.*}} @rt_swift_release {{.*}} [[R]])
26-
// CHECK: [[O2:%[0-9]+]] = bitcast %[[C]]* [[R]] to %swift.refcounted*
27-
// CHECK: call void @swift_verifyEndOfLifetime(%swift.refcounted* [[O2]])
28-
// CHECK: ret void
33+
// CHECK-NEXT: [[M:%[0-9]+]] = call %swift.type* @_TMa[[C]]()
34+
// CHECK-NEXT: [[O:%[0-9]+]] = bitcast %[[C]]* %reference.raw to %swift.refcounted*
35+
// CHECK-NEXT: %reference.new = call %swift.refcounted* @swift_initStackObject(%swift.type* [[M]], %swift.refcounted* [[O]])
36+
// CHECK-NEXT: [[R:%[0-9]+]] = bitcast %swift.refcounted* %reference.new to %[[C]]*
37+
// CHECK-NEXT: call {{.*}} @rt_swift_release {{.*}} [[R]])
38+
// CHECK-NEXT: [[O2:%[0-9]+]] = bitcast %[[C]]* [[R]] to i8*
39+
// CHECK-NEXT: call void @llvm.lifetime.end(i64 -1, i8* [[O2]])
40+
// CHECK-NEXT: ret void
2941
sil @simple_promote : $@convention(thin) () -> () {
3042
bb0:
3143
%o1 = alloc_ref [stack] $TestClass
@@ -67,4 +79,68 @@ bb0:
6779
return %r : $()
6880
}
6981

82+
// CHECK-LABEL: define{{( protected)?}} void @promoted_with_devirtualized_release
83+
// CHECK: %reference.raw = alloca %[[C:[a-zA-Z0-9_]+]], align 8
84+
// CHECK-NEXT: [[M:%[0-9]+]] = call %swift.type* @_TMa[[C]]()
85+
// CHECK-NEXT: [[O:%[0-9]+]] = bitcast %[[C]]* %reference.raw to %swift.refcounted*
86+
// CHECK-NEXT: %reference.new = call %swift.refcounted* @swift_initStackObject(%swift.type* [[M]], %swift.refcounted* [[O]])
87+
// CHECK-NEXT: [[R:%[0-9]+]] = bitcast %swift.refcounted* %reference.new to %[[C]]*
88+
// CHECK-NEXT: call {{.*}} @swift_setDeallocating {{.*}}(%[[C]]* [[R]])
89+
// CHECK-NEXT: call void @not_inlined_destructor(%[[C]]* [[R]])
90+
// CHECK-NEXT: [[O2:%[0-9]+]] = bitcast %[[C]]* [[R]] to i8*
91+
// CHECK-NEXT: call void @llvm.lifetime.end(i64 -1, i8* [[O2]])
92+
// CHECK-NEXT: ret void
93+
sil @promoted_with_devirtualized_release : $@convention(thin) () -> () {
94+
bb0:
95+
%o1 = alloc_ref [stack] $TestClass
96+
set_deallocating %o1 : $TestClass
97+
%f = function_ref @not_inlined_destructor : $@convention(thin) (TestClass) -> ()
98+
%a = apply %f(%o1) : $@convention(thin) (TestClass) -> ()
99+
dealloc_ref [stack] %o1 : $TestClass
100+
101+
%r = tuple()
102+
return %r : $()
103+
}
104+
105+
// CHECK-LABEL: define{{( protected)?}} void @promoted_with_inlined_devirtualized_release
106+
// CHECK: %reference.raw = alloca %[[C:[a-zA-Z0-9_]+]], align 8
107+
// CHECK-NEXT: [[M:%[0-9]+]] = call %swift.type* @_TMa[[C]]()
108+
// CHECK-NEXT: [[O:%[0-9]+]] = bitcast %[[C]]* %reference.raw to %swift.refcounted*
109+
// CHECK-NEXT: %reference.new = call %swift.refcounted* @swift_initStackObject(%swift.type* [[M]], %swift.refcounted* [[O]])
110+
// CHECK-NEXT: [[R:%[0-9]+]] = bitcast %swift.refcounted* %reference.new to %[[C]]*
111+
// CHECK-NOT: call
112+
// CHECK: [[O2:%[0-9]+]] = bitcast %[[C]]* [[R]] to i8*
113+
// CHECK-NEXT: call void @llvm.lifetime.end(i64 -1, i8* [[O2]])
114+
// CHECK-NEXT: ret void
115+
sil @promoted_with_inlined_devirtualized_release : $@convention(thin) () -> () {
116+
bb0:
117+
%o1 = alloc_ref [stack] $TestClass
118+
set_deallocating %o1 : $TestClass
119+
dealloc_ref %o1 : $TestClass
120+
dealloc_ref [stack] %o1 : $TestClass
121+
122+
%r = tuple()
123+
return %r : $()
124+
}
125+
126+
// CHECK-LABEL: define{{( protected)?}} void @not_promoted_with_inlined_devirtualized_release
127+
// CHECK: [[O:%[0-9]+]] = call {{.*}} @rt_swift_allocObject
128+
// CHECK-NEXT: [[BC:%[0-9]+]] = bitcast %swift.refcounted* [[O]] to
129+
// CHECK-NEXT: call {{.*}} @swift_setDeallocating {{.*}}({{.*}} [[BC]])
130+
// CHECK-NEXT: [[BC2:%[0-9]+]] = bitcast {{.*}} [[BC]] to %swift.refcounted*
131+
// CHECK-NEXT: call void @swift_deallocClassInstance(%swift.refcounted* [[BC2]], {{.*}})
132+
// CHECK-NEXT: ret void
133+
sil @not_promoted_with_inlined_devirtualized_release : $@convention(thin) () -> () {
134+
bb0:
135+
%o1 = alloc_ref [stack] $BigClass
136+
set_deallocating %o1 : $BigClass
137+
dealloc_ref %o1 : $BigClass
138+
dealloc_ref [stack] %o1 : $BigClass
139+
140+
%r = tuple()
141+
return %r : $()
142+
}
143+
144+
sil @not_inlined_destructor : $@convention(thin) (TestClass) -> ()
70145
sil @unknown_func : $@convention(thin) (@inout TestStruct) -> ()
146+

‎test/LLVMPasses/stack_promotion.ll

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ target triple = "x86_64-apple-macosx10.9"
55

66
%swift.type = type { i64 }
77
%objc_object = type opaque
8+
%swift.refcounted = type { %swift.type*, i32, i32 }
9+
%BufferStorageType = type <{ %swift.refcounted }>
810

911
; CHECK-LABEL: define{{( protected)?}} void @promote_buffer()
1012
; CHECK: [[B:%.+]] = alloca i8, i32 48, align 8
@@ -22,6 +24,55 @@ entry:
2224
ret void
2325
}
2426

27+
; CHECK-LABEL: define{{( protected)?}} void @promote_buffer_with_devirtualized_release()
28+
; CHECK: [[B:%.+]] = alloca i8, i32 48, align 8
29+
; CHECK: [[M:%.+]] = call %swift.type* @get_buffer_metadata()
30+
; CHECK: [[BC:%.+]] = bitcast i8* [[B]] to %objc_object*
31+
; CHECK: [[I:%.+]] = call %objc_object* @swift_initStackObject(%swift.type* [[M]], %objc_object* [[BC]])
32+
; CHECK-NOT: call
33+
; CHECK: [[BC2:%.+]] = bitcast %objc_object* [[I]] to i8*
34+
; CHECK-NOT: call
35+
; CHECK: call void @llvm.lifetime.end(i64 -1, i8* [[BC2]])
36+
; CHECK-NOT: call
37+
; CHECK: ret void
38+
define void @promote_buffer_with_devirtualized_release() {
39+
entry:
40+
%0 = call %swift.type* @get_buffer_metadata()
41+
%1 = call %objc_object* @swift_bufferAllocateOnStack(%swift.type* %0, i64 48, i64 7)
42+
%2 = bitcast %objc_object* %1 to %BufferStorageType*
43+
call void bitcast (void (%swift.refcounted*)* @swift_setDeallocating to void (%BufferStorageType*)*)(%BufferStorageType* %2)
44+
%3 = bitcast %BufferStorageType* %2 to %swift.refcounted*
45+
call void @swift_deallocClassInstance(%swift.refcounted* %3, i64 48, i64 7)
46+
call void @swift_bufferDeallocateFromStack(%objc_object* %1)
47+
ret void
48+
}
49+
50+
; CHECK-LABEL: define{{( protected)?}} void @promote_buffer_with_devirtualized_release_and_non_trivial_deinit()
51+
; CHECK: [[B:%.+]] = alloca i8, i32 48, align 8
52+
; CHECK: [[M:%.+]] = call %swift.type* @get_buffer_metadata()
53+
; CHECK: [[BC:%.+]] = bitcast i8* [[B]] to %objc_object*
54+
; CHECK: [[I:%.+]] = call %objc_object* @swift_initStackObject(%swift.type* [[M]], %objc_object* [[BC]])
55+
; CHECK: [[BC2:%.+]] = bitcast %objc_object* [[I]] to %BufferStorageType
56+
; CHECK-NEXT: call {{.*}}@swift_setDeallocating {{.*}}({{.*}} [[BC2]])
57+
; CHECK-NEXT: call void @unknown_deinit(%BufferStorageType* [[BC2]])
58+
; CHECK-NOT: call
59+
; CHECK: [[BC3:%.+]] = bitcast %objc_object* [[I]] to i8*
60+
; CHECK-NEXT: call void @llvm.lifetime.end(i64 -1, i8* [[BC3]])
61+
; CHECK-NOT: call
62+
; CHECK: ret void
63+
define void @promote_buffer_with_devirtualized_release_and_non_trivial_deinit() {
64+
entry:
65+
%0 = call %swift.type* @get_buffer_metadata()
66+
%1 = call %objc_object* @swift_bufferAllocateOnStack(%swift.type* %0, i64 48, i64 7)
67+
%2 = bitcast %objc_object* %1 to %BufferStorageType*
68+
call void bitcast (void (%swift.refcounted*)* @swift_setDeallocating to void (%BufferStorageType*)*)(%BufferStorageType* %2)
69+
call void @unknown_deinit(%BufferStorageType* %2)
70+
%3 = bitcast %BufferStorageType* %2 to %swift.refcounted*
71+
call void @swift_deallocClassInstance(%swift.refcounted* %3, i64 48, i64 7)
72+
call void @swift_bufferDeallocateFromStack(%objc_object* %1)
73+
ret void
74+
}
75+
2576
; CHECK-LABEL: define{{( protected)?}} void @dont_promote_buffer_exceeding_limit()
2677
; CHECK: [[M:%.+]] = call %swift.type* @get_buffer_metadata()
2778
; CHECK: call %objc_object* @swift_bufferAllocate(%swift.type* [[M]], i64 48, i64 7)
@@ -38,3 +89,6 @@ entry:
3889
declare %swift.type* @get_buffer_metadata()
3990
declare %objc_object* @swift_bufferAllocateOnStack(%swift.type*, i64, i64)
4091
declare void @swift_bufferDeallocateFromStack(%objc_object*)
92+
declare void @swift_setDeallocating(%swift.refcounted*)
93+
declare void @swift_deallocClassInstance(%swift.refcounted*, i64, i64)
94+
declare void @unknown_deinit(%BufferStorageType*)

0 commit comments

Comments
 (0)