stdlib: Add backward deployment versions for the dynamic-replacement runtime functions #25473

aschwaighofer · 2019-06-14T17:54:53Z

The recent change of how we do dynamic replacements added 2 new runtime
functions. This patch adds those functions to the Compatibility50 static
archive.

This will allow backward deployment to a swift 5.0 runtime.

Patch by Erik Eckstein with a modification to call the standard
libraries implementation (marked as weak) when it is available.

This ensures we can change the implementation in the future and are not
ABI locked.

rdar://problem/51601233

dynamic-replacement runtime functions. The recent change of how we do dynamic replacements added 2 new runtime functions. This patch adds those functions to the Compatibility50 static archive. This will allow backward deployment to a swift 5.0 runtime. Patch by Erik Eckstein with a modification to call the standard libraries implementation (marked as weak) when it is available. This ensures we can change the implementation in the future and are not ABI locked. rdar://problem/51601233

aschwaighofer · 2019-06-14T18:01:38Z

@jordan I have not moved the functions out into a separate library because we always link in the compatibility library.

https://github.com/apple/swift/blob/master/lib/IRGen/GenDecl.cpp#L453

 453   // FIXME: It'd be better to have the driver invocation or build system that
 454   // executes the linker introduce these compatibility libraries, since at
 455   // that point we know whether we're building an executable, which is the only
 456   // place where the compatibility libraries take effect. For the benefit of
 457   // build systems that build Swift code, but don't use Swift to drive
 458   // the linker, we can also use autolinking to pull in the compatibility
 459   // libraries. This may however cause the library to get pulled in in
 460   // situations where it isn't useful, such as for dylibs, though this is
 461   // harmless aside from code size.
 462   if (!IRGen.Opts.UseJIT) {
 463     if (auto compatibilityVersion
 464           = IRGen.Opts.AutolinkRuntimeCompatibilityLibraryVersion) {
 465       if (*compatibilityVersion <= llvm::VersionTuple(5, 0)) {
 466         this->addLinkLibrary(LinkLibrary("swiftCompatibility50",
 467                                          LibraryKind::Library,
 468                                          /*forceLoad*/ true));
 469       }
 470     }
 471   }
 472 }

aschwaighofer · 2019-06-14T18:03:22Z

@swift-ci please test

jrose-apple · 2019-06-14T18:10:57Z

Ah, I didn't realize Joe changed that. Okay.

jckarter · 2019-06-14T18:38:01Z

include/swift/Runtime/Exclusivity.h

@@ -44,13 +44,19 @@ void swift_beginAccess(void *pointer, ValueBuffer *buffer,
 /// replacement function if it should be called.
 /// Returns null if the original function (which is passed in \p CurrFn) should
 /// be called.
+#ifdef __APPLE__
+__attribute__((weak))
+#endif


This will export these symbols as a weak definition, which is probably not what you want (and very bad for load time performance). Did you mean __attribute__((weak_import)) ?

Yes. Will fix.

jckarter · 2019-06-14T18:51:51Z

include/swift/Runtime/RuntimeFunctions.def

+         ARGS(FunctionPtrTy->getPointerTo()),
+         ATTRS(NoUnwind))
+
+FUNCTION(GetReplacement50, swift_getFunctionReplacement50, C_CC,


I had been thinking about this issue of backporting new entry points with the compatibility library, and I wonder whether there might be a more systematic way of going about it. I was thinking that maybe we could turn the return type of the availability function here from a bool to a tristate, ConditionallyAvailable | AvailableByCompatibilityLibrary | AlwaysAvailable. We could factor the logic for choosing the compatibility library or OS version of the entry point into getRuntimeFn then (by establishing a mangling pattern for the backport stub like "<swift_runtimeFunctionName>" + compatibility_version like you have here), instead of repeating this conditional logic for every client of the entry point.

swift-ci · 2019-06-14T19:14:23Z

Build failed
Swift Test OS X Platform
Git Sha - 5003c15

swift-ci · 2019-06-14T19:20:53Z

Build failed
Swift Test Linux Platform
Git Sha - 5003c15

jckarter · 2019-06-14T20:26:39Z

Ah, I didn't realize Joe changed that. Okay.

That’s a bug, though, not a feature. The compatibility library should only link into the main executable, and dylibs should get back deployment support from the executable. An approach that should work without a separate library, and without relying on the compatibility library getting linked into dylibs, might be to add the back-deployment functions to the static archive as exported entry points, then emit a common resolution thunk that tries to find the entry point, first by looking in libswiftCore.dylib, then by looking in the main executable.

aschwaighofer · 2019-06-14T20:33:52Z

@swift-ci Please test

swift-ci · 2019-06-14T20:38:27Z

Build failed
Swift Test Linux Platform
Git Sha - 5003c15

swift-ci · 2019-06-14T20:54:26Z

Build failed
Swift Test OS X Platform
Git Sha - 5003c15

aschwaighofer · 2019-06-14T22:49:56Z

@swift-ci Please test

swift-ci · 2019-06-14T22:56:41Z

Build failed
Swift Test Linux Platform
Git Sha - eb32194

swift-ci · 2019-06-14T23:19:41Z

Build failed
Swift Test OS X Platform
Git Sha - eb32194

aschwaighofer · 2019-06-15T14:22:30Z

@swift-ci Please test

swift-ci · 2019-06-15T14:24:46Z

Build failed
Swift Test Linux Platform
Git Sha - 6f9c50c

swift-ci · 2019-06-15T14:27:08Z

Build failed
Swift Test OS X Platform
Git Sha - 6f9c50c

aschwaighofer · 2019-06-17T14:42:04Z

@jckarter I think I have addressed your concerns. Are you okay with the latest version?

jckarter

There's still the issue of dynamic libraries. Compatibility50 will not be linked into dylibs, so they need to get the back-deployed entry points from the one linked into the main executable, if not from the OS runtime.

jckarter · 2019-06-17T15:38:14Z

include/swift/Runtime/RuntimeFunctions.def

@@ -638,7 +638,7 @@ FUNCTION(GetGenericMetadata, swift_getGenericMetadata,
 //                                     const OpaqueTypeDescriptor *descriptor,
 //                                     uintptr_t index);
 FUNCTION(GetOpaqueTypeMetadata, swift_getOpaqueTypeMetadata,
-         SwiftCC, OpaqueTypeAvailability,
+         SwiftCC, ConditionallyAvailable,


These all still need to call a function to return the availability appropriate for the current deployment target. There will be runtime entry points in the future that are available on a different set of OSes from Swift 5.1, so we shouldn't hardcode that.

This is what conditionally available means?

The code in getRuntimeFn() queries whether the feature is available based on whether something is AlwaysAvailable| ConditionallyAvailable | AvailableByCompatibilityLibrary:

auto isFeatureAvailable = [&]() -> bool { auto deploymentAvailability = AvailabilityContext::forDeploymentTarget(*context); auto featureAvailability = context->getSwift51Availability(); return deploymentAvailability.isContainedIn(featureAvailability); }; switch (availability) { case RuntimeAvailability:: AlwaysAvailable: // Nothing to do. break; case RuntimeAvailability::ConditionallyAvailable: { isWeakLinked = !isFeatureAvailable(); break; } case RuntimeAvailability::AvailableByCompatibilityLibrary: { if (!isFeatureAvailable()) functionName.append("50"); break; } }

In the future we can extend this by adding another entry to runtime.def that specifies at which swift version things became available ...

FUNCTION(GetOpaqueTypeMetadata, swift_getOpaqueTypeMetadata, SwiftCC, ConditionallyAvailable, Swift51

And then generate the availability based on that.

context->get##AVAILVERSION##Availability

An entry point is only ConditionallyAvailable if we're deploying back to a runtime version that may not have the entry point. If we're deploying to an OS with Swift 5.1, everything is AlwaysAvailable. Having a single version doesn't seem sufficient to me, because in the future, we may end up in a situation where a runtime entry point can only be back-deployed in the compatibility library to certain runtime versions; for instance, a Swift 5.2 feature may be completely unavailable on Swift 5.0, although we have a compatibility fallback that works for the 5.1 runtime. The most future-proof thing to do is to have the FUNCTION xmacro refer to a function that returns the enum value appropriate to the deployment target, the way it was before.

I will address this in a follow-up patch.

Attempt of follow-up here: #25573

We use one bit of the third reserved swift private tls key. Also move the functionality into a separate static archive that is always linked dependent on deployment target.

aschwaighofer · 2019-06-17T22:05:00Z

@swift-ci Please test

swift-ci · 2019-06-17T22:07:07Z

Build failed
Swift Test Linux Platform
Git Sha - da2d418

jckarter

We can address some of these issues in follow up commits.

swift-ci · 2019-06-17T22:41:46Z

Build failed
Swift Test OS X Platform
Git Sha - da2d418

jrose-apple · 2019-06-18T01:25:41Z

include/swift/Option/Options.td

+    : Flag<[ "-" ],
+    "disable-autolinking-runtime-compatibility-dynamic-replacements">,
+      Flags<[ FrontendOption ]>,
+      HelpText<"Do not use autolinking for runtime compatibility libraries">;


Please fix the formatting here to match the other lines, and please change the help text.

…does this really need a separate flag, though?

jrose-apple · 2019-06-18T01:26:22Z

lib/Driver/ToolChains.cpp

+  if (context.Args.hasArg(options::
+          OPT_disable_autolinking_runtime_compatibility_dynamic_replacements)) {
+    Arguments.push_back(
+        "-disable-autolinking-runtime-compatibility-dynamic-replacements");


This is just context.Args.AddLastArg(Arguments, OPT_disable_autolinking_runtime_compatibility_dynamic_replacements) (and Joe should have used that too above).

jrose-apple · 2019-06-18T01:28:38Z

stdlib/public/runtime/ThreadLocalStorage.h


 # define SWIFT_RUNTIME_TLS_KEY __PTK_FRAMEWORK_SWIFT_KEY0
 # define SWIFT_STDLIB_TLS_KEY __PTK_FRAMEWORK_SWIFT_KEY1
+# define SWIFT_RUNTIME2_TLS_KEY __PTK_FRAMEWORK_SWIFT_KEY2


Please give this a more descriptive name, like SWIFT_COMPATIBILITY_50_TLS_KEY. That'll indicate when it's okay to reuse it.

aschwaighofer · 2019-06-18T02:52:08Z

@swift-ci Please test

swift-ci · 2019-06-18T02:52:29Z

Build failed
Swift Test OS X Platform
Git Sha - 5d32946

swift-ci · 2019-06-18T02:54:15Z

Build failed
Swift Test Linux Platform
Git Sha - 5d32946

aschwaighofer · 2019-06-18T03:13:58Z

@swift-ci Please test source compatibility

aschwaighofer · 2019-06-18T08:00:50Z

@swift-ci Please test

aschwaighofer · 2019-06-18T08:02:37Z

@swit-ci Please test source compatibility

swift-ci · 2019-06-18T08:03:01Z

Build failed
Swift Test Linux Platform
Git Sha - 05a6299

swift-ci · 2019-06-18T08:04:42Z

Build failed
Swift Test OS X Platform
Git Sha - 05a6299

aschwaighofer · 2019-06-18T09:03:34Z

@swift-ci Please test source compatibility

SwiftPM will pass a -runtime-compatibility-version none in this case.

aschwaighofer · 2019-06-18T10:38:13Z

@swift-ci Please test

aschwaighofer · 2019-06-18T10:38:26Z

@swift-ci Please test source compatibility

swift-ci · 2019-06-18T10:40:25Z

Build failed
Swift Test Linux Platform
Git Sha - 443719d

swift-ci · 2019-06-18T10:42:05Z

Build failed
Swift Test OS X Platform
Git Sha - 443719d

aschwaighofer · 2019-06-18T12:26:00Z

@swift-ci Please test linux

jrose-apple · 2019-06-18T23:29:10Z

lib/Driver/DarwinToolChains.cpp

@@ -427,6 +427,22 @@ toolchains::Darwin::constructInvocation(const LinkJobAction &job,
    }
  }

+  if (job.getKind() == LinkKind::Executable) {


Er, you don't need this part anymore, right? And you never wanted it to just be Executables-linked-with-swiftc anyway.

We still ultimately want this to only be linked into executables. We were going to handle that as a followup. For these specific backfill entry points, they're small enough that burning them into dylibs and having potentially-mismatching versions in different images is probably OK, but that's not generally a situation we want to be in.

I don't understand that part. We can't rely on executables having any Swift in them, and it's unlikely that Xcode will switch to linking with swiftc anyway. The autolinking part for dynamic replacement isn't going away.

(I do want a resolution for this because it's interfering with me rebasing #24787.)

If there's any Swift in the process, then the executable is the only useful place to put backward-compatibility fills, so much like arclite, we might want eventually for Xcode to link this in unconditionally because a non-Swift executable could be using dylibs with Swift in them.

This was referenced Jun 14, 2019

DNM: stdlib: Add backward deployment versions for the dynamic-replacement runtime functions. #25340

Closed

DNM: stdlib: Add backward deployment versions for the dynamic-replacement runtime functions - version 2 #25360

Closed

aschwaighofer requested review from jckarter, mikeash and jrose-apple June 14, 2019 17:56

jckarter requested changes Jun 14, 2019

View reviewed changes

jckarter reviewed Jun 14, 2019

View reviewed changes

Use weak_import not weak for the optional symbols

f29b77f

Refactor to use RuntimeAvailability

eb32194

Fix MicroStdlib test

6f9c50c

Really fix micro stdlib test case

da2d418

jckarter requested changes Jun 17, 2019

View reviewed changes

Use thread private key to avoid weak linkage

5d32946

We use one bit of the third reserved swift private tls key. Also move the functionality into a separate static archive that is always linked dependent on deployment target.

jckarter approved these changes Jun 17, 2019

View reviewed changes

jrose-apple reviewed Jun 18, 2019

View reviewed changes

aschwaighofer added 3 commits June 17, 2019 19:47

Fix swiftpm build

d76c814

Address review comments

906f0b4

Another review comment I missed

05a6299

Fix invalid deref

443719d

Fix swiftpm targets without swift

a9c83e7

SwiftPM will pass a -runtime-compatibility-version none in this case.

aschwaighofer merged commit 85bfbe2 into swiftlang:master Jun 18, 2019

jrose-apple reviewed Jun 18, 2019

View reviewed changes

aschwaighofer mentioned this pull request Jul 30, 2019

[SR-11237] Receiving SIGABRT when assigning dynamic property #53638

Closed

stdlib: Add backward deployment versions for the dynamic-replacement runtime functions #25473

stdlib: Add backward deployment versions for the dynamic-replacement runtime functions #25473

Uh oh!

Conversation

aschwaighofer commented Jun 14, 2019

Uh oh!

aschwaighofer commented Jun 14, 2019

Uh oh!

aschwaighofer commented Jun 14, 2019

Uh oh!

jrose-apple commented Jun 14, 2019

Uh oh!

jckarter Jun 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jckarter Jun 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

swift-ci commented Jun 14, 2019

Uh oh!

swift-ci commented Jun 14, 2019

Uh oh!

jckarter commented Jun 14, 2019

Uh oh!

aschwaighofer commented Jun 14, 2019

Uh oh!

swift-ci commented Jun 14, 2019

Uh oh!

swift-ci commented Jun 14, 2019

Uh oh!

aschwaighofer commented Jun 14, 2019

Uh oh!

swift-ci commented Jun 14, 2019

Uh oh!

swift-ci commented Jun 14, 2019

Uh oh!

aschwaighofer commented Jun 15, 2019

Uh oh!

swift-ci commented Jun 15, 2019

Uh oh!

swift-ci commented Jun 15, 2019

Uh oh!

aschwaighofer commented Jun 17, 2019

Uh oh!

jckarter left a comment

Choose a reason for hiding this comment

Uh oh!

jckarter Jun 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aschwaighofer Jun 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jckarter Jun 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aschwaighofer commented Jun 17, 2019

Uh oh!

swift-ci commented Jun 17, 2019

Uh oh!

jckarter left a comment

Choose a reason for hiding this comment

Uh oh!

swift-ci commented Jun 17, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jckarter Jun 14, 2019 •

edited

Loading

jckarter Jun 14, 2019 •

edited

Loading

jckarter Jun 17, 2019 •

edited

Loading

aschwaighofer Jun 17, 2019 •

edited

Loading

jckarter Jun 17, 2019 •

edited

Loading