MemoryLifetimeVerifier: also verify locations with trivial types.

It helps to catch more problems

Author: eeckstein

include/swift/SIL/MemoryLocations.h

@@ -198,9 +198,13 @@ class MemoryLocations {
   /// init_existential_addr and open_existential_addr.
   bool handleNonTrivialProjections;
 
+  /// If true, also analyze trivial memory locations.
+  bool handleTrivialLocations;
+
 public:
-  MemoryLocations(bool handleNonTrivialProjections) :
-    handleNonTrivialProjections(handleNonTrivialProjections) {}
+  MemoryLocations(bool handleNonTrivialProjections, bool handleTrivialLocations) :
+    handleNonTrivialProjections(handleNonTrivialProjections),
+    handleTrivialLocations(handleTrivialLocations) {}
 
   MemoryLocations(const MemoryLocations &) = delete;
   MemoryLocations &operator=(const MemoryLocations &) = delete;
@@ -287,9 +291,6 @@ class MemoryLocations {
   /// Debug dump the MemoryLifetime internals.
   void dump() const;
 
-  /// Debug dump a bit set .
-  static void dumpBits(const Bits &bits);
-
 private:
   /// Clears all datastructures, except singleBlockLocations;
   void clear();

lib/SIL/Utils/MemoryLocations.cpp

@@ -212,7 +212,7 @@ void MemoryLocations::analyzeLocation(SILValue loc) {
   // Ignore trivial types to keep the number of locations small. Trivial types
   // are not interesting anyway, because such memory locations are not
   // destroyed.
-  if (loc->getType().isTrivial(*function))
+  if (!handleTrivialLocations && loc->getType().isTrivial(*function))
     return;
 
   if (isEmptyType(loc->getType(), function))

lib/SIL/Verifier/MemoryLifetimeVerifier.cpp

@@ -133,7 +133,8 @@ class MemoryLifetimeVerifier {
 
 public:
   MemoryLifetimeVerifier(SILFunction *function) :
-    function(function), locations(/*handleNonTrivialProjections*/ true) {}
+    function(function), locations(/*handleNonTrivialProjections*/ true,
+                                  /*handleTrivialLocations*/ true) {}
 
   /// The main entry point to verify the lifetime of all memory locations in
   /// the function.
@@ -593,7 +594,7 @@ void MemoryLifetimeVerifier::checkBlock(SILBasicBlock *block, Bits &bits) {
       case SILInstructionKind::InitExistentialAddrInst:
       case SILInstructionKind::InitEnumDataAddrInst: {
         SILValue addr = I.getOperand(0);
-        requireBitsClear(bits, addr, &I);
+        requireBitsClear(bits & nonTrivialLocations, addr, &I);
         requireNoStoreBorrowLocation(addr, &I);
         break;
       }
@@ -701,9 +702,17 @@ void MemoryLifetimeVerifier::checkFuncArgument(Bits &bits, Operand &argumentOp,
       break;
     case SILArgumentConvention::Indirect_In_Guaranteed:
     case SILArgumentConvention::Indirect_Inout:
-    case SILArgumentConvention::Indirect_InoutAliasable:
       requireBitsSet(bits, argumentOp.get(), applyInst);
       break;
+    case SILArgumentConvention::Indirect_InoutAliasable:
+      // We don't require any locations to be initialized for a partial_apply
+      // which takes an inout_aliasable argument. This is used for implicit
+      // closures (e.g. for the Bool '||' and '&&' operator arguments). Such
+      // closures capture the whole "self". When this is done in an initializer
+      // it can happen that not all fields of "self" are initialized, yet.
+      if (!isa<PartialApplyInst>(applyInst))
+        requireBitsSet(bits, argumentOp.get(), applyInst);
+      break;
     case SILArgumentConvention::Direct_Owned:
     case SILArgumentConvention::Direct_Unowned:
     case SILArgumentConvention::Direct_Guaranteed:

lib/SILOptimizer/Transforms/DestroyHoisting.cpp

@@ -127,7 +127,9 @@ class DestroyHoisting {
     function(function),
     // We currently don't handle enum and existential projections, because they
     // cannot be re-created easily. We could support this in future.
-    locations(/*handleNonTrivialProjections*/ false), DA(DA) {}
+    locations(/*handleNonTrivialProjections*/ false,
+              /*handleTrivialLocations*/ false),
+    DA(DA) {}
 
   bool hoistDestroys();
 };

test/Reflection/capture_descriptors.sil

@@ -71,10 +71,11 @@ bb0(%i : $*T, %b : @owned $<τ_0_0> { var τ_0_0 } <U>):
   return %12 : $()
 }
 
-sil [ossa] @concrete_caller2 : $@convention(thin) () -> @owned @callee_guaranteed () -> () {
-bb0:
+sil [ossa] @concrete_caller2 : $@convention(thin) (Int) -> @owned @callee_guaranteed () -> () {
+bb0(%0 : $Int):
   %f = function_ref @generic_callee2 : $@convention(thin) <T, U> (@in T, @owned <τ_0_0> { var τ_0_0 } <U>) -> ()
   %i = alloc_stack $Int
+  store %0 to [trivial] %i : $*Int
   %b = alloc_box $<τ_0_0> { var τ_0_0 } <String>
   %c = partial_apply [callee_guaranteed] %f<Int, String>(%i, %b) : $@convention(thin) <T, U> (@in T, @owned <τ_0_0> { var τ_0_0 } <U>) -> ()
   dealloc_stack %i : $*Int

test/SIL/memory_lifetime_failures.sil

@@ -525,3 +525,16 @@ bb0:
   return %5 : $()
 }
 
+sil @modify_bool : $@convention(thin) (@inout_aliasable Bool) -> ()
+
+// CHECK: SIL memory lifetime failure in @test_trivial_alloc_stack: memory is not initialized, but should
+sil [ossa] @test_trivial_alloc_stack : $@convention(thin) (Bool) -> () {
+bb0(%0 : $Bool):
+  %1 = alloc_stack $Bool
+  store %0 to [trivial] %1 : $*Bool
+  dealloc_stack %1 : $*Bool
+  %8 = function_ref @modify_bool : $@convention(thin) (@inout_aliasable Bool) -> ()
+  %9 = apply %8(%1) : $@convention(thin) (@inout_aliasable Bool) -> ()
+  %10 = tuple ()
+  return %10 : $()
+}

test/SILOptimizer/abcopts_ossa_guaranteed.sil

@@ -137,6 +137,7 @@ bb0(%0 : $*ArrayInt, %1 : $*ArrayInt):
   // Not redundant same index and array but append in between.
   %17 = function_ref @append : $@convention(method) (@in Int32, @inout ArrayInt) -> ()
   %18 = alloc_stack $Int32
+  store %i2 to [trivial] %18 : $*Int32
   %19 = apply %17(%18, %0) : $@convention(method) (@in Int32, @inout ArrayInt) -> ()
   %20 = apply %func(%i1, %102, %2) : $@convention(method) (Int32, Bool, @guaranteed ArrayInt) -> _DependenceToken
   // CHECK: apply [[CHECKBOUNDS]]([[IDX1]], {{.*}}[[LD1]]
@@ -351,6 +352,7 @@ bb2:
   %21 = tuple_extract %20 : $(Builtin.Int32, Builtin.Int1), 0
   %117 = function_ref @append : $@convention(method) (@in Int32, @inout ArrayInt) -> ()
   %118 = alloc_stack $Int32
+  store %0 to [trivial] %118 : $*Int32
   %119 = apply %117(%118, %24) : $@convention(method) (@in Int32, @inout ArrayInt) -> ()
   %120 = apply %52(%37, %101, %3) : $@convention(method) (Int32, Bool, @guaranteed ArrayInt) -> _DependenceToken
   dealloc_stack %118 : $*Int32

test/SILOptimizer/abcopts_ossa_owned.sil

@@ -143,6 +143,7 @@ bb0(%0 : $*ArrayInt, %1 : $*ArrayInt):
   // Not redundant same index and array but append in between.
   %17 = function_ref @append : $@convention(method) (@in Int32, @inout ArrayInt) -> ()
   %18 = alloc_stack $Int32
+  store %i2 to [trivial] %18 : $*Int32
   %19 = apply %17(%18, %0) : $@convention(method) (@in Int32, @inout ArrayInt) -> ()
   %copy2_10 = copy_value %2 : $ArrayInt
   %20 = apply %func(%i1, %102, %copy2_10) : $@convention(method) (Int32, Bool, @owned ArrayInt) -> _DependenceToken
@@ -377,6 +378,7 @@ bb2:
   %21 = tuple_extract %20 : $(Builtin.Int32, Builtin.Int1), 0
   %117 = function_ref @append : $@convention(method) (@in Int32, @inout ArrayInt) -> ()
   %118 = alloc_stack $Int32
+  store %0 to [trivial] %118 : $*Int32
   %119 = apply %117(%118, %24) : $@convention(method) (@in Int32, @inout ArrayInt) -> ()
   %121 = load [copy] %24 : $*ArrayInt
   %120 = apply %52(%37, %101, %121) : $@convention(method) (Int32, Bool, @owned ArrayInt) -> _DependenceToken

test/SILOptimizer/capture_promotion_generic_context.sil

@@ -96,6 +96,7 @@ bb1(%f : $@callee_guaranteed @substituted <A, B> (@in_guaranteed A) -> @out B fo
 
 bb2(%default : @guaranteed $E<(R<T>)->Int>):
   end_borrow %e : $E<(R<T>)->Int>
+  copy_addr %1 to %result : $*Int
   br exit
 
 exit:
@@ -113,7 +114,7 @@ exit:
 // CHECK-NEXT:    retain_value %3
 // CHECK-NEXT:    return [[CLOSURE]]
 // CHECK: } // end sil function 'call_generic_promotable_box_from_different_generic2'
-sil [ossa] @call_generic_promotable_box_from_different_generic2 : $@convention(thin) <T, U: P> (@in_guaranteed R<T>, @in_guaranteed E<(R<U>)->Int>, @in_guaranteed Int) -> @owned @callee_guaranteed (@in_guaranteed R<U>) -> @out Int {
+sil [ossa] @call_generic_promotable_box_from_different_generic2 : $@convention(thin) <T, U: P> (@in_guaranteed R<T>, @in_guaranteed E<(R<U>)->Int>, @in Int) -> @owned @callee_guaranteed (@in_guaranteed R<U>) -> @out Int {
 entry(%0 : $*R<T>, %1 : $*E<(R<U>)->Int>, %2 : $*Int):
   %f = function_ref @generic_promotable_box2 : $@convention(thin) <V> (@in_guaranteed R<V>, @in_guaranteed Int, @guaranteed <τ_0_0> { var E<(R<τ_0_0>)->Int> } <V>) -> @out Int
   %b = alloc_box $<τ_0_0> { var E<(R<τ_0_0>)->Int> } <U>

test/SILOptimizer/exclusivity_static_diagnostics.sil

@@ -527,7 +527,7 @@ bb0(%0 : $*(Int, Int)):
 // This only happens when mandatory passes are applied repeatedly.
 //
 // CHECK-LABEL: sil hidden [ossa] @inlinedEnumValue
-sil hidden [ossa] @inlinedEnumValue : $@convention(thin) (Int) -> (@out Optional<Int>, Int) {
+sil hidden [ossa] @inlinedEnumValue : $@convention(thin) (@in Optional<Int>, Int) -> Int {
 bb0(%0 : $*Optional<Int>, %1 : $Int):
   %6 = unchecked_take_enum_data_addr %0 : $*Optional<Int>, #Optional.some!enumelt
   %7 = begin_access [read] [static] %6 : $*Int