Versioning & back deployment of isolated deinit

Nickolas_Pohilets · November 18, 2024, 12:15pm

PR that was making isolated deinit non-experiment, and which I hoped will be the last one, got reverted, because tests were failing on iOS device.

Which I strongly suspect is just a matter of correct versioning.

Tests pass on simulator, because SIMCTL_CHILD_DYLD_LIBRARY_PATH is set when running executable tests, and it points to updated stdlib which contains the new runtime function.

/usr/bin/env SIMCTL_CHILD_DYLD_LIBRARY_PATH=$SWIFT_PROJECT_DIR/build/Ninja-RelWithDebInfoAssert/swift-macosx-arm64/lib/swift/iphonesimulator xcrun --toolchain default --sdk /Applications/Xcode-16.1.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator18.1.sdk simctl spawn --standalone 'iPhone 15' $SWIFT_PROJECT_DIR/build/Ninja-RelWithDebInfoAssert/swift-macosx-arm64/test-iphonesimulator-arm64/Concurrency/Runtime/Output/actor_recursive_deinit.swift.tmp/a.out

But when running on a physical device, IIUC, program is executed against stdlib shipped on the device, which does not have the required runtime function, causing app to crash either during dynamic linking or when trying to call unresolved function pointer.

To fix this in the short term, we need make sure that isolated deinit as a feature is limited only to specific runtime versions and does not produce binaries that use the feature but can target earlier runtime versions.

This needs to be done in at least 3 places:

In Swift declaration of the _deinitOnExecutor() - should it be @available(SwiftStdlib 6.1, *)? Does it actually do anything, if the call to this function is generated by the compiler in the SILGen phase? Are there versioning checks later in the pipeline that care about this?
In frontend - instead of feature check, it should be a version check. But IIUC, user code cannot check against SwiftStdlib, it should be a check against iOS/macOS/tvOS/xrOS version, right? Where is mapping of stdlib version to OS version maintained?
In unit tests - ideally we should skip relevant tests on iOS devices with old runtime, but run them on devices with new one. Is there an existing LIT feature that can be used for this?

In the long-term, we can consider back-deployment of feature.

I was under impression that COMPATIBILITY_OVERRIDE and friends are responsible exactly for back-deployment of C++ code of the stdlib. Comment in CompatibilityOverride.h says:

// The compatibility override system supports the back-deployment of
// bug fixes and ABI additions to existing Swift systems.

Is it supposed to work for this case of "ABI addition" or not?

I've tried to look into some examples of back-deployment of the C++ code, and I've found swift_getFunctionTypeMetadataGlobalActorBackDeploy , which seems to be a relevant example, but I don't fully understand where the actually back-deployment implementation is coming from. Is swift_getFunctionTypeMetadataGlobalActorStandalone a back-deployed version, or a symbol used for embedded builds? Where is it implemented?

Do I understand correctly that back-deployed implementation cannot depend on private API (DefaultActorImpl, ExecutorTrackingInfo), and should only use public ABI-stable API?

If so, back-deployed implementation still should be possible with certain limitations:

No resetting TLs for the fast path
No optimization for isolated deinit of default actors

I think it still would be valuable for many users. Is it acceptable for back-deployed version to diverge from the spec approved by the proposal review?

John_McCall · November 18, 2024, 11:05pm

It won't cause any diagnostics, no. It might fix some linking issues, though.

ASTContext::getSwiftAvailability, informed by a .def file. But the real answer is that ASTContext::get##YourFeature##Availability() already handles this mapping for you.

You can use -target %target-swift-6.1-abi-triple.

Overrides are required when you need to change the behavior of existing runtimes. When it's a purely additive new feature, that's unnecessary; you're mostly just shipping a dylib with the runtime support in it. Adding such a dylib to the toolchain requires a pretty serious investment of resources from Apple, though, and it's not something we've generally done for features like this.

ktoso · November 18, 2024, 11:30pm

Thanks for chiming in, John.

We should also clarify at this point probably that this won't ship as stable in 6.1, it's too late and the core team has indicated we're nervous about this feature and will need to give it more time to iron out the problems, so we'll stabilize it only in the next feature release it seems.

Nickolas_Pohilets · November 19, 2024, 7:07pm

For some reason clang importer is not happy about it:

Summary

<unknown>:0: error: invalid version number in '-target arm64-apple-ios9999-simulator'
Assertion failed: (OsVersion < VersionTuple(100) && "Invalid version!"), function getDarwinDefines, file OSTargets.cpp, line 78.
Please submit a bug report (https://swift.org/contributing/#reporting-bugs) and include the crash backtrace.
Stack dump:
0.	Program arguments: $SWIFT_PROJECT_DIR/build/Ninja-RelWithDebInfoAssert/swift-macosx-arm64/bin/swift-frontend -frontend -c -primary-file $SWIFT_PROJECT_DIR/swift/test/Concurrency/Runtime/actor_deinit_escaping_self.swift -target arm64-apple-ios9999-simulator -Xllvm -aarch64-use-tbi -enable-objc-interop -sdk /Applications/Xcode-16.1.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator18.1.sdk -F /Applications/Xcode-16.1.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/Library/Frameworks -module-cache-path $SWIFT_PROJECT_DIR/build/Ninja-RelWithDebInfoAssert/swift-macosx-arm64/swift-test-results/arm64-apple-ios16.0-simulator/clang-module-cache -swift-version 4 -define-availability "SwiftStdlib 9999:macOS 9999, iOS 9999, watchOS 9999, tvOS 9999" -define-availability "SwiftStdlib 5.0:macOS 10.14.4, iOS 12.2, watchOS 5.2, tvOS 12.2" -define-availability "SwiftStdlib 5.1:macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0" -define-availability "SwiftStdlib 5.2:macOS 10.15.4, iOS 13.4, watchOS 6.2, tvOS 13.4" -define-availability "SwiftStdlib 5.3:macOS 11.0, iOS 14.0, watchOS 7.0, tvOS 14.0" -define-availability "SwiftStdlib 5.4:macOS 11.3, iOS 14.5, watchOS 7.4, tvOS 14.5" -define-availability "SwiftStdlib 5.5:macOS 12.0, iOS 15.0, watchOS 8.0, tvOS 15.0" -define-availability "SwiftStdlib 5.6:macOS 12.3, iOS 15.4, watchOS 8.5, tvOS 15.4" -define-availability "SwiftStdlib 5.7:macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0" -define-availability "SwiftStdlib 5.8:macOS 13.3, iOS 16.4, watchOS 9.4, tvOS 16.4" -define-availability "SwiftStdlib 5.9:macOS 14.0, iOS 17.0, watchOS 10.0, tvOS 17.0" -define-availability "SwiftStdlib 5.10:macOS 14.4, iOS 17.4, watchOS 10.4, tvOS 17.4, visionOS 1.1" -define-availability "SwiftStdlib 6.0:macOS 15.0, iOS 18.0, watchOS 11.0, tvOS 18.0, visionOS 2.0" -define-availability "SwiftStdlib 6.1:macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, visionOS 9999" -external-plugin-path /Applications/Xcode-16.1.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator18.1.sdk/usr/lib/swift/host/plugins#/Applications/Xcode-16.1.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator18.1.sdk/usr/bin/swift-plugin-server -external-plugin-path /Applications/Xcode-16.1.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator18.1.sdk/usr/local/lib/swift/host/plugins#/Applications/Xcode-16.1.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator18.1.sdk/usr/bin/swift-plugin-server -external-plugin-path /Applications/Xcode-16.1.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/usr/lib/swift/host/plugins#/Applications/Xcode-16.1.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/usr/bin/swift-plugin-server -external-plugin-path /Applications/Xcode-16.1.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/usr/local/lib/swift/host/plugins#/Applications/Xcode-16.1.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/usr/bin/swift-plugin-server -in-process-plugin-server-path $SWIFT_PROJECT_DIR/build/Ninja-RelWithDebInfoAssert/swift-macosx-arm64/lib/swift/host/libSwiftInProcPluginServer.dylib -plugin-path $SWIFT_PROJECT_DIR/build/Ninja-RelWithDebInfoAssert/swift-macosx-arm64/lib/swift/host/plugins -plugin-path $SWIFT_PROJECT_DIR/build/Ninja-RelWithDebInfoAssert/swift-macosx-arm64/local/lib/swift/host/plugins -target-sdk-version 18.1 -dwarf-version=5 -parse-as-library -module-name main -o /var/folders/f6/qllm_pk56m56tdvyx0g_4d3m0000gp/T/lit-tmp-vgle5uwg/actor_deinit_escaping_self-31f1a3.o
1.	Apple Swift version 6.1-dev (LLVM fcc20a24e57c484, Swift 42eb48dd4c6c73f)
2.	Compiling with effective version 4.1.50
 #0 0x0000000109f3292c llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) ($SWIFT_PROJECT_DIR/build/Ninja-RelWithDebInfoAssert/swift-macosx-arm64/bin/swift-frontend+0x1058a292c)
 #1 0x0000000109f30ee8 llvm::sys::RunSignalHandlers() ($SWIFT_PROJECT_DIR/build/Ninja-RelWithDebInfoAssert/swift-macosx-arm64/bin/swift-frontend+0x1058a0ee8)
 #2 0x0000000109f32f70 SignalHandler(int) ($SWIFT_PROJECT_DIR/build/Ninja-RelWithDebInfoAssert/swift-macosx-arm64/bin/swift-frontend+0x1058a2f70)
 #3 0x0000000185ef2584 (/usr/lib/system/libsystem_platform.dylib+0x18047a584)
 #4 0x0000000185ec1c20 (/usr/lib/system/libsystem_pthread.dylib+0x180449c20)
 #5 0x0000000185dcea30 (/usr/lib/system/libsystem_c.dylib+0x180356a30)
 #6 0x0000000185dcdd20 (/usr/lib/system/libsystem_c.dylib+0x180355d20)
 #7 0x000000010a12cb24 clang::targets::PPCTargetInfo::~PPCTargetInfo() ($SWIFT_PROJECT_DIR/build/Ninja-RelWithDebInfoAssert/swift-macosx-arm64/bin/swift-frontend+0x105a9cb24)
 #8 0x00000001092dd620 clang::targets::getDarwinDefines(clang::MacroBuilder&, clang::LangOptions const&, llvm::Triple const&, llvm::StringRef&, llvm::VersionTuple&) ($SWIFT_PROJECT_DIR/build/Ninja-RelWithDebInfoAssert/swift-macosx-arm64/bin/swift-frontend+0x104c4d620)
 #9 0x00000001092cb810 clang::targets::DarwinAArch64TargetInfo::getOSDefines(clang::LangOptions const&, llvm::Triple const&, clang::MacroBuilder&) const ($SWIFT_PROJECT_DIR/build/Ninja-RelWithDebInfoAssert/swift-macosx-arm64/bin/swift-frontend+0x104c3b810)
#10 0x0000000108012984 InitializePredefinedMacros(clang::TargetInfo const&, clang::LangOptions const&, clang::FrontendOptions const&, clang::PreprocessorOptions const&, clang::MacroBuilder&) ($SWIFT_PROJECT_DIR/build/Ninja-RelWithDebInfoAssert/swift-macosx-arm64/bin/swift-frontend+0x103982984)
#11 0x000000010800c53c clang::InitializePreprocessor(clang::Preprocessor&, clang::PreprocessorOptions const&, clang::PCHContainerReader const&, clang::FrontendOptions const&, clang::CodeGenOptions const&) ($SWIFT_PROJECT_DIR/build/Ninja-RelWithDebInfoAssert/swift-macosx-arm64/bin/swift-frontend+0x10397c53c)
#12 0x0000000107f662bc clang::CompilerInstance::createPreprocessor(clang::TranslationUnitKind) ($SWIFT_PROJECT_DIR/build/Ninja-RelWithDebInfoAssert/swift-macosx-arm64/bin/swift-frontend+0x1038d62bc)
#13 0x0000000107fd7590 clang::FrontendAction::BeginSourceFile(clang::CompilerInstance&, clang::FrontendInputFile const&) ($SWIFT_PROJECT_DIR/build/Ninja-RelWithDebInfoAssert/swift-macosx-arm64/bin/swift-frontend+0x103947590)
#14 0x0000000105bd5bfc swift::ClangImporter::create(swift::ASTContext&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, swift::DependencyTracker*, swift::DWARFImporterDelegate*, bool) ($SWIFT_PROJECT_DIR/build/Ninja-RelWithDebInfoAssert/swift-macosx-arm64/bin/swift-frontend+0x101545bfc)
#15 0x0000000104b7f494 swift::CompilerInstance::setUpModuleLoaders() ($SWIFT_PROJECT_DIR/build/Ninja-RelWithDebInfoAssert/swift-macosx-arm64/bin/swift-frontend+0x1004ef494)
#16 0x0000000104b7f224 swift::CompilerInstance::setUpASTContextIfNeeded() ($SWIFT_PROJECT_DIR/build/Ninja-RelWithDebInfoAssert/swift-macosx-arm64/bin/swift-frontend+0x1004ef224)
#17 0x0000000104b80ea4 swift::CompilerInstance::setup(swift::CompilerInvocation const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>&, llvm::ArrayRef<char const*>) ($SWIFT_PROJECT_DIR/build/Ninja-RelWithDebInfoAssert/swift-macosx-arm64/bin/swift-frontend+0x1004f0ea4)
#18 0x00000001048e4b48 swift::performFrontend(llvm::ArrayRef<char const*>, char const*, void*, swift::FrontendObserver*) ($SWIFT_PROJECT_DIR/build/Ninja-RelWithDebInfoAssert/swift-macosx-arm64/bin/swift-frontend+0x100254b48)
#19 0x00000001046c6500 swift::mainEntry(int, char const**) ($SWIFT_PROJECT_DIR/build/Ninja-RelWithDebInfoAssert/swift-macosx-arm64/bin/swift-frontend+0x100036500)
#20 0x0000000185b37154 
<unknown>:0: error: unable to execute command: Abort trap: 6
<unknown>:0: error: compile command failed due to signal 6 (use -v to see invocation)

My WIP - Handle versioning of the IsolatedDeinit feature · swiftlang/swift@f2e904d · GitHub

Could you pls elaborate?

John_McCall · November 20, 2024, 3:13am

Hah. Well, clang should definitely not be asserting just because somebody passed it a surprising target triple. @drexin, is this related to why you made that "future OS" change to 99.99 somewhere else? We should really just fix this in LLVM.

If the function isn't declared as availability-limited, Swift will probably link strongly against it, and then the image might not load if the symbol isn't available even if the code that would use the symbol is never executed.

tshortli · November 20, 2024, 7:40am

For now, you can use %target-future-triple in the tests. You'd need to do that anyways since this feature now targets the release after 6.1 which hasn't been added to the branch yet. I do agree with John that we should lift the restriction in LLVM though.

John_McCall · November 20, 2024, 3:55pm

Can we have these lit substitutions expand to that for Swift versions we haven’t locked to OS releases yet? It’d be nice if people landing evolution-approved features could go ahead and write code and tests for the upcoming Swift release without specifically needing to come back and update all that after Apple ties that into an OS release. In fact, FeatureAvailability.def has several features marked as future that have already been released.

In Nickolas’s case, that would be 6.2, though.

tshortli · November 20, 2024, 4:28pm

I agree that should be the goal but I'm not sure yet what the best way to achieve it is. Unfortunately, there's a disagreement between the stdlib availability checker helpers and the runtime about what the triple for "future" ought to be. The stdlib wants it to be 9999, while the runtime currently requires it to be 99.99. These are going to need to be brought into alignment in order for executable tests of not-yet-released features to work holistically when they include availability checks and attributes written in source. I'll figure something out, it's just not quite as straightforward as it may appear.

Nickolas_Pohilets · November 21, 2024, 12:17pm

Do I understand correctly that 6.1 release will be cut off from the version that already contains _deinitOnExecutor() in the stdlib? Then, assuming that there are no bugs in the stdlib implementation that require patching, we can release version 6.2 of the compiler where feature is non-experimental and allows to target 6.1 stdlib. Right?

John_McCall · November 21, 2024, 6:09pm

The library/runtime support for this feature is the key for the deployment restriction, so if that's already successfully landed and it's just the compiler support that's been difficult, the correct target requirement is 6.1, yeah.