Merge pull request #69476 from atrick/fix-conversion-lifetime

atrick · web-flow · commit 86365cdbd16e · 2023-11-01T17:36:27.000-07:00
Fix SILGen to emit fix_lifetime for inout-to-pointer conversion.
diff --git a/lib/SILGen/LValue.h b/lib/SILGen/LValue.h
@@ -633,6 +633,25 @@ struct LLVM_LIBRARY_VISIBILITY UnenforcedFormalAccess : FormalAccess {
   void finishImpl(SILGenFunction &SGF) override;
 };
 
+// A formal access that keeps an LValue alive across an expression that uses an
+// unsafe pointer into that LValue. This supports emitLValueToPointer, which
+// handles InoutToPointerExpr. This formal access is nested within whatever
+// formal access is needed for the LValue itself and emits a fix_lifetime
+// instruction after the apply.
+struct LLVM_LIBRARY_VISIBILITY LValueToPointerFormalAccess : FormalAccess {
+  static SILValue enter(SILGenFunction &SGF, SILLocation loc, SILValue address);
+
+  SILValue address;
+
+  LValueToPointerFormalAccess(SILLocation loc, SILValue address,
+                              CleanupHandle cleanup)
+    : FormalAccess(sizeof(*this), FormalAccess::Unenforced, loc, cleanup),
+    address(address) {}
+
+  // Only called at the end formal evaluation scope. Emit fix_lifetime.
+  void finishImpl(SILGenFunction &SGF) override;
+};
+
 } // namespace Lowering
 } // namespace swift
 
diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp
@@ -5934,6 +5934,67 @@ static void diagnoseImplicitRawConversion(Type sourceTy, Type pointerTy,
   }
 }
 
+namespace {
+/// Cleanup to insert fix_lifetime on an LValue address.
+class FixLifetimeLValueCleanup : public Cleanup {
+  friend LValueToPointerFormalAccess;
+
+  FormalEvaluationContext::stable_iterator depth;
+
+public:
+  FixLifetimeLValueCleanup() : depth() {}
+
+  LValueToPointerFormalAccess &getFormalAccess(SILGenFunction &SGF) const {
+    auto &access = *SGF.FormalEvalContext.find(depth);
+    return static_cast<LValueToPointerFormalAccess &>(access);
+  }
+
+  void emit(SILGenFunction &SGF, CleanupLocation l,
+            ForUnwind_t forUnwind) override {
+    getFormalAccess(SGF).finish(SGF);
+  }
+
+  SILValue getAddress(SILGenFunction &SGF) const {
+    return getFormalAccess(SGF).address;
+  }
+
+  void dump(SILGenFunction &SGF) const override {
+#ifndef NDEBUG
+    llvm::errs() << "FixLifetimeLValueCleanup "
+                 << "State:" << getState() << " "
+                 << "Address: " << getAddress(SGF) << "\n";
+#endif
+  }
+};
+} // end anonymous namespace
+
+SILValue LValueToPointerFormalAccess::enter(SILGenFunction &SGF,
+                                            SILLocation loc,
+                                            SILValue address) {
+  auto &lowering = SGF.getTypeLowering(address->getType().getObjectType());
+  SILValue pointer = SGF.B.createAddressToPointer(
+    loc, address, SILType::getRawPointerType(SGF.getASTContext()),
+    /*needsStackProtection=*/ true);
+  if (!lowering.isTrivial()) {
+    assert(SGF.isInFormalEvaluationScope() &&
+           "Must be in formal evaluation scope");
+    auto &cleanup = SGF.Cleanups.pushCleanup<FixLifetimeLValueCleanup>();
+    CleanupHandle handle = SGF.Cleanups.getTopCleanup();
+    SGF.FormalEvalContext.push<LValueToPointerFormalAccess>(loc, address,
+                                                            handle);
+    cleanup.depth = SGF.FormalEvalContext.stable_begin();
+  }
+  return pointer;
+}
+
+// Address-to-pointer conversion always requires either a fix_lifetime or
+// mark_dependence. Emitting a fix_lifetime immediately after the call as
+// opposed to a mark_dependence allows the lvalue's lifetime to be optimized
+// outside of this narrow scope.
+void LValueToPointerFormalAccess::finishImpl(SILGenFunction &SGF) {
+  SGF.B.emitFixLifetime(loc, address);
+}
+
 /// Convert an l-value to a pointer type: unsafe, unsafe-mutable, or
 /// autoreleasing-unsafe-mutable.
 ManagedValue SILGenFunction::emitLValueToPointer(SILLocation loc, LValue &&lv,
@@ -5979,9 +6040,8 @@ ManagedValue SILGenFunction::emitLValueToPointer(SILLocation loc, LValue &&lv,
   // Get the lvalue address as a raw pointer.
   SILValue address =
     emitAddressOfLValue(loc, std::move(lv)).getUnmanagedValue();
-  address = B.createAddressToPointer(loc, address,
-                               SILType::getRawPointerType(getASTContext()),
-              /*needsStackProtection=*/ true);
+
+  SILValue pointer = LValueToPointerFormalAccess::enter(*this, loc, address);
   
   // Disable nested writeback scopes for any calls evaluated during the
   // conversion intrinsic.
@@ -5996,7 +6056,7 @@ ManagedValue SILGenFunction::emitLValueToPointer(SILLocation loc, LValue &&lv,
                                                        getPointerProtocol());
   return emitApplyOfLibraryIntrinsic(
              loc, converter, subMap,
-             ManagedValue::forObjectRValueWithoutOwnership(address),
+             ManagedValue::forObjectRValueWithoutOwnership(pointer),
              SGFContext())
       .getAsSingleValue(*this, loc);
 }
diff --git a/lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp b/lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp
@@ -2448,6 +2448,17 @@ bool GatherUsesVisitor::visitUse(Operand *op) {
     return true;
   }
 
+  if (auto *fixLifetime = dyn_cast<FixLifetimeInst>(op->getUser())) {
+    auto leafRange = TypeTreeLeafTypeRange::get(op->get(), getRootAddress());
+    if (!leafRange) {
+      LLVM_DEBUG(llvm::dbgs() << "Failed to compute leaf range!\n");
+      return false;
+    }
+
+    useState.recordLivenessUse(user, *leafRange);
+    return true;
+  }
+
   // If we don't fit into any of those categories, just track as a liveness
   // use. We assume all such uses must only be reads to the memory. So we assert
   // to be careful.
diff --git a/lib/SILOptimizer/Mandatory/MoveOnlyWrappedTypeEliminator.cpp b/lib/SILOptimizer/Mandatory/MoveOnlyWrappedTypeEliminator.cpp
@@ -199,6 +199,8 @@ struct SILMoveOnlyWrappedTypeEliminatorVisitor
   NO_UPDATE_NEEDED(BeginAccess)
   NO_UPDATE_NEEDED(EndAccess)
   NO_UPDATE_NEEDED(ClassMethod)
+  NO_UPDATE_NEEDED(FixLifetime)
+  NO_UPDATE_NEEDED(AddressToPointer)
 #undef NO_UPDATE_NEEDED
 
   bool eliminateIdentityCast(SingleValueInstruction *svi) {
diff --git a/test/SILGen/pointer_conversion.swift b/test/SILGen/pointer_conversion.swift
@@ -1,4 +1,3 @@
-
 // RUN: %target-swift-emit-silgen -module-name pointer_conversion -sdk %S/Inputs -I %S/Inputs -enable-source-import %s -enable-objc-interop | %FileCheck %s
 
 // FIXME: rdar://problem/19648117 Needs splitting objc parts out
@@ -22,6 +21,9 @@ func takesOptConstRawPointer(_ x: UnsafeRawPointer?, and: Int) {}
 func takesOptOptConstRawPointer(_ x: UnsafeRawPointer??, and: Int) {}
 func takesMutableFunctionPointer(_ x: UnsafeMutablePointer<() -> Void>) {}
 
+@_silgen_name("takeObjectPointer")
+func takeObjectPointer(_: UnsafePointer<AnyObject>)
+
 // CHECK-LABEL: sil hidden [ossa] @$s18pointer_conversion0A9ToPointeryySpySiG_SPySiGSvtF
 // CHECK: bb0([[MP:%.*]] : $UnsafeMutablePointer<Int>, [[CP:%.*]] : $UnsafePointer<Int>, [[MRP:%.*]] : $UnsafeMutableRawPointer):
 func pointerToPointer(_ mp: UnsafeMutablePointer<Int>,
@@ -449,3 +451,31 @@ func optOptStringToOptOptPointer(string: String??) {
   // CHECK:   br [[SOME_CONT_BB]]([[NO_VALUE]] : $Optional<Optional<UnsafeRawPointer>>, [[NO_OWNER]] : $Optional<AnyObject>)
   takesOptOptConstRawPointer(string, and: sideEffect1())
 }
+
+final public class Obj {
+  var object: AnyObject
+
+  init(object: AnyObject) { self.object = object }
+}
+
+public struct RefObj {
+  var o: Obj
+}
+
+// CHECK-LABEL: sil [ossa] @$s18pointer_conversion20objectFieldToPointer2rcyAA6RefObjV_tF : $@convention(thin) (@guaranteed RefObj) -> () {
+// CHECK: bb0(%0 : @guaranteed $RefObj):
+// CHECK:   [[B:%.*]] = begin_borrow %{{.*}} : $Obj
+// CHECK:   [[R:%.*]] = ref_element_addr [[B]] : $Obj, #Obj.object
+// CHECK:   [[A:%.*]] = begin_access [read] [dynamic] [[R]] : $*AnyObject
+// CHECK:   [[P:%.*]] = address_to_pointer [stack_protection] [[A]] : $*AnyObject to $Builtin.RawPointer
+// CHECK:   apply %{{.*}}<UnsafePointer<AnyObject>>(%{{.*}}, %7) : $@convention(thin) <τ_0_0 where τ_0_0 : _Pointer> (Builtin.RawPointer) -> @out τ_0_0
+// CHECK:   apply {{.*}} : $@convention(thin) (UnsafePointer<AnyObject>) -> ()
+// CHECK:   fix_lifetime [[A]] : $*AnyObject
+// CHECK:   end_access [[A]] : $*AnyObject
+// CHECK:   end_borrow [[B]] : $Obj
+// CHECK:   destroy_value %{{.*}} : $Obj
+// CHECK:   dealloc_stack %{{.*}} : $*UnsafePointer<AnyObject>
+// CHECK-LABEL: } // end sil function '$s18pointer_conversion20objectFieldToPointer2rcyAA6RefObjV_tF'
+public func objectFieldToPointer(rc: RefObj) {
+  takeObjectPointer(&rc.o.object)
+}
diff --git a/test/SILOptimizer/pointer_conversion.swift b/test/SILOptimizer/pointer_conversion.swift
@@ -118,3 +118,95 @@ public func testWithMemoryRebound<T, Result>(
   }
   return try body(.init(rawPtr))
 }
+
+@_silgen_name("takeRawPointer")
+func takeRawPointer(_: UnsafeRawPointer)
+
+@_silgen_name("takeObjectPointer")
+func takeObjectPointer(_: UnsafePointer<AnyObject>)
+
+@_silgen_name("takeStringPointer")
+func takeStringPointer(_: UnsafePointer<String>)
+
+@_silgen_name("takeDictionaryPointer")
+func takeDictionaryPointer(_: UnsafePointer<Dictionary<Int, Int>>)
+
+// Test conversion of a dictionary to a raw pointer. This is not a sane
+// conversion, but the compiler must still generate memory-safe code.
+//
+// A dictionary is an eager-move type, so it will be destroyed at its
+// last use. An address_to_pointer operation escapes the pointer, so
+// the compiler never sees those pointer uses. The pointer's scope
+// must be protected by a fix_lifetime.
+//
+// NOTE: If this test triggers a compiler error because of the unsafe
+// inout conversion, then we have arrived at a better world. Delete
+// the test. The eagerMoveToPointer test below is sufficient.
+//
+// CHECK-LABEL: sil [stack_protection] @$s18pointer_conversion22dictionaryToRawPointeryyF : $@convention(thin) () -> () {
+// CHECK: [[A:%.*]] = alloc_stack $Dictionary<Int, Int>, var, name "d"
+// CHECK: [[PTR:%.*]] = address_to_pointer [stack_protection] [[A]] : $*Dictionary<Int, Int> to $Builtin.RawPointer
+// CHECK: [[UP:%.*]] = struct $UnsafeRawPointer ([[PTR]] : $Builtin.RawPointer)
+// CHECK: apply %{{.*}}([[UP]]) : $@convention(thin) (UnsafeRawPointer) -> ()
+// CHECK: [[D:%.*]] = load %0 : $*Dictionary<Int, Int>
+// CHECK: fix_lifetime [[D]] : $Dictionary<Int, Int>
+// CHECK: release_value [[D]] : $Dictionary<Int, Int>
+// CHECK: dealloc_stack [[A]] : $*Dictionary<Int, Int>
+// CHECK-LABEL: } // end sil function '$s18pointer_conversion22dictionaryToRawPointeryyF'
+public func dictionaryToRawPointer() {
+  var d = [1:1]
+  takeRawPointer(&d)
+}
+
+// Test conversion of a non-trivial eager-move type to a raw pointer.
+// This currently only applies to Dictionary, but converting a
+// dictionary to a pointer will likely be a compiler error in the
+// future. So force an eagerMove type here.
+//
+// An eager-move type will be destroyed at its last use. An
+// address_to_pointer operation escapes the pointer, so the compiler
+// never sees those pointer uses. The pointer's scope must be
+// protected by a fix_lifetime.
+// CHECK-LABEL: sil [stack_protection] @$s18pointer_conversion18eagerMoveToPointer1oyyXln_tF : $@convention(thin) (@owned AnyObject) -> () {
+// CHECK: [[A:%.*]] = alloc_stack [moveable_value_debuginfo] $AnyObject, var, name "o"
+// CHECK: [[PTR:%.*]] = address_to_pointer [stack_protection] [[A]] : $*AnyObject to $Builtin.RawPointer
+// CHECK: [[UP:%.*]] = struct $UnsafePointer<AnyObject> ([[PTR]] : $Builtin.RawPointer)
+// CHECK: apply %{{.*}}([[UP]]) : $@convention(thin) (UnsafePointer<AnyObject>) -> ()
+// CHECK: [[O:%.*]] = load [[A]] : $*AnyObject
+// CHECK: fix_lifetime [[O]] : $AnyObject
+// CHECK: strong_release [[O]] : $AnyObject
+// CHECK: dealloc_stack [[A]] : $*AnyObject
+// CHECK-LABEL: } // end sil function '$s18pointer_conversion18eagerMoveToPointer1oyyXln_tF'
+public func eagerMoveToPointer(@_eagerMove o: consuming AnyObject ) {
+  takeObjectPointer(&o)
+}
+
+// CHECK-LABEL: sil [stack_protection] @$s18pointer_conversion15stringToPointer2ssySS_tF : $@convention(thin) (@guaranteed String) -> () {
+// CHECK:   [[A:%.*]] = alloc_stack $String, var, name "s"
+// CHECK:   [[PTR:%.*]] = address_to_pointer [stack_protection] [[A]] : $*String to $Builtin.RawPointer
+// CHECK:   [[UP:%.*]] = struct $UnsafePointer<String> ([[PTR]] : $Builtin.RawPointer)
+// CHECK:   apply {{.*}}([[UP]]) : $@convention(thin) (UnsafePointer<String>) -> ()
+// CHECK:   [[S:%.*]] = load [[A]] : $*String
+// CHECK:   fix_lifetime [[S]] : $String
+// CHECK:   release_value [[S]] : $String
+// CHECK:   dealloc_stack [[A]] : $*String
+// CHECK-LABEL: } // end sil function '$s18pointer_conversion15stringToPointer2ssySS_tF'
+public func stringToPointer(ss: String) {
+  var s = ss
+  takeStringPointer(&s)
+}
+
+// CHECK-LABEL: sil [stack_protection] @$s18pointer_conversion19dictionaryToPointer2ddySDyS2iG_tF : $@convention(thin) (@guaranteed Dictionary<Int, Int>) -> () {
+// CHECK:   [[A:%.*]] = alloc_stack $Dictionary<Int, Int>, var, name "d"
+// CHECK:   [[PTR:%.*]] = address_to_pointer [stack_protection] [[A]] : $*Dictionary<Int, Int> to $Builtin.RawPointer
+// CHECK:   [[UP:%.*]] = struct $UnsafePointer<Dictionary<Int, Int>> ([[PTR]] : $Builtin.RawPointer)
+// CHECK:   apply %{{.*}}([[UP]]) : $@convention(thin) (UnsafePointer<Dictionary<Int, Int>>) -> ()
+// CHECK:   [[D:%.*]] = load [[A]] : $*Dictionary<Int, Int>
+// CHECK:   fix_lifetime [[D]] : $Dictionary<Int, Int>
+// CHECK:   release_value [[D]] : $Dictionary<Int, Int>
+// CHECK:   dealloc_stack [[A]] : $*Dictionary<Int, Int>
+// CHECK-LABEL: } // end sil function '$s18pointer_conversion19dictionaryToPointer2ddySDyS2iG_tF'
+public func dictionaryToPointer(dd: Dictionary<Int, Int>) {
+  var d = dd
+  takeDictionaryPointer(&d)
+}
