Skip to content

Commit 32a8dbc

Browse files
committed
Fix a dangling stack pointer in SILCombine.
The bug was introduced recently: commit ac8a48b Date: Fri Oct 6 12:34:14 2017 -0700 Fixes <rdar://35800315> (crash using freed OpenedArchetypesTracker).
1 parent 708aadf commit 32a8dbc

File tree

2 files changed

+70
-12
lines changed

2 files changed

+70
-12
lines changed

lib/SILOptimizer/SILCombiner/SILCombinerApplyVisitors.cpp

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -943,6 +943,29 @@ static bool isAddressInitializedAtCall(SILValue addr, SILInstruction *AI,
943943
return true;
944944
}
945945

946+
/// Scoped registration of opened archetypes.
947+
class RAIIOpenedArchetypesTracker {
948+
SILBuilder &B;
949+
// The original tracker may be null.
950+
SILOpenedArchetypesTracker *OldOpenedArchetypesTracker;
951+
SILOpenedArchetypesTracker OpenedArchetypesTracker;
952+
953+
public:
954+
RAIIOpenedArchetypesTracker(SILBuilder &B)
955+
: B(B), OldOpenedArchetypesTracker(B.getOpenedArchetypesTracker()),
956+
OpenedArchetypesTracker(&B.getFunction()) {
957+
B.setOpenedArchetypesTracker(&OpenedArchetypesTracker);
958+
}
959+
960+
SILOpenedArchetypesTracker &getTracker() {
961+
return OpenedArchetypesTracker;
962+
}
963+
964+
~RAIIOpenedArchetypesTracker() {
965+
B.setOpenedArchetypesTracker(OldOpenedArchetypesTracker);
966+
}
967+
};
968+
946969
/// Propagate information about a concrete type from init_existential_addr
947970
/// or init_existential_ref into witness_method conformances and into
948971
/// apply instructions.
@@ -974,19 +997,14 @@ SILInstruction *SILCombiner::propagateConcreteTypeOfInitExistential(
974997
if (!CCT.isValid())
975998
return nullptr;
976999

977-
SILOpenedArchetypesTracker *OldOpenedArchetypesTracker =
978-
Builder.getOpenedArchetypesTracker();
979-
980-
SILOpenedArchetypesTracker OpenedArchetypesTracker(Apply.getFunction());
981-
1000+
RAIIOpenedArchetypesTracker tempTracker(Builder);
9821001
if (CCT.ConcreteType->isOpenedExistential()) {
9831002
// Temporarily record this opened existential def. Don't permanently record
9841003
// in the Builder's tracker because this opened existential's original
9851004
// dominating def may not have been recorded yet.
9861005
// FIXME: Redesign the tracker. This is not robust.
987-
OpenedArchetypesTracker.addOpenedArchetypeDef(
1006+
tempTracker.getTracker().addOpenedArchetypeDef(
9881007
cast<ArchetypeType>(CCT.ConcreteType), CCT.ConcreteTypeDef);
989-
Builder.setOpenedArchetypesTracker(&OpenedArchetypesTracker);
9901008
}
9911009

9921010
// Propagate the concrete type into the callee-operand if required.
@@ -1006,8 +1024,6 @@ SILInstruction *SILCombiner::propagateConcreteTypeOfInitExistential(
10061024
cast<InitExistentialAddrInst>(InitExistential)->getOperand();
10071025
return isAddressInitializedAtCall(existentialAddr, AI, DT);
10081026
};
1009-
1010-
// FIXME: Intentionally preserve this bug to avoid changing functionality.
10111027
if (isCopied && !canReplaceCopiedSelf())
10121028
return nullptr;
10131029

@@ -1017,9 +1033,6 @@ SILInstruction *SILCombiner::propagateConcreteTypeOfInitExistential(
10171033
Apply, CCT.NewSelf, Self, CCT.ConcreteType, CCT.ConcreteTypeDef,
10181034
CCT.getConformance(), OpenedArchetype);
10191035

1020-
if (CCT.ConcreteType->isOpenedExistential())
1021-
Builder.setOpenedArchetypesTracker(OldOpenedArchetypesTracker);
1022-
10231036
return NewAI;
10241037
}
10251038

test/SILOptimizer/existential_type_propagation.sil

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,51 @@ bb0(%0 : $XXX):
293293
return %11 : $()
294294
}
295295

296+
@objc protocol Q : class {
297+
@objc func bar()
298+
}
299+
300+
// rdar://35800315: crash in SILBuilder::addOpenedArchetypeOperands.
301+
//
302+
// When SILCombineApply propagates concrete types it temporarily installed an
303+
// opened archetype tracker to rewrite witness_method. It may then fail because
304+
// the copied existential is destroyed before the method is applied leaving a
305+
// dangling stack pointer. If silcombine continues iterating it may hit an
306+
// objc_method, which does a lookup in the archetype tracker.
307+
sil @silcombine_apply_opened_archetype : $@convention(thin) (@in P, Q) -> () {
308+
bb0(%0 : $*P, %1 : $Q):
309+
// A single operand instruction that uses an opened existential.
310+
// For some reason this is handled specialy in
311+
// SILBuilder::addOpenedArchetypeOperands.
312+
%ref = open_existential_ref %1 : $Q to $@opened("D66B9398-D800-11E7-8432-0A0000BBF6C0") Q
313+
%om = objc_method %ref : $@opened("D66B9398-D800-11E7-8432-0A0000BBF6C0") Q, #Q.bar!1.foreign : <Self where Self : Q> (Self) -> () -> (), $@convention(objc_method) <τ_0_0 where τ_0_0 : Q> (τ_0_0) -> ()
314+
%call1 = apply %om<@opened("D66B9398-D800-11E7-8432-0A0000BBF6C0") Q>(%ref) : $@convention(objc_method) <τ_0_0 where τ_0_0 : Q> (τ_0_0) -> ()
315+
316+
// Some code to simplify in iteration #1
317+
%ptr = address_to_pointer %0 : $*P to $Builtin.RawPointer
318+
%adr = pointer_to_address %ptr : $Builtin.RawPointer to $*P
319+
320+
// Some code to confuse SILCombineApply.
321+
%p0 = alloc_stack $P
322+
%opened = open_existential_addr immutable_access %adr : $*P to $*@opened("A66B9398-D800-11E7-8432-0A0000BBF6C0") P
323+
%p1 = alloc_stack $P
324+
%v1 = init_existential_addr %p1 : $*P, $@opened("A66B9398-D800-11E7-8432-0A0000BBF6C0") P
325+
copy_addr %opened to [initialization] %v1 : $*@opened("A66B9398-D800-11E7-8432-0A0000BBF6C0") P
326+
copy_addr [take] %p1 to [initialization] %p0 : $*P
327+
destroy_addr %0 : $*P
328+
%v0 = open_existential_addr immutable_access %p0 : $*P to $*@opened("B66B9398-D800-11E7-8432-0A0000BBF6C0") P
329+
%p2 = alloc_stack $@opened("B66B9398-D800-11E7-8432-0A0000BBF6C0") P
330+
copy_addr %v0 to [initialization] %p2 : $*@opened("B66B9398-D800-11E7-8432-0A0000BBF6C0") P
331+
%wm = witness_method $@opened("B66B9398-D800-11E7-8432-0A0000BBF6C0") P, #P.foo : <T where T : P> (T) -> () -> Int64, %opened : $*@opened("A66B9398-D800-11E7-8432-0A0000BBF6C0") P : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int64
332+
%call2 = apply %wm<@opened("B66B9398-D800-11E7-8432-0A0000BBF6C0") P>(%p2) : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> Int64
333+
destroy_addr %p2 : $*@opened("B66B9398-D800-11E7-8432-0A0000BBF6C0") P
334+
dealloc_stack %p2 : $*@opened("B66B9398-D800-11E7-8432-0A0000BBF6C0") P
335+
dealloc_stack %p1 : $*P
336+
dealloc_stack %p0 : $*P
337+
%r = tuple ()
338+
return %r : $()
339+
}
340+
296341
sil @_TTWC4nix23XXXS_3PPPS_FS1_3foofT_T_ : $@convention(witness_method: PPP) (@guaranteed XXX) -> ()
297342

298343
sil_witness_table XXX: PPP module nix2 {

0 commit comments

Comments
 (0)