Skip to content

Commit dc8b1c2

Browse files
committed
[silgen] When pushing an RValue through a scope, use dealloc_box on any boxes we create instead of destroy_value.
It is only safe to perform a destroy_value on an alloc_box that contains an initialized value. We preserve the original cleanups for the value we are pushing through the scope implying that the box will not contain an initialized value when its lifetime ends (just like an alloc_stack). Thus we must use dealloc_box here. The surprising thing about tracking down this error is that I was not hitting any memory issues at -Onone. Instead what was happening was that at -O, we were miscompiling (creating non-dominating uses of SSA values) in the face of an address being taken twice. This does not seem to hit any SILGen tests today (I hit this when testing a patch I am trying to land today). rdar://31521023
1 parent c1e77bf commit dc8b1c2

File tree

3 files changed

+12
-6
lines changed

3 files changed

+12
-6
lines changed

lib/SILGen/SILGenApply.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3617,9 +3617,9 @@ class DeallocateUninitializedBox : public Cleanup {
36173617
};
36183618
} // end anonymous namespace
36193619

3620-
static CleanupHandle enterDeallocBoxCleanup(SILGenFunction &SGF, SILValue box) {
3621-
SGF.Cleanups.pushCleanup<DeallocateUninitializedBox>(box);
3622-
return SGF.Cleanups.getTopCleanup();
3620+
CleanupHandle SILGenFunction::enterDeallocBoxCleanup(SILValue box) {
3621+
Cleanups.pushCleanup<DeallocateUninitializedBox>(box);
3622+
return Cleanups.getTopCleanup();
36233623
}
36243624

36253625
/// This is an initialization for a box.
@@ -3706,7 +3706,7 @@ ManagedValue SILGenFunction::emitInjectEnum(SILLocation loc,
37063706

37073707
CleanupHandle initCleanup = enterDestroyCleanup(box);
37083708
Cleanups.setCleanupState(initCleanup, CleanupState::Dormant);
3709-
CleanupHandle uninitCleanup = enterDeallocBoxCleanup(*this, box);
3709+
CleanupHandle uninitCleanup = enterDeallocBoxCleanup(box);
37103710

37113711
BoxInitialization dest(box, addr, uninitCleanup, initCleanup);
37123712

lib/SILGen/SILGenFunction.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1668,6 +1668,8 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
16681668
CleanupHandle enterDormantTemporaryCleanup(SILValue temp,
16691669
const TypeLowering &tempTL);
16701670

1671+
CleanupHandle enterDeallocBoxCleanup(SILValue box);
1672+
16711673
/// Enter a currently-dormant cleanup to destroy the value in the
16721674
/// given address.
16731675
CleanupHandle

lib/SILGen/Scope.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,13 @@ RValue Scope::popPreservingValue(RValue &&rv) {
9898
pop();
9999

100100
// Then create cleanups for any lifetime extending boxes that we may have to
101-
// ensure that the boxes are cleaned up /after/ the value stored in the box.
101+
// ensure that the boxes are cleaned up /after/ the value stored in the
102+
// box. We assume that our values will be destroyed via a destroy_addr or the
103+
// like /before/ the end of our box's lifetime, implying that the value inside
104+
// the box should be uninitialized when the box is destroyed, so it is
105+
// important that we use a dealloc_box.
102106
for (auto v : lifetimeExtendingBoxes) {
103-
SGF.emitManagedRValueWithCleanup(v);
107+
SGF.enterDeallocBoxCleanup(v);
104108
}
105109

106110
// Reconstruct the managed values from the underlying sil values in the outer

0 commit comments

Comments
 (0)