Guard the lifetime of StringObject.nativeStorage in mutating methods.

lib/SIL/Utils/MemAccessUtils.cpp

@@ -420,7 +420,9 @@ static bool mayAccessPointer(SILInstruction *instruction) {
 static bool mayLoadWeakOrUnowned(SILInstruction *instruction) {
   // TODO: It is possible to do better here by looking at the address that is
   //       being loaded.
-  return isa<LoadWeakInst>(instruction) || isa<LoadUnownedInst>(instruction);
+  return isa<LoadWeakInst>(instruction) || isa<LoadUnownedInst>(instruction)
+    || isa<StrongCopyUnownedValueInst>(instruction)
+    || isa<StrongCopyUnmanagedValueInst>(instruction);
 }
 
 bool swift::isDeinitBarrier(SILInstruction *instruction) {

lib/SILOptimizer/PassManager/PassPipeline.cpp

@@ -153,8 +153,12 @@ static void addMandatoryDiagnosticOptPipeline(SILPassPipelinePlan &P) {
     P.addSILSkippingChecker();
 #endif
 
-  if (Options.shouldOptimize() && EnableDestroyHoisting) {
-    P.addDestroyHoisting();
+  if (Options.shouldOptimize()) {
+    if (EnableDestroyHoisting) {
+      P.addDestroyHoisting();
+    } else {
+      P.addSSADestroyHoisting();
+    }
   }
   P.addMandatoryInlining();
   P.addMandatorySILLinker();

stdlib/cmake/modules/AddSwiftStdlib.cmake

@@ -1743,7 +1743,6 @@ function(add_swift_target_library name)
   if (SWIFTLIB_IS_STDLIB)
     list(APPEND SWIFTLIB_SWIFT_COMPILE_FLAGS "-warn-implicit-overrides")
     list(APPEND SWIFTLIB_SWIFT_COMPILE_FLAGS "-Xfrontend;-enable-ossa-modules")
-    list(APPEND SWIFTLIB_SWIFT_COMPILE_FLAGS "-Xfrontend;-enable-lexical-lifetimes=false")
   endif()
 
   if(NOT SWIFT_BUILD_RUNTIME_WITH_HOST_COMPILER AND NOT BUILD_STANDALONE AND

stdlib/public/core/StringGutsRangeReplaceable.swift

@@ -247,7 +247,8 @@ extension _StringGuts {
 
     // We re-initialize from the modified storage to pick up new count, flags,
     // etc.
-    self = _StringGuts(self._object.nativeStorage)
+    var object = self._object // var keeps object alive during self mutation
+    self = _StringGuts(object.nativeStorage)
   }
 
   @inline(never) // slow-path
@@ -260,7 +261,8 @@ extension _StringGuts {
 
     // We re-initialize from the modified storage to pick up new count, flags,
     // etc.
-    self = _StringGuts(self._object.nativeStorage)
+    var object = self._object // var keeps object alive during self mutation
+    self = _StringGuts(object.nativeStorage)
   }
 
   internal mutating func clear() {
@@ -271,7 +273,8 @@ extension _StringGuts {
 
     // Reset the count
     _object.nativeStorage.clear()
-    self = _StringGuts(_object.nativeStorage)
+    var object = self._object // var keeps object alive during self mutation
+    self = _StringGuts(object.nativeStorage)
   }
 
   internal mutating func remove(from lower: Index, to upper: Index) {
@@ -284,7 +287,8 @@ extension _StringGuts {
       _object.nativeStorage.remove(from: lowerOffset, to: upperOffset)
       // We re-initialize from the modified storage to pick up new count, flags,
       // etc.
-      self = _StringGuts(self._object.nativeStorage)
+      var object = self._object // var keeps object alive during self mutation
+      self = _StringGuts(object.nativeStorage)
       return
     }
 
@@ -413,7 +417,8 @@ extension _StringGuts {
     let start = bounds.lowerBound._encodedOffset
     let end = bounds.upperBound._encodedOffset
     _object.nativeStorage.replace(from: start, to: end, with: codeUnits)
-    self = _StringGuts(_object.nativeStorage)
+    var object = self._object // var keeps object alive during self mutation
+    self = _StringGuts(object.nativeStorage)
     return Range(_uncheckedBounds: (start, start + codeUnits.count))
   }
 
@@ -437,7 +442,8 @@ extension _StringGuts {
     let end = bounds.upperBound._encodedOffset
     _object.nativeStorage.replace(
       from: start, to: end, with: codeUnits, replacementCount: replCount)
-    self = _StringGuts(_object.nativeStorage)
+    var object = self._object // var keeps object alive during self mutation
+    self = _StringGuts(object.nativeStorage)
     return Range(_uncheckedBounds: (start, start + replCount))
   }
 

stdlib/public/core/StringObject.swift

@@ -940,7 +940,10 @@ extension _StringObject {
     return _unsafeUncheckedDowncast(storage, to: __StringStorage.self)
 #else
     _internalInvariant(hasNativeStorage)
-    return Builtin.reinterpretCast(largeAddressBits)
+    let storageAddress =
+      UnsafeRawPointer(bitPattern:largeAddressBits)._unsafelyUnwrappedUnchecked
+    let unmanagedRef = Unmanaged<__StringStorage>.fromOpaque(storageAddress)
+    return unmanagedRef.takeUnretainedValue()
 #endif
   }
 

test/SILOptimizer/shrink_borrow_scope.sil

@@ -29,6 +29,20 @@ class PointerWrapper {
 
 enum OneOfThree { case one, two, three }
 
+final class Object {
+  init()
+}
+
+public struct Storage {
+  @_hasStorage var object: Builtin.BridgeObject { get set }
+}
+
+public struct Wrapper {
+  @_hasStorage var storage: Storage { get set }
+}
+
+sil @hasGuaranteedStorageArg : $@convention(method) (@guaranteed Storage) -> Builtin.Word
+
 sil [ossa] @callee_guaranteed: $@convention(thin) (@guaranteed C) -> ()
 sil [ossa] @get_owned_c : $@convention(thin) () -> (@owned C)
 sil [ossa] @callee_owned : $@convention(thin) (@owned C) -> ()
@@ -1016,6 +1030,81 @@ exit(%copy_1_2 : @owned $C, %borrow_in : @guaranteed $C, %copy_2_2 : @owned $C):
   return %copy_2_2 : $C
 }
 
+// Do not hoist the end_borrow (or destroy_value) of Storage above the copy_value
+// of the BridgeObject.
+//
+// rdar://90909833 (Add mayRetainUnmanagedReference deinit barrier)
+//
+// CHECK-LABEL: sil [ossa] @testMayRetainUnmanagedInlined : $@convention(method) (@inout Wrapper) -> () {
+// CHECK:   [[STORAGE:%.*]] = load [take] %1 : $*Storage
+// CHECK:   [[BORROW:%.*]] = begin_borrow [[STORAGE]] : $Storage
+// CHECK:   [[CAST:%.*]] = unchecked_bitwise_cast %{{.*}} : $Builtin.Word to $Builtin.BridgeObject
+// CHECK:   copy_value [[CAST]] : $Builtin.BridgeObject
+// CHECK:   end_borrow [[BORROW]] : $Storage
+// CHECK:   destroy_value [[STORAGE]] : $Storage
+// CHECK-LABEL: } // end sil function
+sil [ossa] @testMayRetainUnmanagedInlined : $@convention(method) (@inout Wrapper) -> () {
+bb0(%0 : $*Wrapper):
+  %1 = struct_element_addr %0 : $*Wrapper, #Wrapper.storage
+  %2 = load [take] %1 : $*Storage
+  // TODO: Convert this begin_borrow to lexical only after
+  // enabling lexical lifetimes in the stdlib.
+  %3 = begin_borrow %2 : $Storage
+  %4 = alloc_stack $Builtin.Word
+  %5 = struct_extract %3 : $Storage, #Storage.object
+  %6 = unchecked_trivial_bit_cast %5 : $Builtin.BridgeObject to $Builtin.Word
+  store %6 to [trivial] %4 : $*Builtin.Word
+  %10 = load [trivial] %4 : $*Builtin.Word
+  dealloc_stack %4 : $*Builtin.Word
+  %13 = unchecked_bitwise_cast %10 : $Builtin.Word to $Object
+  %16 = unchecked_trivial_bit_cast %13 : $Object to $Builtin.Word
+  %17 = unchecked_bitwise_cast %16 : $Builtin.Word to $Builtin.BridgeObject
+  %18 = copy_value %17 : $Builtin.BridgeObject
+  // Do not hoist this end_borrow above the copy_value!!!
+  end_borrow %3 : $Storage
+  destroy_value %2 : $Storage
+  %20 = struct $Storage (%18 : $Builtin.BridgeObject)
+  %21 = struct $Wrapper (%20 : $Storage)
+  store %21 to [init] %0 : $*Wrapper
+  %23 = tuple ()
+  return %23 : $()
+}
+
+// Do not hoist the end_borrow (or destroy_value) of Storage above the copy_value
+// of the BridgeObject. Here, the initial bitcast is hidden by a call.
+//
+// rdar://90909833 (Add mayRetainUnmanagedReference deinit barrier)
+//
+// CHECK-LABEL: sil [ossa] @testMayRetainUnmanagedCall : $@convention(method) (@inout Wrapper) -> () {
+// CHECK:   [[STORAGE:%.*]] = load [take] %1 : $*Storage
+// CHECK:   [[BORROW:%.*]] = begin_borrow [[STORAGE]] : $Storage
+// CHECK:   [[CALL:%.*]] = apply %{{.*}}([[BORROW]]) : $@convention(method) (@guaranteed Storage) -> Builtin.Word
+// CHECK:   [[CAST:%.*]] = unchecked_bitwise_cast [[CALL]] : $Builtin.Word to $Builtin.BridgeObject
+// CHECK:   copy_value [[CAST]] : $Builtin.BridgeObject
+// CHECK:   end_borrow [[BORROW]] : $Storage
+// CHECK:   destroy_value [[STORAGE]] : $Storage
+// CHECK-LABEL: } // end sil function
+sil [ossa] @testMayRetainUnmanagedCall : $@convention(method) (@inout Wrapper) -> () {
+bb0(%0 : $*Wrapper):
+  %1 = struct_element_addr %0 : $*Wrapper, #Wrapper.storage
+  %2 = load [take] %1 : $*Storage
+  // TODO: Convert this begin_borrow to lexical only after
+  // enabling lexical lifetimes in the stdlib.
+  %3 = begin_borrow %2 : $Storage
+  %4 = function_ref @hasGuaranteedStorageArg : $@convention(method) (@guaranteed Storage) -> Builtin.Word
+  %5 = apply %4(%3) : $@convention(method) (@guaranteed Storage) -> Builtin.Word
+  %6 = unchecked_bitwise_cast %5 : $Builtin.Word to $Builtin.BridgeObject
+  %7 = copy_value %6 : $Builtin.BridgeObject
+  // Do not hoist this end_borrow above the copy_value!!!
+  end_borrow %3 : $Storage
+  destroy_value %2 : $Storage
+  %10 = struct $Storage (%7 : $Builtin.BridgeObject)
+  %11 = struct $Wrapper (%10 : $Storage)
+  store %11 to [init] %0 : $*Wrapper
+  %23 = tuple ()
+  return %23 : $()
+}
+
 // =============================================================================
 // instruction tests                                                          }}
 // =============================================================================