SILGen: Ease off +0 peepholes for load exprs.

Now that we more tightly close formal accesses on lvalues, having LoadExpr and friends try to return a +0 loaded value is unsafe without deeper analysis, since the access will be closed immediately after the load and potentially free temporary memory that might be the only thing keeping the borrowed object alive. Fixes rdar://problem/32730865.

Author: jckarter

lib/SILGen/SGFContext.h

@@ -152,9 +152,9 @@ class SGFContext {
   /// a value.  The Initialization is not okay to propagate down, but
   /// the +0/+1-ness is.
   SGFContext withFollowingProjection() const {
-    SGFContext copy;
-    copy.state.setInt(state.getInt());
-    return copy;
+    SGFContext result;
+    result.state.setInt(state.getInt());
+    return result;
   }
 };
 

lib/SILGen/SILGenExpr.cpp

@@ -1299,7 +1299,10 @@ RValue RValueEmitter::visitLoadExpr(LoadExpr *E, SGFContext C) {
   // Any writebacks here are tightly scoped.
   FormalEvaluationScope writeback(SGF);
   LValue lv = SGF.emitLValue(E->getSubExpr(), AccessKind::Read);
-  return SGF.emitLoadOfLValue(E, std::move(lv), C);
+  // We can't load at immediate +0 from the lvalue without deeper analysis,
+  // since the access will be immediately ended and might invalidate the value
+  // we loaded.
+  return SGF.emitLoadOfLValue(E, std::move(lv), C.withFollowingSideEffects());
 }
 
 SILValue SILGenFunction::emitTemporaryAllocation(SILLocation loc,
@@ -2367,7 +2370,9 @@ RValue RValueEmitter::visitMemberRefExpr(MemberRefExpr *E, SGFContext C) {
   FormalEvaluationScope scope(SGF);
 
   LValue lv = SGF.emitLValue(E, AccessKind::Read);
-  return SGF.emitLoadOfLValue(E, std::move(lv), C);
+  // We can't load at +0 without further analysis, since the formal access into
+  // the lvalue will end immediately.
+  return SGF.emitLoadOfLValue(E, std::move(lv), C.withFollowingSideEffects());
 }
 
 RValue RValueEmitter::visitDynamicMemberRefExpr(DynamicMemberRefExpr *E,
@@ -2386,7 +2391,9 @@ RValue RValueEmitter::visitSubscriptExpr(SubscriptExpr *E, SGFContext C) {
   FormalEvaluationScope scope(SGF);
 
   LValue lv = SGF.emitLValue(E, AccessKind::Read);
-  return SGF.emitLoadOfLValue(E, std::move(lv), C);
+  // We can't load at +0 without further analysis, since the formal access into
+  // the lvalue will end immediately.
+  return SGF.emitLoadOfLValue(E, std::move(lv), C.withFollowingSideEffects());
 }
 
 RValue RValueEmitter::visitDynamicSubscriptExpr(

lib/SILGen/SILGenLValue.cpp

@@ -3127,7 +3127,7 @@ RValue SILGenFunction::emitLoadOfLValue(SILLocation loc, LValue &&src,
   PathComponent &&component =
     drillToLastComponent(*this, loc, std::move(src), addr, AccessKind::Read);
 
-  // If the last component is physical, just drill down and load from it.
+  // If the last component is physical, drill down and load from it.
   if (component.isPhysical()) {
     addr = std::move(component.asPhysical())
              .offset(*this, loc, addr, AccessKind::Read);
@@ -3137,7 +3137,7 @@ RValue SILGenFunction::emitLoadOfLValue(SILLocation loc, LValue &&src,
                            isGuaranteedValid));
   }
 
-  // If the last component is logical, just emit a get.
+  // If the last component is logical, emit a get.
   return std::move(component.asLogical()).get(*this, loc, addr, C);
 }
 

test/SILGen/dynamic_lookup.swift

@@ -62,7 +62,7 @@ func direct_to_static_method(_ obj: AnyObject) {
   // CHECK: store [[ARG_COPY]] to [init] [[PBOBJ]] : $*AnyObject
   // CHECK: end_borrow [[BORROWED_ARG]] from [[ARG]]
   // CHECK: [[READ:%.*]] = begin_access [read] [unknown] [[PBOBJ]]
-  // CHECK-NEXT: [[OBJCOPY:%[0-9]+]] = load_borrow [[READ]] : $*AnyObject
+  // CHECK-NEXT: [[OBJCOPY:%[0-9]+]] = load [copy] [[READ]] : $*AnyObject
   // CHECK: end_access [[READ]]
   // CHECK-NEXT: [[OBJMETA:%[0-9]+]] = existential_metatype $@thick AnyObject.Type, [[OBJCOPY]] : $AnyObject
   // CHECK-NEXT: [[OPENMETA:%[0-9]+]] = open_existential_metatype [[OBJMETA]] : $@thick AnyObject.Type to $@thick (@opened([[UUID:".*"]]) AnyObject).Type
@@ -141,7 +141,7 @@ func opt_to_static_method(_ obj: AnyObject) {
   // CHECK:   [[OPTBOX:%[0-9]+]] = alloc_box ${ var Optional<@callee_owned () -> ()> }
   // CHECK:   [[PBO:%.*]] = project_box [[OPTBOX]]
   // CHECK:   [[READ:%.*]] = begin_access [read] [unknown] [[PBOBJ]]
-  // CHECK:   [[OBJCOPY:%[0-9]+]] = load_borrow [[READ]] : $*AnyObject
+  // CHECK:   [[OBJCOPY:%[0-9]+]] = load [copy] [[READ]] : $*AnyObject
   // CHECK:   [[OBJMETA:%[0-9]+]] = existential_metatype $@thick AnyObject.Type, [[OBJCOPY]] : $AnyObject
   // CHECK:   [[OPENMETA:%[0-9]+]] = open_existential_metatype [[OBJMETA]] : $@thick AnyObject.Type to $@thick (@opened
   // CHECK:   [[OBJCMETA:%[0-9]+]] = thick_to_objc_metatype [[OPENMETA]]

test/SILGen/functions.swift

@@ -176,7 +176,7 @@ func calls(_ i:Int, j:Int, k:Int) {
   // -- Curry the Type onto static method argument lists.
 
   // CHECK: [[READC:%.*]] = begin_access [read] [unknown] [[CADDR]]
-  // CHECK: [[C:%[0-9]+]] = load_borrow [[READC]]
+  // CHECK: [[C:%[0-9]+]] = load [copy] [[READC]]
   // CHECK: [[META:%.*]] = value_metatype $@thick SomeClass.Type, [[C]]
   // CHECK: [[METHOD:%[0-9]+]] = class_method [[META]] : {{.*}}, #SomeClass.static_method!1
   // CHECK: [[READI:%.*]] = begin_access [read] [unknown] [[IADDR]]

test/SILGen/load_from_lvalue_in_plus_zero_context.swift

@@ -0,0 +1,32 @@
+// RUN: %target-swift-frontend -emit-silgen %s | %FileCheck %s
+
+class A {
+  lazy var b: B = B()
+}
+
+final class B {
+  var c: C? {
+    get { return nil }
+    set {}
+  }
+}
+
+struct C {
+  let d: String
+}
+
+// CHECK-LABEL: sil hidden @{{.*}}test
+func test(a: A) {
+  let s: String?
+  // CHECK:   [[C_TEMP:%.*]] = alloc_stack $Optional<C>
+  // CHECK:   [[TAG:%.*]] = select_enum_addr [[C_TEMP]]
+  // CHECK:   cond_br [[TAG]], [[SOME:bb[0-9]+]], [[NONE:bb[0-9]+]]
+  // CHECK: [[SOME]]:
+  // CHECK:   [[C_PAYLOAD:%.*]] = unchecked_take_enum_data_addr [[C_TEMP]]
+  // -- This must be a copy, since we'll immediately destroy the value in the
+  //    temp buffer
+  // CHECK:   [[LOAD:%.*]] = load [copy] [[C_PAYLOAD]]
+  // CHECK:   destroy_addr [[C_TEMP]]
+  s = a.b.c?.d
+  print(s)
+}

test/SILGen/opaque_values_silgen.swift

@@ -1022,20 +1022,21 @@ func s480_________getError(someError: Error) -> Any {
 // CHECK-LABEL: sil private @_T020opaque_values_silgen21s490_______loadBorrowyyF3FooL_V3foo7ElementQzSg5IndexQz3pos_tF : $@convention(method) <Elements where Elements : Collection> (@in Elements.Index, @inout Foo<Elements>) -> @out Optional<Elements.Element> {
 // CHECK: bb0(%0 : $Elements.Index, %1 : $*Foo<Elements>):
 // CHECK: [[READ:%.*]] = begin_access [read] [unknown] %1 : $*Foo<Elements>
-// CHECK: [[LOAD:%.*]] = load_borrow [[READ]] : $*Foo<Elements>
+// CHECK: [[LOAD:%.*]] = load [copy] [[READ]] : $*Foo<Elements>
 // CHECK: end_access [[READ]] : $*Foo<Elements>
-// CHECK: [[EXTRACT:%.*]] = struct_extract [[LOAD]] : $Foo<Elements>, #<abstract function>Foo._elements
+// CHECK: [[BORROW_LOAD:%.*]] = begin_borrow [[LOAD]]
+// CHECK: [[EXTRACT:%.*]] = struct_extract [[BORROW_LOAD]] : $Foo<Elements>, #<abstract function>Foo._elements
 // CHECK: [[COPYELT:%.*]] = copy_value [[EXTRACT]] : $Elements
 // CHECK: [[BORROW:%.*]] = begin_borrow %0 : $Elements.Index
 // CHECK: [[COPYIDX:%.*]] = copy_value [[BORROW]] : $Elements.Index
 // CHECK: [[WT:%.*]] = witness_method $Elements, #Collection.subscript!getter.1 : <Self where Self : Collection> (Self) -> (Self.Index) -> Self.Element : $@convention(witness_method) <τ_0_0 where τ_0_0 : Collection> (@in τ_0_0.Index, @in_guaranteed τ_0_0) -> @out τ_0_0.Element
 // CHECK: %{{.*}} = apply [[WT]]<Elements>([[COPYIDX]], [[COPYELT]]) : $@convention(witness_method) <τ_0_0 where τ_0_0 : Collection> (@in τ_0_0.Index, @in_guaranteed τ_0_0) -> @out τ_0_0.Element
 // CHECK: destroy_value [[COPYELT]] : $Elements
-// CHECK: [[ENUM:%.*]] = enum $Optional<Elements.Element>, #Optional.some!enumelt.1, %12 : $Elements.Element
+// CHECK: [[ENUM:%.*]] = enum $Optional<Elements.Element>, #Optional.some!enumelt.1, %13 : $Elements.Element
 // CHECK: end_borrow [[BORROW]] from %0 : $Elements.Index, $Elements.Index
-// CHECK: end_borrow [[LOAD]] from [[READ]] : $Foo<Elements>, $*Foo<Elements>
+// CHECK: destroy_value [[LOAD]]
 // CHECK: destroy_value %0 : $Elements.Index
-// CHECK: return %14 : $Optional<Elements.Element>
+// CHECK: return %15 : $Optional<Elements.Element>
 // CHECK-LABEL: } // end sil function '_T020opaque_values_silgen21s490_______loadBorrowyyF3FooL_V3foo7ElementQzSg5IndexQz3pos_tF'
 
 func s490_______loadBorrow() {