[Runtime] Avoid weak definitions for client retain/release.

Instead of using weak definitions of swift_retain_preservemost and swift_release_preservemost, use weak references and explicitly check them for NULL.

Embedded Swift uses weak definitions for its runtime symbols. If swiftCore is able to override the weak definitions of the _preservemost functions, it will also override the Embedded symbols. They are not compatible, so this ends poorly.

Author: mikeash

stdlib/public/ClientRetainRelease/RetainRelease.s

@@ -36,6 +36,11 @@
 // offset so that it's still correct when the weak reference resolves to zero.
 .weak_reference __swift_retainRelease_slowpath_mask_v1
 
+// preservemost retain/release entrypoints in the runtime
+.weak_reference _swift_retain_preservemost
+.weak_reference _swift_release_preservemost
+
+
 // Grab our own copy of the slowpath mask. This mask is a value which indicates
 // when we must call into the runtime slowpath. If the object's refcount field
 // has any bits set that are in the mask, then we must take the slow path. The
@@ -87,57 +92,13 @@ ret
 // limits, in addition to the usual x19 and up. Any calls to functions that use
 // the standard calling convention need to save/restore x9-x15.
 
-.globl _swift_retain_preservemost
-.weak_definition _swift_retain_preservemost
-_swift_retain_preservemost:
-  maybe_pacibsp
-  stp   x0, x9, [sp, #-0x50]!
-  stp   x10, x11, [sp, #0x10]
-  stp   x12, x13, [sp, #0x20]
-  stp   x14, x15, [sp, #0x30]
-  stp   fp, lr, [sp, #0x40];
-  add   fp, sp, #0x40
-
-  // Clear the unused bits from the pointer
-  and   x0, x0, #BRIDGEOBJECT_POINTER_BITS
-  bl    _swift_retain
-
-  ldp   fp, lr, [sp, #0x40]
-  ldp   x14, x15, [sp, #0x30]
-  ldp   x12, x13, [sp, #0x20]
-  ldp   x10, x11, [sp, #0x10]
-  ldp   x0, x9, [sp], #0x50
-  ret_maybe_ab
-
-.globl _swift_release_preservemost
-.weak_definition _swift_release_preservemost
-_swift_release_preservemost:
-  maybe_pacibsp
-  str   x9, [sp, #-0x50]!
-  stp   x10, x11, [sp, #0x10]
-  stp   x12, x13, [sp, #0x20]
-  stp   x14, x15, [sp, #0x30]
-  stp   fp, lr, [sp, #0x40];
-  add   fp, sp, #0x40
-
-  // Clear the unused bits from the pointer
-  and   x0, x0, #BRIDGEOBJECT_POINTER_BITS
-  bl    _swift_release
-
-  ldp   fp, lr, [sp, #0x40]
-  ldp   x14, x15, [sp, #0x30]
-  ldp   x12, x13, [sp, #0x20]
-  ldp   x10, x11, [sp, #0x10]
-  ldr   x9, [sp], #0x50
-  ret_maybe_ab
-
 .private_extern _swift_bridgeObjectReleaseClient
 #if SWIFT_OBJC_INTEROP
 _swift_bridgeObjectReleaseClient:
   tbz  x0, #63, LbridgeObjectReleaseNotTagged
   ret
 LbridgeObjectReleaseNotTagged:
-  tbnz  x0, #62, _bridgeObjectReleaseClientObjC
+  tbnz  x0, #62, LbridgeObjectReleaseClientObjC
   and   x0, x0, 0x0ffffffffffffff8
 
 #else
@@ -221,10 +182,35 @@ Lrelease_ret:
 Lslowpath_release:
   CONDITIONAL USE_LDX_STX, \
     clrex
+// If the weak preservemost symbol is NULL, call our helper. Otherwise call the
+// runtime directly.
+  adrp x17, _swift_release_preservemost@GOTPAGE
+  ldr  x17, [x17, _swift_release_preservemost@GOTPAGEOFF]
+  cbz x17, Lcall_swift_release
   b _swift_release_preservemost
 
-.alt_entry _bridgeObjectReleaseClientObjC
-_bridgeObjectReleaseClientObjC:
+// Save/restore the preservemost registers and call swift_retain.
+Lcall_swift_release:
+  maybe_pacibsp
+  str   x9, [sp, #-0x50]!
+  stp   x10, x11, [sp, #0x10]
+  stp   x12, x13, [sp, #0x20]
+  stp   x14, x15, [sp, #0x30]
+  stp   fp, lr, [sp, #0x40];
+  add   fp, sp, #0x40
+
+  // Clear the unused bits from the pointer
+  and   x0, x0, #BRIDGEOBJECT_POINTER_BITS
+  bl    _swift_release
+
+  ldp   fp, lr, [sp, #0x40]
+  ldp   x14, x15, [sp, #0x30]
+  ldp   x12, x13, [sp, #0x20]
+  ldp   x10, x11, [sp, #0x10]
+  ldr   x9, [sp], #0x50
+  ret_maybe_ab
+
+LbridgeObjectReleaseClientObjC:
   maybe_pacibsp
   stp   x0, x9, [sp, #-0x50]!
   stp   x10, x11, [sp, #0x10]
@@ -252,7 +238,7 @@ _swift_bridgeObjectRetainClient:
   tbz  x0, #63, LbridgeObjectRetainNotTagged
   ret
 LbridgeObjectRetainNotTagged:
-  tbnz  x0, #62, _swift_bridgeObjectRetainClientObjC
+  tbnz  x0, #62, Lswift_bridgeObjectRetainClientObjC
 
 .alt_entry _swift_retainClient
 #else
@@ -334,10 +320,35 @@ Lretain_ret:
 Lslowpath_retain:
   CONDITIONAL USE_LDX_STX, \
     clrex
+// If the weak preservemost symbol is NULL, call our helper. Otherwise call the
+// runtime directly.
+  adrp x17, _swift_retain_preservemost@GOTPAGE
+  ldr  x17, [x17, _swift_retain_preservemost@GOTPAGEOFF]
+  cbz x17, Lcall_swift_retain
   b _swift_retain_preservemost
 
-.alt_entry _swift_bridgeObjectRetainClientObjC
-_swift_bridgeObjectRetainClientObjC:
+// Save/restore the preservemost registers and call swift_retain.
+Lcall_swift_retain:
+  maybe_pacibsp
+  stp   x0, x9, [sp, #-0x50]!
+  stp   x10, x11, [sp, #0x10]
+  stp   x12, x13, [sp, #0x20]
+  stp   x14, x15, [sp, #0x30]
+  stp   fp, lr, [sp, #0x40];
+  add   fp, sp, #0x40
+
+  // Clear the unused bits from the pointer
+  and   x0, x0, #BRIDGEOBJECT_POINTER_BITS
+  bl    _swift_retain
+
+  ldp   fp, lr, [sp, #0x40]
+  ldp   x14, x15, [sp, #0x30]
+  ldp   x12, x13, [sp, #0x20]
+  ldp   x10, x11, [sp, #0x10]
+  ldp   x0, x9, [sp], #0x50
+  ret_maybe_ab
+
+Lswift_bridgeObjectRetainClientObjC:
   maybe_pacibsp
   stp   x0, x9, [sp, #-0x50]!
   stp   x10, x11, [sp, #0x10]

test/abi/macOS/arm64/stdlib.swift

@@ -1147,5 +1147,4 @@ Added: __swift_debug_metadataAllocatorPageSize
 // New symbols to support emit-into-client retain/release.
 Added: __swift_retainRelease_slowpath_mask_v1
 Added: _swift_release_preservemost
-Added: _swift_release_preservemost_weak_placeholder
 Added: _swift_retain_preservemost

test/stdlib/symbol-visibility-darwin.test-sh

@@ -1,20 +1,19 @@
-// Ensure that we do not export any unwanted weak symbols from the dylibs.
+// Ensure that we do not export any weak symbols from the dylibs.
 //
 // Weak symbols require additional work from the loader to resolve the symbol at
 // load time and can cause ODR violations as well as unexpected symbol
 // satisfaction because the weak symbol may be used from a separate module.
 //
-// The _swift_release_preservemost_weak_placeholder symbol is present to allow
-// strong symbols from swiftCore to override weak symbols in other libraries,
-// to allow back-deployment usage of the swift_retain_preservemost and
-// swift_release_preservemost entrypoints.
+// Weak symbols in the runtime also conflict with the use of weak symbols in
+// embedded Swift. The presence of any weak symbol in swiftCore will allow its
+// strong symbols to override the weak symbols in the embedded runtime, and they
+// are not compatible.
 
 // RUN: %empty-directory(%t)
 
 // RUN: %llvm-nm --defined-only --extern-only --demangle %platform-dylib-dir/%target-library-name(swiftCore) > %t/swiftCore-all.txt
 // RUN: %llvm-nm --defined-only --extern-only --no-weak --demangle %platform-dylib-dir/%target-library-name(swiftCore) > %t/swiftCore-no-weak.txt
-// RUN: grep -v _swift_release_preservemost_weak_placeholder < %t/swiftCore-all.txt > %t/swiftCore-all-filtered.txt
-// RUN: diff -u %t/swiftCore-all-filtered.txt %t/swiftCore-no-weak.txt
+// RUN: diff -u %t/swiftCore-all.txt %t/swiftCore-no-weak.txt
 
 // RUN: %llvm-nm --defined-only --extern-only --demangle %platform-dylib-dir/%target-library-name(swiftRemoteMirror) > %t/swiftRemoteMirror-all.txt
 // RUN: %llvm-nm --defined-only --extern-only --no-weak --demangle %platform-dylib-dir/%target-library-name(swiftRemoteMirror) > %t/swiftRemoteMirror-no-weak.txt