Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

This driver no longer looks in a passed-in -sdk for the Swift modules and libraries #1562

Open
finagolfin opened this issue Mar 13, 2024 · 2 comments

Comments

@finagolfin
Copy link
Contributor

@compnerd and some Windows devs have been trying out Android cross-compilation from Windows, and in response to some issues they've been seeing, I suggested symlinking the Swift files into the C sysroot in the Android NDK. I then tried it myself on linux, using my Android SDK for Swift for convenience, and found that it works with the legacy C++ Driver, but not this one:

> wget https://github.com/finagolfin/swift-android-sdk/releases/download/5.9.2/swift-5.9.2-android-24-sdk.tar.xz
> tar xf swift-5.9.2-android-24-sdk.tar.xz
> ln -s ~/swift-5.9.2-android-24-sdk/usr/lib/swift android-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/swift
> ./swift-5.9.2-RELEASE-ubi9/usr/bin/swiftc -tools-directory ~/android-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/bin/ -target aarch64-unknown-linux-android24 -sdk android-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/sysroot swift/test/Interpreter/hello_toplevel.swift  -v -L swift-5.9.2-android-24-sdk/usr/lib/aarch64-linux-android/ -disallow-use-new-driver
<unknown>:0: warning: legacy driver is now deprecated; consider avoiding specifying '-disallow-use-new-driver'
Swift version 5.9.2 (swift-5.9.2-RELEASE)
Target: aarch64-unknown-linux-android24
/home/fina/swift-5.9.2-RELEASE-ubi9/usr/bin/swift-frontend -frontend -c -primary-file swift/test/Interpreter/hello_toplevel.swift -target aarch64-unknown-linux-android24 -Xllvm -aarch64-use-tbi -disable-objc-interop -sdk android-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/sysroot -color-diagnostics -tools-directory /home/fina/android-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/bin/ -plugin-path /home/fina/swift-5.9.2-RELEASE-ubi9/usr/lib/swift/host/plugins -plugin-path /home/fina/swift-5.9.2-RELEASE-ubi9/usr/local/lib/swift/host/plugins -module-name hello_toplevel -o /tmp/hello_toplevel-b8d1e8.o
/home/fina/swift-5.9.2-RELEASE-ubi9/usr/bin/swift-autolink-extract /tmp/hello_toplevel-b8d1e8.o -o /tmp/hello_toplevel-9c4d55.autolink
/home/fina/android-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/bin/clang -fuse-ld=lld -Xlinker -z -Xlinker nostart-stop-gc -B /home/fina/android-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/bin/ -pie android-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/swift/android/aarch64/swiftrt.o /tmp/hello_toplevel-b8d1e8.o --sysroot android-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/sysroot @/tmp/hello_toplevel-9c4d55.autolink -L android-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/swift/android -lswiftCore --target=aarch64-unknown-linux-android24 -v -L swift-5.9.2-android-24-sdk/usr/lib/aarch64-linux-android/ -o hello_toplevel
Android (11349228, +pgo, +bolt, +lto, -mlgo, based on r487747e) clang version 17.0.2 (https://android.googlesource.com/toolchain/llvm-project d9f89f4d16663d5012e5c09495f3b30ece3d2362)
Target: aarch64-unknown-linux-android24
Thread model: posix
InstalledDir: /home/fina/android-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/bin
 "/home/fina/android-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/bin/ld.lld" --sysroot=android-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/sysroot -pie -EL --fix-cortex-a53-843419 -z now -z relro -z max-page-size=4096 --hash-style=gnu --eh-frame-hdr -m aarch64linux -dynamic-linker /system/bin/linker64 -o hello_toplevel android-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/aarch64-linux-android/24/crtbegin_dynamic.o -Landroid-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/swift/android -Lswift-5.9.2-android-24-sdk/usr/lib/aarch64-linux-android/ -L/home/fina/android-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/lib/clang/17/lib/linux/aarch64 -Landroid-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/aarch64-linux-android/24 -Landroid-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/aarch64-linux-android -Landroid-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib -z nostart-stop-gc android-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/swift/android/aarch64/swiftrt.o /tmp/hello_toplevel-b8d1e8.o -lswiftSwiftOnoneSupport -lswiftCore -lswift_Concurrency -lswift_StringProcessing -lswift_RegexParser -lswiftCore /home/fina/android-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/lib/clang/17/lib/linux/libclang_rt.builtins-aarch64-android.a -l:libunwind.a -ldl -lc /home/fina/android-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/lib/clang/17/lib/linux/libclang_rt.builtins-aarch64-android.a -l:libunwind.a -ldl android-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/aarch64-linux-android/24/crtend_android.o
> file hello_toplevel
hello_toplevel: ELF 64-bit LSB pie executable, ARM aarch64, version 1 (SYSV), dynamically linked, interpreter /system/bin/linker64, not stripped
> ./swift-5.9.2-RELEASE-ubi9/usr/bin/swiftc -tools-directory ~/android-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/bin/ -target aarch64-unknown-linux-android24 -sdk android-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/sysroot swift/test/Interpreter/hello_toplevel.swift  -v -L swift-5.9.2-android-24-sdk/usr/lib/aarch64-linux-android/
warning: Could not read SDKSettings.json for SDK at: /home/fina/android-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/sysroot
Swift version 5.9.2 (swift-5.9.2-RELEASE)
Target: aarch64-unknown-linux-android24
/home/fina/swift-5.9.2-RELEASE-ubi9/usr/bin/swift-frontend -frontend -c -primary-file swift/test/Interpreter/hello_toplevel.swift -target aarch64-unknown-linux-android24 -Xllvm -aarch64-use-tbi -disable-objc-interop -sdk /home/fina/android-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/sysroot -color-diagnostics -new-driver-path /home/fina/swift-5.9.2-RELEASE-ubi9/usr/bin/swift-driver -empty-abi-descriptor -resource-dir /home/fina/swift-5.9.2-RELEASE-ubi9/usr/lib/swift -module-name hello_toplevel -plugin-path /home/fina/swift-5.9.2-RELEASE-ubi9/usr/lib/swift/host/plugins -plugin-path /home/fina/swift-5.9.2-RELEASE-ubi9/usr/local/lib/swift/host/plugins -o /tmp/TemporaryDirectory.A4x0Xq/hello_toplevel-1.o
/home/fina/swift-5.9.2-RELEASE-ubi9/usr/bin/swift-autolink-extract /tmp/TemporaryDirectory.A4x0Xq/hello_toplevel-1.o -o /tmp/TemporaryDirectory.A4x0Xq/hello_toplevel-2.autolink
/home/fina/android-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/bin/clang -fuse-ld=lld -Xlinker -z -Xlinker nostart-stop-gc -B /home/fina/android-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/bin -pie /home/fina/swift-5.9.2-RELEASE-ubi9/usr/lib/swift/android/aarch64/swiftrt.o /tmp/TemporaryDirectory.A4x0Xq/hello_toplevel-1.o @/tmp/TemporaryDirectory.A4x0Xq/hello_toplevel-2.autolink --sysroot /home/fina/android-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/sysroot -L /home/fina/swift-5.9.2-RELEASE-ubi9/usr/lib/swift/android -lswiftCore --target=aarch64-unknown-linux-android24 -v -L swift-5.9.2-android-24-sdk/usr/lib/aarch64-linux-android -o hello_toplevel
error: link command failed with exit code 1 (use -v to see invocation)
Android (11349228, +pgo, +bolt, +lto, -mlgo, based on r487747e) clang version 17.0.2 (https://android.googlesource.com/toolchain/llvm-project d9f89f4d16663d5012e5c09495f3b30ece3d2362)
Target: aarch64-unknown-linux-android24
Thread model: posix
InstalledDir: /home/fina/android-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/bin
clang-17: error: no such file or directory: '/home/fina/swift-5.9.2-RELEASE-ubi9/usr/lib/swift/android/aarch64/swiftrt.o'
error: fatalError

Note how the old C++ driver correctly passes -Landroid-ndk-r26c/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/swift/android to the linker, while this swift-driver incorrectly passes a non-existent -L /home/fina/swift-5.9.2-RELEASE-ubi9/usr/lib/swift/android to the linker and strangely also -resource-dir /home/fina/swift-5.9.2-RELEASE-ubi9/usr/lib/swift to the frontend.

At the very least, we should resolve this discrepancy between the two drivers, which was probably not noticed because nobody is distributing full Unix cross-compilation SDKs, ie a single platform SDK containing both a C/C++ sysroot and Swift libraries, not bundled with the toolchain.

@finagolfin
Copy link
Contributor Author

Alright, spent a couple hours looking into this, looks to me like a long-standing latent bug in the swift-frontend.

Some background first: the legacy C++ Driver has long had a getResourceDirPath() method, which first checks for an explicitly specified -resource-dir, then looks for usr/lib/swift/ in an explicitly specified -sdk, and finally, if neither flag was specified, just looks for one next to the compiler, ie usr/bin/swiftc/../../lib/swift/. All the toolchains in the old C++ driver appear to use this core logic, and that's why the legacy C++ Driver command above works.

This new swift-driver has no such method and simply delegates finding the Swift resource directory to the swift-frontend's -print-target-info option. However, the swift-frontend only checks for an explicit -resource-dir then looks for one next to the compiler, ie it doesn't look for a Swift resource directory in the -sdk like the old C++ Driver does.

I was surprised that the swift-frontend never looked in a passed-in -sdk for Swift resources, but the following commands convinced me that is indeed the case. First, I tried to use the latest trunk toolchain on linux with the legacy C++ Driver to build with an -sdk that contains 5.10 resource files, which should fail when compiling the object file but doesn't:

> ./swift-DEVELOPMENT-SNAPSHOT-2024-03-13-a-ubi9/usr/bin/swiftc -v swift/test/Interpreter/hello_toplevel.swift -sdk swift-5.10-RELEASE-ubi9/ -disallow-use-new-driver
<unknown>:0: warning: legacy driver is now deprecated; consider avoiding specifying '-disallow-use-new-driver'
Swift version 6.0-dev (LLVM d1625da873daa4c, Swift bae6450bf96dceb)
Target: x86_64-unknown-linux-gnu
/home/fina/swift-DEVELOPMENT-SNAPSHOT-2024-03-13-a-ubi9/usr/bin/swift-frontend -frontend -c -primary-file swift/test/Interpreter/hello_toplevel.swift -target x86_64-unknown-linux-gnu -disable-objc-interop -sdk swift-5.10-RELEASE-ubi9 -color-diagnostics -plugin-path /home/fina/swift-DEVELOPMENT-SNAPSHOT-2024-03-13-a-ubi9/usr/lib/swift/host/plugins -plugin-path /home/fina/swift-DEVELOPMENT-SNAPSHOT-2024-03-13-a-ubi9/usr/local/lib/swift/host/plugins -module-name hello_toplevel -o /tmp/hello_toplevel-7b72d3.o
<unknown>:0: warning: libc not found for 'x86_64-unknown-linux-gnu'; C stdlib may be unavailable
/home/fina/swift-DEVELOPMENT-SNAPSHOT-2024-03-13-a-ubi9/usr/bin/swift-autolink-extract /tmp/hello_toplevel-7b72d3.o -o /tmp/hello_toplevel-739e5c.autolink
/home/fina/swift-DEVELOPMENT-SNAPSHOT-2024-03-13-a-ubi9/usr/bin/clang -fuse-ld=gold -pie -Xlinker -rpath -Xlinker swift-5.10-RELEASE-ubi9/usr/lib/swift/linux swift-5.10-RELEASE-ubi9/usr/lib/swift/linux/x86_64/swiftrt.o /tmp/hello_toplevel-7b72d3.o --sysroot swift-5.10-RELEASE-ubi9 @/tmp/hello_toplevel-739e5c.autolink -L swift-5.10-RELEASE-ubi9/usr/lib/swift/linux -lswiftCore --target=x86_64-unknown-linux-gnu -v -o hello_toplevel

It gets to the link step before failing, this suggests the swift-frontend compilation step is not using the Swift 5.10 resource directory. If I specify the 5.10 -resource-dir explicitly, it fails when compiling as expected:

> ./swift-DEVELOPMENT-SNAPSHOT-2024-03-13-a-ubi9/usr/bin/swiftc -v swift/test/Interpreter/hello_toplevel.swift -sdk swift-5.10-RELEASE-ubi9/ -disallow-use-new-driver -resource-dir swift-5.10-RELEASE-ubi9/usr/lib/swift
<unknown>:0: warning: legacy driver is now deprecated; consider avoiding specifying '-disallow-use-new-driver'
Swift version 6.0-dev (LLVM d1625da873daa4c, Swift bae6450bf96dceb)
Target: x86_64-unknown-linux-gnu
/home/fina/swift-DEVELOPMENT-SNAPSHOT-2024-03-13-a-ubi9/usr/bin/swift-frontend -frontend -c -primary-file swift/test/Interpreter/hello_toplevel.swift -target x86_64-unknown-linux-gnu -disable-objc-interop -sdk swift-5.10-RELEASE-ubi9 -color-diagnostics -resource-dir swift-5.10-RELEASE-ubi9/usr/lib/swift -plugin-path /home/fina/swift-DEVELOPMENT-SNAPSHOT-2024-03-13-a-ubi9/usr/lib/swift/host/plugins -plugin-path /home/fina/swift-DEVELOPMENT-SNAPSHOT-2024-03-13-a-ubi9/usr/local/lib/swift/host/plugins -module-name hello_toplevel -o /tmp/hello_toplevel-dcf2f5.o
<unknown>:0: warning: libc not found for 'x86_64-unknown-linux-gnu'; C stdlib may be unavailable
<unknown>:0: error: module compiled with Swift 5.10 cannot be imported by the Swift 6.0 compiler: swift-5.10-RELEASE-ubi9/usr/lib/swift/linux/Swift.swiftmodule/x86_64-unknown-linux-gnu.swiftmodule

To hammer the point home, let's use the new swift-driver, which I noted above always passes a -resource-dir to the swift-frontend, regardless if one was passed in to the new swift-driver or not:

> ./swift-DEVELOPMENT-SNAPSHOT-2024-03-13-a-ubi9/usr/bin/swiftc -v swift/test/Interpreter/hello_toplevel.swift -sdk swift-5.10-RELEASE-ubi9/            warning: Could not read SDKSettings.json for SDK at: /home/fina/swift-5.10-RELEASE-ubi9
Swift version 6.0-dev (LLVM d1625da873daa4c, Swift bae6450bf96dceb)
Target: x86_64-unknown-linux-gnu
/home/fina/swift-DEVELOPMENT-SNAPSHOT-2024-03-13-a-ubi9/usr/bin/swift-frontend -frontend -c -primary-file swift/test/Interpreter/hello_toplevel.swift -target x86_64-unknown-linux-gnu -disable-objc-interop -sdk /home/fina/swift-5.10-RELEASE-ubi9 -color-diagnostics -empty-abi-descriptor -resource-dir /home/fina/swift-DEVELOPMENT-SNAPSHOT-2024-03-13-a-ubi9/usr/lib/swift -module-name hello_toplevel -plugin-path /home/fina/swift-DEVELOPMENT-SNAPSHOT-2024-03-13-a-ubi9/usr/lib/swift/host/plugins -plugin-path /home/fina/swift-DEVELOPMENT-SNAPSHOT-2024-03-13-a-ubi9/usr/local/lib/swift/host/plugins -o /tmp/TemporaryDirectory.5mibhA/hello_toplevel-1.o
<unknown>:0: warning: libc not found for 'x86_64-unknown-linux-gnu'; C stdlib may be unavailable
/home/fina/swift-DEVELOPMENT-SNAPSHOT-2024-03-13-a-ubi9/usr/bin/swift-autolink-extract /tmp/TemporaryDirectory.5mibhA/hello_toplevel-1.o -o /tmp/TemporaryDirectory.5mibhA/hello_toplevel-2.autolink
/home/fina/swift-DEVELOPMENT-SNAPSHOT-2024-03-13-a-ubi9/usr/bin/clang -fuse-ld=gold -pie -Xlinker -rpath -Xlinker /home/fina/swift-DEVELOPMENT-SNAPSHOT-2024-03-13-a-ubi9/usr/lib/swift/linux /home/fina/swift-DEVELOPMENT-SNAPSHOT-2024-03-13-a-ubi9/usr/lib/swift/linux/x86_64/swiftrt.o /tmp/TemporaryDirectory.5mibhA/hello_toplevel-1.o @/tmp/TemporaryDirectory.5mibhA/hello_toplevel-2.autolink --sysroot /home/fina/swift-5.10-RELEASE-ubi9 -L /home/fina/swift-DEVELOPMENT-SNAPSHOT-2024-03-13-a-ubi9/usr/lib/swift/linux -lswiftCore --target=x86_64-unknown-linux-gnu -v -o hello_toplevel

This passes the swift-frontend compilation step and we can see it is not looking in the -sdk for the resource directory, passing in -resource-dir /home/fina/swift-DEVELOPMENT-SNAPSHOT-2024-03-13-a-ubi9/usr/lib/swift to the swift-frontend instead. If I specify the 5.10 resource directory explicitly, it fails as expected:

> ./swift-DEVELOPMENT-SNAPSHOT-2024-03-13-a-ubi9/usr/bin/swiftc -v swift/test/Interpreter/hello_toplevel.swift -sdk swift-5.10-RELEASE-ubi9/ -resource-dir swift-5.10-RELEASE-ubi9/usr/lib/swift
warning: Could not read SDKSettings.json for SDK at: /home/fina/swift-5.10-RELEASE-ubi9
Swift version 6.0-dev (LLVM d1625da873daa4c, Swift bae6450bf96dceb)
Target: x86_64-unknown-linux-gnu
/home/fina/swift-DEVELOPMENT-SNAPSHOT-2024-03-13-a-ubi9/usr/bin/swift-frontend -frontend -c -primary-file swift/test/Interpreter/hello_toplevel.swift -target x86_64-unknown-linux-gnu -disable-objc-interop -sdk /home/fina/swift-5.10-RELEASE-ubi9 -color-diagnostics -empty-abi-descriptor -resource-dir swift-5.10-RELEASE-ubi9/usr/lib/swift -module-name hello_toplevel -plugin-path /home/fina/swift-DEVELOPMENT-SNAPSHOT-2024-03-13-a-ubi9/usr/lib/swift/host/plugins -plugin-path /home/fina/swift-DEVELOPMENT-SNAPSHOT-2024-03-13-a-ubi9/usr/local/lib/swift/host/plugins -o /tmp/TemporaryDirectory.ZDGmZ2/hello_toplevel-1.o
<unknown>:0: warning: libc not found for 'x86_64-unknown-linux-gnu'; C stdlib may be unavailable
<unknown>:0: error: module compiled with Swift 5.10 cannot be imported by the Swift 6.0 compiler: swift-5.10-RELEASE-ubi9/usr/lib/swift/linux/Swift.swiftmodule/x86_64-unknown-linux-gnu.swiftmodule

To sum up, the old C++ Driver always looked in -sdk for a Swift resource directory, but the swift-frontend and now the new swift-driver don't. That appears to be a bug that hasn't been noticed, perhaps because almost nobody is shipping full platform SDKs that aren't situated next to the compiler, ie a separate platform sysroot containing both the platform C headers/libraries and Swift resource files. I suppose it's also possible we're always explicitly specifying the -resource-dir, so this issue wasn't noticed.

Now that we're starting to ship cross-compilation SDK bundles that may have such full platform SDKs, we should close this hole up. It will need to be fixed in the swift-frontend or at least overridden in this new swift-driver.

Pinging @artemcm and @DougGregor on how I should fix this, since you guys wrote some of the relevant logic, and @compnerd and @MaxDesiatov, since this fix will affect your work on Windows and SDK bundles. Since this bug was never noticed this long, I figure the fix won't break anything, but this will affect all toolchains, so I want to run it by you all.

@artemcm
Copy link
Contributor

artemcm commented Mar 19, 2024

Thanks for the investigation @finagolfin.
It seems to me that fixing this ought to be done in the frontend's print-target-info logic, since the driver fully delegates to it and then relies on the resulting target info as the source of truth all over the driver logic.

Moreover, it would be tough to implement this logic in the new driver since the target info is queried when the target triple is not known to the driver yet, relying on the target info query to yield the default host triple when one is not specified to the compilation.

Potentially something like this could work, but I will need to do more testing on it:
apple/swift#72409

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants