Merge pull request #16962 from gottesmm/swift-4.2-branch-rdar40332620
[definite-init] Treat destroys of mark_uninitialized [var] container …
diff --git a/lib/SIL/SILVerifier.cpp b/lib/SIL/SILVerifier.cpp
index c044f2b..2bf1250 100644
--- a/lib/SIL/SILVerifier.cpp
+++ b/lib/SIL/SILVerifier.cpp
@@ -1749,18 +1749,34 @@
require(MU->getModule().getStage() == SILStage::Raw,
"mark_uninitialized instruction can only exist in raw SIL");
require(Src->getType().isAddress() ||
- Src->getType()
- .getSwiftRValueType()
- ->getClassOrBoundGenericClass() ||
- Src->getType().getAs<SILBoxType>(),
+ Src->getType().getClassOrBoundGenericClass() ||
+ Src->getType().getAs<SILBoxType>(),
"mark_uninitialized must be an address, class, or box type");
require(Src->getType() == MU->getType(),"operand and result type mismatch");
// FIXME: When the work to force MUI to be on Allocations/SILArguments
// complete, turn on this assertion.
-#if 0
- require(isa<AllocationInst>(Src) || isa<SILArgument>(Src),
- "Mark Uninitialized should always be on the storage location");
-#endif
+ require(isa<AllocationInst>(Src)
+ || isa<GlobalAddrInst>(Src)
+ // TODO: Should we support SILUndef on mark_uninitialized? We
+ // currently have a test that verifies this behavior, but it seems
+ // like this would always be a bug due to the implications around
+ // the code in DI. This just bakes in the current behavior.
+ || isa<SILUndef>(Src)
+ // We allow SILArguments to support the case of initializing
+ // initializers. In such a case, the underlying case is allocated
+ // outside by the allocating initializer and we pass in the to be
+ // initialized value as a SILArgument.
+ || isa<SILArgument>(Src)
+ // FIXME: Once the MarkUninitializedFixup pass is eliminated,
+ // mark_uninitialized should never be applied to a project_box. So
+ // at that point, this should be eliminated.
+ || isa<ProjectBoxInst>(Src)
+ // FIXME: We only support pointer to address here to not break LLDB. It is
+ // important that long term we get rid of this since this is a situation
+ // where LLDB is breaking SILGen/DI invariants by not creating a new
+ // independent stack location for the pointer to address.
+ || isa<PointerToAddressInst>(Src),
+ "Mark Uninitialized must be applied to a storage location");
}
void checkMarkUninitializedBehaviorInst(MarkUninitializedBehaviorInst *MU) {
diff --git a/lib/SILOptimizer/Mandatory/DIMemoryUseCollectorOwnership.cpp b/lib/SILOptimizer/Mandatory/DIMemoryUseCollectorOwnership.cpp
index abd4e44..94e35c6 100644
--- a/lib/SILOptimizer/Mandatory/DIMemoryUseCollectorOwnership.cpp
+++ b/lib/SILOptimizer/Mandatory/DIMemoryUseCollectorOwnership.cpp
@@ -28,7 +28,42 @@
using namespace ownership;
//===----------------------------------------------------------------------===//
-// DIMemoryObjectInfo Implementation
+// Utility
+//===----------------------------------------------------------------------===//
+
+static void gatherDestroysOfContainer(const MarkUninitializedInst *MUI,
+ DIElementUseInfo &UseInfo) {
+ // The MUI must be used on an alloc_box, alloc_stack, or global_addr. If we
+ // have an alloc_stack or a global_addr, there is nothing further to do.
+ if (isa<AllocStackInst>(MUI->getOperand()) ||
+ isa<GlobalAddrInst>(MUI->getOperand()) ||
+ isa<SILArgument>(MUI->getOperand()) ||
+ // FIXME: We only support pointer to address here to not break LLDB. It is
+ // important that long term we get rid of this since this is a situation
+ // where LLDB is breaking SILGen/DI invariants by not creating a new
+ // independent stack location for the pointer to address.
+ isa<PointerToAddressInst>(MUI->getOperand()))
+ return;
+
+ // Otherwise, we assume that we have a project_box. This is a hard cast to
+ // ensure that we catch any new patterns emitted by SILGen and assert.
+ auto *PBI = cast<ProjectBoxInst>(MUI->getOperand());
+ auto *ABI = cast<AllocBoxInst>(PBI->getOperand());
+
+ // Treat destroys of the container as load+destroys of the original value.
+ //
+ // TODO: This should really be tracked separately from other destroys so that
+ // we distinguish the lifetime of the container from the value itself.
+ for (auto *Op : ABI->getUses()) {
+ SILInstruction *User = Op->getUser();
+ if (isa<StrongReleaseInst>(User) || isa<DestroyValueInst>(User)) {
+ UseInfo.trackDestroy(User);
+ }
+ }
+}
+
+//===----------------------------------------------------------------------===//
+// DIMemoryObjectInfo Implementation
//===----------------------------------------------------------------------===//
static unsigned getElementCountRec(SILModule &Module, SILType T,
@@ -511,6 +546,7 @@
}
collectUses(TheMemory.MemoryInst, 0);
+ gatherDestroysOfContainer(TheMemory.MemoryInst, UseInfo);
}
void trackUse(DIMemoryUse Use) { UseInfo.trackUse(Use); }
@@ -1610,20 +1646,8 @@
assert(StoresOfArgumentToSelf == 1 &&
"The 'self' argument should have been stored into the box exactly once");
- // The MUI must be used on an alloc_box or alloc_stack instruction. If we have
- // an alloc_stack, there is nothing further to do.
- if (isa<AllocStackInst>(MUI->getOperand()))
- return;
-
- auto *PBI = cast<ProjectBoxInst>(MUI->getOperand());
- auto *ABI = cast<AllocBoxInst>(PBI->getOperand());
-
- for (auto Op : ABI->getUses()) {
- SILInstruction *User = Op->getUser();
- if (isa<StrongReleaseInst>(User) || isa<DestroyValueInst>(User)) {
- UseInfo.trackDestroy(User);
- }
- }
+ // Gather the uses of the
+ gatherDestroysOfContainer(MUI, UseInfo);
}
void DelegatingClassInitElementUseCollector::
diff --git a/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp b/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp
index defb78b..32c4bb6 100644
--- a/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp
+++ b/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp
@@ -496,6 +496,10 @@
void processUninitializedRelease(SILInstruction *Release,
bool consumed,
SILBasicBlock::iterator InsertPt);
+ void processUninitializedReleaseOfBox(AllocBoxInst *ABI,
+ SILInstruction *Release,
+ bool consumed,
+ SILBasicBlock::iterator InsertPt);
void deleteDeadRelease(unsigned ReleaseID);
void processNonTrivialRelease(unsigned ReleaseID);
@@ -1857,56 +1861,68 @@
assert(isa<StoreInst>(Inst) && "Unknown store instruction!");
}
+void LifetimeChecker::processUninitializedReleaseOfBox(
+ AllocBoxInst *ABI, SILInstruction *Release, bool consumed,
+ SILBasicBlock::iterator InsertPt) {
+ assert(ABI == Release->getOperand(0));
+ SILBuilderWithScope B(Release);
+ B.setInsertionPoint(InsertPt);
+ Destroys.push_back(B.createDeallocBox(Release->getLoc(), ABI));
+}
+
void LifetimeChecker::processUninitializedRelease(SILInstruction *Release,
bool consumed,
SILBasicBlock::iterator InsertPt) {
// If this is an early release of a class instance, we need to emit a
// dealloc_partial_ref to free the memory. If this is a derived class, we
// may have to do a load of the 'self' box to get the class reference.
- if (TheMemory.isClassInitSelf()) {
- auto Loc = Release->getLoc();
-
- SILBuilderWithScope B(Release);
- B.setInsertionPoint(InsertPt);
-
- SILValue Pointer = Release->getOperand(0);
-
- // If we see an alloc_box as the pointer, then we're deallocating a 'box'
- // for self. Make sure we're using its address result, not its refcount
- // result, and make sure that the box gets deallocated (not released)
- // since the pointer it contains will be manually cleaned up.
- auto *ABI = dyn_cast<AllocBoxInst>(Release->getOperand(0));
- if (ABI)
- Pointer = getOrCreateProjectBox(ABI, 0);
-
- if (!consumed) {
- if (Pointer->getType().isAddress())
- Pointer = B.createLoad(Loc, Pointer, LoadOwnershipQualifier::Take);
-
- auto MetatypeTy = CanMetatypeType::get(
- TheMemory.MemorySILType.getSwiftRValueType(),
- MetatypeRepresentation::Thick);
- auto SILMetatypeTy = SILType::getPrimitiveObjectType(MetatypeTy);
- SILValue Metatype;
-
- // In an inherited convenience initializer, we must use the dynamic
- // type of the object since nothing is initialized yet.
- if (TheMemory.isDelegatingInit())
- Metatype = B.createValueMetatype(Loc, SILMetatypeTy, Pointer);
- else
- Metatype = B.createMetatype(Loc, SILMetatypeTy);
-
- // We've already destroyed any instance variables initialized by this
- // constructor, now destroy instance variables initialized by subclass
- // constructors that delegated to us, and finally free the memory.
- B.createDeallocPartialRef(Loc, Pointer, Metatype);
+ if (!TheMemory.isClassInitSelf()) {
+ if (auto *ABI = dyn_cast<AllocBoxInst>(Release->getOperand(0))) {
+ return processUninitializedReleaseOfBox(ABI, Release, consumed, InsertPt);
}
-
- // dealloc_box the self box if necessary.
- if (ABI) {
- auto DB = B.createDeallocBox(Loc, ABI);
- Destroys.push_back(DB);
- }
+ return;
+ }
+
+ auto Loc = Release->getLoc();
+
+ SILBuilderWithScope B(Release);
+ B.setInsertionPoint(InsertPt);
+
+ SILValue Pointer = Release->getOperand(0);
+
+ // If we see an alloc_box as the pointer, then we're deallocating a 'box' for
+ // self. Make sure that the box gets deallocated (not released) since the
+ // pointer it contains will be manually cleaned up.
+ auto *ABI = dyn_cast<AllocBoxInst>(Release->getOperand(0));
+ if (ABI)
+ Pointer = getOrCreateProjectBox(ABI, 0);
+
+ if (!consumed) {
+ if (Pointer->getType().isAddress())
+ Pointer = B.createLoad(Loc, Pointer, LoadOwnershipQualifier::Take);
+
+ auto MetatypeTy = CanMetatypeType::get(TheMemory.MemorySILType.getASTType(),
+ MetatypeRepresentation::Thick);
+ auto SILMetatypeTy = SILType::getPrimitiveObjectType(MetatypeTy);
+ SILValue Metatype;
+
+ // In an inherited convenience initializer, we must use the dynamic
+ // type of the object since nothing is initialized yet.
+ if (TheMemory.isDelegatingInit())
+ Metatype = B.createValueMetatype(Loc, SILMetatypeTy, Pointer);
+ else
+ Metatype = B.createMetatype(Loc, SILMetatypeTy);
+
+ // We've already destroyed any instance variables initialized by this
+ // constructor, now destroy instance variables initialized by subclass
+ // constructors that delegated to us, and finally free the memory.
+ B.createDeallocPartialRef(Loc, Pointer, Metatype);
+ }
+
+ // dealloc_box the self box if necessary.
+ if (ABI) {
+ auto DB = B.createDeallocBox(Loc, ABI);
+ Destroys.push_back(DB);
}
}
diff --git a/test/SILOptimizer/definite_init_markuninitialized_var.sil b/test/SILOptimizer/definite_init_markuninitialized_var.sil
index f4958e1..f927a84 100644
--- a/test/SILOptimizer/definite_init_markuninitialized_var.sil
+++ b/test/SILOptimizer/definite_init_markuninitialized_var.sil
@@ -542,3 +542,76 @@
%23 = tuple () // user: %24
return %23 : $() // id: %24
}
+// CHECK: } // end sil function 'conditionalInitOrAssign'
+
+// Make sure that we use a dealloc_box on the path where we do not assign.
+//
+// CHECK-LABEL: sil @conditionalInitOrAssignAllocBox : $@convention(thin) () -> () {
+// CHECK: bb0:
+// CHECK: [[BOX:%.*]] = alloc_box
+//
+// CHECK: bb1:
+// CHECK: destroy_value [[BOX]]
+// CHECK: br bb3
+//
+// CHECK: bb2:
+// CHECK: dealloc_box [[BOX]]
+// CHECK: br bb3
+//
+// CHECK: bb3:
+// CHECK-NEXT: tuple
+// CHECK-NEXT: return
+// CHECK: } // end sil function 'conditionalInitOrAssignAllocBox'
+sil @conditionalInitOrAssignAllocBox : $@convention(thin) () -> () {
+bb0:
+ %box = alloc_box $<τ_0_0> { var τ_0_0 } <SomeClass>
+ %5 = project_box %box : $<τ_0_0> { var τ_0_0 } <SomeClass>, 0
+ %7 = mark_uninitialized [var] %5 : $*SomeClass
+ %2 = function_ref @getSomeClass : $@convention(thin) () -> @owned SomeClass
+ cond_br undef, bb1, bb2
+
+// Value is stored into box and then destroyed.
+bb1:
+ %12 = apply %2() : $@convention(thin) () -> @owned SomeClass
+ assign %12 to %7 : $*SomeClass
+ destroy_value %box : $<τ_0_0> { var τ_0_0 } <SomeClass>
+ br bb3
+
+bb2:
+ destroy_value %box : $<τ_0_0> { var τ_0_0 } <SomeClass>
+ br bb3
+
+bb3:
+ %9999 = tuple()
+ return %9999 : $()
+}
+
+// LLDB uses mark_uninitialized [var] in an unsupported way via an address to
+// pointer. LLDB shouldn't do this, but until it is removed and validated we
+// need a test for this.
+// CHECK-LABEL: sil @lldb_unsupported_markuninitialized_testcase : $@convention(thin) (UnsafeMutablePointer<Any>) -> () {
+// CHECK-NOT: mark_uninitialized [var]
+// CHECK: } // end sil function 'lldb_unsupported_markuninitialized_testcase'
+sil @lldb_unsupported_markuninitialized_testcase : $@convention(thin) (UnsafeMutablePointer<Any>) -> () {
+bb0(%0 : @trivial $UnsafeMutablePointer<Any>):
+ %2 = struct_extract %0 : $UnsafeMutablePointer<Any>, #UnsafeMutablePointer._rawValue
+ %3 = integer_literal $Builtin.Int64, 8
+ %4 = index_raw_pointer %2 : $Builtin.RawPointer, %3 : $Builtin.Int64
+ %5 = pointer_to_address %4 : $Builtin.RawPointer to [strict] $*Builtin.RawPointer
+ %6 = load [trivial] %5 : $*Builtin.RawPointer
+ %7 = pointer_to_address %6 : $Builtin.RawPointer to [strict] $*Error
+ %8 = mark_uninitialized [var] %7 : $*Error
+ %9 = struct_extract %0 : $UnsafeMutablePointer<Any>, #UnsafeMutablePointer._rawValue
+ %10 = integer_literal $Builtin.Int64, 0
+ %11 = index_raw_pointer %9 : $Builtin.RawPointer, %10 : $Builtin.Int64
+ %12 = pointer_to_address %11 : $Builtin.RawPointer to [strict] $*Builtin.RawPointer
+ %13 = load [trivial] %12 : $*Builtin.RawPointer
+ %14 = pointer_to_address %13 : $Builtin.RawPointer to [strict] $*@thick SomeClass.Type
+ %15 = mark_uninitialized [var] %14 : $*@thick SomeClass.Type
+ %16 = metatype $@thick SomeClass.Type
+ %17 = begin_access [modify] [unsafe] %15 : $*@thick SomeClass.Type
+ assign %16 to %17 : $*@thick SomeClass.Type
+ end_access %17 : $*@thick SomeClass.Type
+ %9999 = tuple()
+ return %9999 : $()
+}
