Merge pull request #80507 from jckarter/builtin-emplace-double-free

SILGen: Correct ownership forwarding of `Builtin.emplace`.

Author: jckarter

lib/SILGen/SILGenBuiltin.cpp

@@ -2130,6 +2130,7 @@ static ManagedValue emitBuiltinEmplace(SILGenFunction &SGF,
                                              resultASTTy);
   bool didEmitInto;
   Initialization *dest;
+  TemporaryInitialization *destTemporary = nullptr;
   std::unique_ptr<Initialization> destOwner;
 
   // Use the context destination if available.
@@ -2139,8 +2140,9 @@ static ManagedValue emitBuiltinEmplace(SILGenFunction &SGF,
     dest = C.getEmitInto();
   } else {
     didEmitInto = false;
-    destOwner = SGF.emitTemporary(loc, loweredBufferTy);
-    dest = destOwner.get();
+    auto destTempOwner = SGF.emitTemporary(loc, loweredBufferTy);
+    dest = destTemporary = destTempOwner.get();
+    destOwner = std::move(destTempOwner);
   }
 
   auto buffer = dest->getAddressForInPlaceInitialization(SGF, loc);
@@ -2187,16 +2189,19 @@ static ManagedValue emitBuiltinEmplace(SILGenFunction &SGF,
     return ManagedValue::forInContext();
   }
 
+  assert(destTemporary
+         && "didn't emit into context but also didn't emit into temporary?");
+  auto result = destTemporary->getManagedAddress();
   auto resultTy = SGF.getLoweredType(subs.getReplacementTypes()[0]);
 
   // If the result is naturally address-only, then we can adopt the stack slot
   // as the value directly.
   if (resultTy == loweredBufferTy.getLoweredType().getAddressType()) {
-    return SGF.emitManagedRValueWithCleanup(buffer);
+    return result;
   }
 
   // If the result is loadable, load it.
-  return SGF.B.createLoadTake(loc, ManagedValue::forLValue(buffer));
+  return SGF.B.createLoadTake(loc, result);
 }
 
 std::optional<SpecializedEmitter>

test/SILGen/emplace.swift

@@ -0,0 +1,93 @@
+// RUN: %target-swift-frontend -enable-experimental-feature BuiltinModule -emit-silgen %s | %FileCheck %s
+
+// REQUIRES: swift_feature_BuiltinModule
+
+import Builtin
+
+@_silgen_name("do_emplace")
+func do_emplace(_: Builtin.RawPointer)
+
+struct AO {
+    var x: Any
+}
+
+struct Loadable {
+    var x: AnyObject
+}
+
+// CHECK-LABEL: sil {{.*}} @$s{{.*}}_in_place_loadable
+func emplace_in_place_loadable() -> Loadable {
+    // CHECK: [[TMP:%.*]] = alloc_stack $Loadable
+    // CHECK-NEXT: builtin "zeroInitializer"([[TMP]])
+    // CHECK-NEXT: [[PTR:%.*]] = address_to_pointer {{.*}} [[TMP]]
+    // CHECK-NEXT: apply {{.*}}([[PTR]])
+    // CHECK-NEXT: load [take] [[TMP]]
+    // CHECK-NEXT: dealloc_stack [[TMP]]
+    return Builtin.emplace(do_emplace)
+}
+
+// CHECK-LABEL: sil {{.*}} @$s{{.*}}_in_place_ao
+// CHECK: bb0([[OUT:%.*]] : $*AO):
+func emplace_in_place_ao() -> AO {
+    // CHECK: builtin "zeroInitializer"([[OUT]])
+    // CHECK-NEXT: [[PTR:%.*]] = address_to_pointer {{.*}} [[OUT]]
+    // CHECK-NEXT: apply [[FN:%.*]]([[PTR]])
+    // CHECK-NEXT: destroy_value [[FN]]
+    // CHECK-NEXT: tuple ()
+    // CHECK-NEXT: return
+    return Builtin.emplace(do_emplace)
+}
+
+// CHECK-LABEL: sil {{.*}} @$s{{.*}}_assign_loadable
+// CHECK: bb0([[INOUT:%.*]] : $*Loadable):
+func emplace_assign_loadable(_ x: inout Loadable) {
+    // CHECK: [[TMP:%.*]] = alloc_stack $Loadable
+    // CHECK-NEXT: builtin "zeroInitializer"([[TMP]])
+    // CHECK-NEXT: [[PTR:%.*]] = address_to_pointer {{.*}} [[TMP]]
+    // CHECK-NEXT: apply {{.*}}([[PTR]])
+    // CHECK-NEXT: [[RESULT:%.*]] = load [take] [[TMP]]
+    // CHECK-NEXT: [[WRITE:%.*]] = begin_access [modify] [unknown] [[INOUT]]
+    // CHECK-NEXT: assign [[RESULT]] to [[WRITE]]
+    // CHECK-NEXT: end_access [[WRITE]]
+    // CHECK-NEXT: dealloc_stack [[TMP]]
+    x = Builtin.emplace(do_emplace)
+}
+
+// CHECK-LABEL: sil {{.*}} @$s{{.*}}_assign_ao
+// CHECK: bb0([[INOUT:%.*]] : $*AO):
+func emplace_assign_ao(_ x: inout AO) {
+    // CHECK: [[TMP:%.*]] = alloc_stack $AO
+    // CHECK-NEXT: builtin "zeroInitializer"([[TMP]])
+    // CHECK-NEXT: [[PTR:%.*]] = address_to_pointer {{.*}} [[TMP]]
+    // CHECK-NEXT: apply {{.*}}([[PTR]])
+    // CHECK-NEXT: [[WRITE:%.*]] = begin_access [modify] [unknown] [[INOUT]]
+    // CHECK-NEXT: copy_addr [take] [[TMP]] to [[WRITE]]
+    // CHECK-NEXT: end_access [[WRITE]]
+    // CHECK-NEXT: dealloc_stack [[TMP]]
+    x = Builtin.emplace(do_emplace)
+}
+
+// CHECK-LABEL: sil {{.*}} @$s{{.*}}_ignore_loadable
+func emplace_ignore_loadable() {
+    // CHECK: [[TMP:%.*]] = alloc_stack $Loadable
+    // CHECK-NEXT: builtin "zeroInitializer"([[TMP]])
+    // CHECK-NEXT: [[PTR:%.*]] = address_to_pointer {{.*}} [[TMP]]
+    // CHECK-NEXT: apply {{.*}}([[PTR]])
+    // CHECK-NEXT: [[RESULT:%.*]] = load [take] [[TMP]]
+    // CHECK-NEXT: ignored_use [[RESULT]]
+    // CHECK-NEXT: destroy_value [[RESULT]]
+    // CHECK-NEXT: dealloc_stack [[TMP]]
+    let _: Loadable = Builtin.emplace(do_emplace)
+}
+
+// CHECK-LABEL: sil {{.*}} @$s{{.*}}_ignore_ao
+func emplace_ignore_ao() {
+    // CHECK: [[TMP:%.*]] = alloc_stack $AO
+    // CHECK-NEXT: builtin "zeroInitializer"([[TMP]])
+    // CHECK-NEXT: [[PTR:%.*]] = address_to_pointer {{.*}} [[TMP]]
+    // CHECK-NEXT: apply {{.*}}([[PTR]])
+    // CHECK-NEXT: ignored_use [[TMP]]
+    // CHECK-NEXT: destroy_addr [[TMP]]
+    // CHECK-NEXT: dealloc_stack [[TMP]]
+    let _: AO = Builtin.emplace(do_emplace)
+}