swiftlang / swift

The Swift Programming Language
https://swift.org
Apache License 2.0
66.76k stars 10.29k forks source link

ClangImporter looks in the wrong path for the new C++ module map when `-sdk` is specified, possibly the libc module map also #74696

Open finagolfin opened 1 week ago

finagolfin commented 1 week ago

Description

I was just building the latest June 13 trunk source snapshot natively on Android in the Termux app, when I ran into a compilation error while trying to build CxxStdlib for the first time, which was only recently enabled for Android too, 273edcf2a.

For some background, the Termux app has an Android C/C++ sysroot installed at /data/data/com.termux/files/, ie C/C++ header files are found in /data/data/com.termux/files/usr/include/, and I have a Swift 5.10.1 toolchain installed there too, so the Swift modules can also be found in /data/data/com.termux/files/usr/lib/swift/android/. This constitutes a full Swift 5.10.1 SDK, as the C/C++ headers/libraries and Swift modules/libraries are all together in a single sysroot.

Well, this caused problems, as the build of the trunk 6.1 Swift toolchain was picking up the wrong libcxxshim.modulemap, ie the one from the full Swift 5.10.1 SDK not the newly generated one in build/Ninja-Release/swift-android-aarch64/, which caused a compilation error as one of those shim headers was recently updated. I worked around this bug by removing that C++ interop module map from the 5.10.1 SDK.

Since there was nothing specific to Android here, I just tried to reproduce this issue in Fedora 40 x86_64. However, it turns out Fedora 40 is still installing Swift 5.8.1 in the system, so I could not reproduce, as libcxxshim.modulemap was installed in usr/lib/swift/linux/x86_64/ back then while a more recent 5.10 toolchain looks in usr/lib/swift/linux/ for that C++ module map:

> dnf repoquery -l swift-lang | ag "lib/swift/.*cxx.*modulemap"
/usr/libexec/swift/5.8.1/lib/swift/linux/x86_64/libcxxshim.modulemap
/usr/libexec/swift/5.8.1/lib/swift/linux/x86_64/libstdcxx.modulemap
> ls -l /usr/lib/swift
lrwxrwxrwx. 1 root root 34 Feb 16 00:00 /usr/lib/swift -> /usr/libexec/swift/5.8.1/lib/swift

So I simply moved those shims over to test for this bug, mv /usr/lib/swift/linux/x86_64/lib* /usr/lib/swift/linux/, and found that I could reproduce on linux also:

/home/finagolfin/swift-5.10.1-RELEASE-fedora39/usr/bin/swift-frontend -frontend -c -primary-file swift/test/Interpreter/hello_toplevel.swift -target x86_64-unknown-linux-gnu -disable-objc-interop -cxx-interoperability-mode=default -sdk / -color-diagnostics -new-driver-path /home/finagolfin/swift-5.10.1-RELEASE-fedora39/usr/bin/swift-driver -dump-clang-diagnostics -empty-abi-descriptor -resource-dir /home/finagolfin/swift-5.10.1-RELEASE-fedora39/usr/lib/swift -module-name hello_toplevel -plugin-path /home/finagolfin/swift-5.10.1-RELEASE-fedora39/usr/lib/swift/host/plugins -plugin-path /home/finagolfin/swift-5.10.1-RELEASE-fedora39/usr/local/lib/swift/host/plugins -o /tmp/TemporaryDirectory.hfI2hS/hello_toplevel-1.o
'/home/finagolfin/swift-5.10.1-RELEASE-fedora39/usr/bin/clang' '-fsyntax-only' '-fblocks' '-D__swift__=51001' '-fretain-comments-from-system-headers' '-isystem' '/home/finagolfin/swift-5.10.1-RELEASE-fedora39/usr/lib/swift' '-fPIC' '-fmodules' '-Xclang' '-fmodule-feature' '-Xclang' 'swift' '-x' 'c++' '-std=gnu++14' '-fmodule-map-file=/usr/lib/swift/linux/libcxxshim.modulemap' '--sysroot' '/' '-fmodules-validate-system-headers' '-Xclang' '-fmodule-format=obj' '-fapinotes-modules' '-fapinotes-swift-version=5' '-iapinotes-modules' '/home/finagolfin/swift-5.10.1-RELEASE-fedora39/usr/lib/swift/apinotes' '-target' 'x86_64-unknown-linux-gnu' '<swift-imported-modules>' '-mcx16' '-resource-dir' '/home/finagolfin/swift-5.10.1-RELEASE-fedora39/usr/lib/swift/clang' '-fansi-escape-codes' '-Xclang' '-opaque-pointers' '-working-directory' '/home/finagolfin'

Note the wrong 5.8.1 C++ module map used in the above output from the 5.10.1 repro command below, -fmodule-map-file=/usr/lib/swift/linux/libcxxshim.modulemap, despite the frontend explicitly specifying the right resource directory, -resource-dir /home/finagolfin/swift-5.10.1-RELEASE-fedora39/usr/lib/swift.

I just checked and the Fedora system package was finally updated to 5.10.1 last week, with the Fedora 40 build currently undergoing testing before deployment, so this bug will start hitting Fedora installs soon. It only happens when the -sdk / flag is explicitly passed in, which SwiftPM doesn't appear to for normal package builds, but the new SDK bundles and building a new stdlib as part of a fresh toolchain build do pass in. @tachoknight, perhaps you can run the repro command below on Fedora 41 with your new 5.10.1 package installed and confirm.

The issue is the getActualModuleMap() function in ClangImporter, which first looks in -sdk for these module maps, then in -resource-dir. That is not the precedence used everywhere else in the compiler, ie -resource-dir is always given precedence normally.

With more distros starting to package Swift in the system and SDK bundles starting to be used, this bug will start hitting more people.

Reproduction

./swift-5.10.1-RELEASE-fedora39/usr/bin/swiftc swift/test/Interpreter/hello_toplevel.swift -v -Xfrontend -dump-clang-diagnostics -cxx-interoperability-mode=default -sdk /

Only the -cxx-interoperability-mode=default -sdk / flags and an installed system toolchain are required, the other flags simply dump verbose output to show what's going on.

Expected behavior

-resource-dir is always given precedence in the compiler for other files, but not for these module maps. The compiler might also be using the wrong glibc.modulemap and libstdcxx.modulemap, but it simply does not dump those paths in the verbose output. Whatever happens with this bug, the compiler should be modified to dump those paths in verbose mode, so we can easily check them.

Environment

I tested with Swift 5.10, but the code goes back to 5.8.

Additional information

I should also note that a new cross-compilation model is planned. In that new scenario, it depends if these module maps are considered compiler resources or "Platform Swift Modules." If the former, they should then only be looked for in -resource-dir at that point, and if the latter, only in -sdk, ie not looked for in both as getActualModuleMap() currently does.

Pinging @zoecarver, who originally wrote this code in #59754 years ago, and @egorzhdan, who subsequently moved some of it around. @compnerd may want to chime in on whether these C++ module maps are compiler resources and how they should be handled under his new cross-compilation model.

compnerd commented 1 week ago

The C++ module maps are not compiler resources, but are part of the Swift SDK ("Platform Swift Modules"). They are auxiliary content that is overlaid on the Platform SDK. We wouldn't want to treat this as a compiler resource as we would be unable to release a SDK update to fix modularisation issues without re-rolling the full compiler.

The C++ modulemaps should be pulled from -sdk, and -resource-dir is only honoured as a compatibility path.

egorzhdan commented 1 week ago

I'm not an expert in Android, so I'm not sure what is the right solution here. The Cxx and CxxStdlib modules were intentionally moved out of the SDK and into the Swift toolchain, because they are version-locked to the Swift compiler. Those modules are evolving quickly along with the compiler support for C++ interoperability, and are tightly integrated with the compiler logic that auto-conforms certain C++ types to Swift protocols like RandomAccessCollection. Eventually we would like to move Cxx and CxxStdlib back into the SDK, but it's too early for that now – we're planning a few changes that would break compatibility of e.g. old Cxx & CxxStdlib binaries with a new Swift compiler.

compnerd commented 1 week ago

@egorzhdan right, there's no ABI stability currently for that module. But the final place once that is achieved would be the SDK. The module maps however do not have any real association with the compiler version. Those are associated with the platform and being provided by Swift, seems proper to be in the Swift SDK.

finagolfin commented 1 week ago

The C++ modulemaps should be pulled from -sdk, and -resource-dir is only honoured as a compatibility path.

@compnerd, that's fine if you make that change once you start changing what -resource-dir means, but these two flags have different meanings right now. -sdk is where the Swift compiler looks for the C/C++ headers and libraries and -resource-dir is where it looks for the "Platform Swift Modules." This SDK module map code in getActualModuleMap() is an exception to that, by looking in -sdk first instead, even when a separate -resource-dir is specified.

We can switch to your compatibility path plan later, whenever you make those changes to the meaning of -resource-dir.

I'm not an expert in Android, so I'm not sure what is the right solution here.

@egorzhdan, this bug has nothing to do with Android- as I stated above, "there was nothing specific to Android here"- so my repro command and output above is on Fedora linux compiling natively for linux x86_64, while this C++ shim module map appears to be used on Darwin also.

To explain the problem succinctly without all the repro detail above, non-Darwin platforms generally do not have full SDKs, ie a path containing both the C/C++ headers and Swift modules, so one specifies both -sdk and -resource-dir for each with separate paths.

However, some linux distros and the Termux app do provide Swift toolchain packages, so in that case, they do have a full SDK, but often with older Swift modules and runtime libraries. In such a scenario, care must be taken to always give -resource-dir precedence, otherwise you may end up with old Swift files from the full SDK being used instead of newer Swift files from the -resource-dir. For example, the Swift frontend always looks for runtime libraries in the RuntimeResourcePath set by -resource-dir first, before checking the SDKPath set by -sdk also as a backup.

Similarly, the getActualModuleMap() function should be reordered to give -resource-dir preference.

As for the separate issue of whether these C++ module maps don't belong in the platform SDK "because they are version-locked to the Swift compiler," the Swift modules are also version-locked to a Swift version and are in the platform SDK, so I agree with @compnerd that I don't think that is the main distinction.

@etcwilde, perhaps you'd like to chime in.

egorzhdan commented 1 week ago

But the final place once that is achieved would be the SDK.

@compnerd Yeap, that is right. Eventually we would move Cxx and CxxStdlib to the SDK.

egorzhdan commented 1 week ago

As for the separate issue of whether these C++ module maps don't belong in the platform SDK "because they are version-locked to the Swift compiler," the Swift modules are also version-locked to a Swift version and are in the platform SDK

@finagolfin Not quite, you can build a top-of-tree Swift compiler and use it with an SDK from Xcode. This wouldn't be possible with the Cxx and CxxStdlib modules.

Similarly, the getActualModuleMap() function should be reordered to give -resource-dir preference.

That sounds reasonable to me.

finagolfin commented 6 days ago

OK, the Swift 5.10.1 compiler finally shipped as part of Fedora 40, so here's the repro command and same build error I saw in Termux on there, after installing the system Swift package with sudo dnf install swift-lang, then downloading the trunk source as detailed in the Getting Started doc.

I downloaded the latest trunk snapshot toolchain and patched the stdlib CMake config, to work around a bug with building a standalone stdlib:

--- a/stdlib/cmake/modules/SwiftSource.cmake        2024-06-13 23:16:24.000000000 +0000
+++ b/stdlib/cmake/modules/SwiftSource.cmake        2024-06-30 19:47:00.579675599 +0000
@@ -880,9 +880,9 @@
   endif()

   set(custom_env "PYTHONIOENCODING=UTF8")
-  if(SWIFTFILE_IS_STDLIB OR
+  if(SWIFT_INCLUDE_TOOLS AND (SWIFTFILE_IS_STDLIB OR                                                                                                                    # Linux "hosttools" build require builder's runtime before building the runtime.
-     (BOOTSTRAPPING_MODE STREQUAL "HOSTTOOLS" AND SWIFT_HOST_VARIANT_SDK MATCHES "LINUX|ANDROID|OPENBSD|FREEBSD")
+     (BOOTSTRAPPING_MODE STREQUAL "HOSTTOOLS" AND SWIFT_HOST_VARIANT_SDK MATCHES "LINUX|ANDROID|OPENBSD|FREEBSD"))
   )
     get_bootstrapping_swift_lib_dir(bs_lib_dir "${SWIFTFILE_BOOTSTRAPPING}")                                                                                           if(bs_lib_dir)

Finally, running this build-script command fails because it is wrongly pulling the system 5.10.1 libcxxshim.modulemap, which is outdated compared to the trunk libcxxshim.modulemap:

> ./swift/utils/build-script -RA --skip-build-cmark --build-llvm=0 --native-clang-tools-path=/home/finagolfin/swift-DEVELOPMENT-SNAPSHOT-2024-06-13-a-ubi9/usr/bin --native-swift-tools-path=/home/finagolfin/swift-DEVELOPMENT-SNAPSHOT-2024-06-13-a-ubi9/usr/bin --build-swift-tools=0 --skip-early-swift-driver
--- snip output ---
FAILED: stdlib/public/Cxx/std/LINUX/x86_64/CxxStdlib.o /home/finagolfin/build/Ninja-Release/swift-linux-x86_64/stdlib/public/Cxx/std/LINUX/x86_64/CxxStdlib.o
cd /home/finagolfin/build/Ninja-Release/swift-linux-x86_64/stdlib/public/Cxx/std && /usr/bin/cmake -E make_directory /home/finagolfin/build/Ninja-Release/swift-linux-x86_64/stdlib/public/Cxx/std/LINUX/x86_64 && /usr/bin/cmake -E env PYTHONIOENCODING=UTF8 /usr/bin/python3.12 /home/finagolfin/swift/utils/line-directive @/home/finagolfin/build/Ninja-Release/swift-linux-x86_64/stdlib/public/Cxx/std/e1a548a31bae2b871f7a1f12cec5037e80240ac6.txt -- /home/finagolfin/swift-DEVELOPMENT-SNAPSHOT-2024-06-13-a-ubi9/usr/bin/swiftc -c -sdk / -target x86_64-unknown-linux-gnu -resource-dir /home/finagolfin/build/Ninja-Release/swift-linux-x86_64/./lib/swift -O -D SWIFT_ENABLE_EXPERIMENTAL_CONCURRENCY -D SWIFT_ENABLE_EXPERIMENTAL_DISTRIBUTED -D SWIFT_ENABLE_EXPERIMENTAL_DIFFERENTIABLE_PROGRAMMING -D SWIFT_ENABLE_EXPERIMENTAL_STRING_PROCESSING -D SWIFT_ENABLE_EXPERIMENTAL_OBSERVATION -D SWIFT_ENABLE_SYNCHRONIZATION -D SWIFT_RUNTIME_OS_VERSIONING -D SWIFT_STDLIB_ENABLE_UNICODE_DATA -D SWIFT_STDLIB_ENABLE_VECTOR_TYPES -D SWIFT_STDLIB_HAS_COMMANDLINE -D SWIFT_STDLIB_HAS_STDIN -D SWIFT_STDLIB_HAS_ENVIRON -Xcc -DSWIFT_STDLIB_HAS_ENVIRON -D SWIFT_CONCURRENCY_USES_DISPATCH -D SWIFT_STDLIB_OVERRIDABLE_RETAIN_RELEASE -D SWIFT_THREADING_LINUX -static -tools-directory /home/finagolfin/swift-DEVELOPMENT-SNAPSHOT-2024-06-13-a-ubi9/usr/bin -module-cache-path /home/finagolfin/build/Ninja-Release/swift-linux-x86_64/./module-cache -no-link-objc-runtime -Xfrontend -enforce-exclusivity=unchecked -D SWIFT_ENABLE_REFLECTION -module-name CxxStdlib -swift-version 5 -Xfrontend -empty-abi-descriptor -runtime-compatibility-version none -disable-autolinking-runtime-compatibility-dynamic-replacements -Xfrontend -disable-autolinking-runtime-compatibility-concurrency -Xfrontend -disable-objc-interop -enable-experimental-feature NoncopyableGenerics2 -enable-experimental-feature SuppressedAssociatedTypes -enable-experimental-feature ExtensionImportVisiblity -cxx-interoperability-mode=default -Xfrontend -module-interface-preserve-types-as-written -warn-implicit-overrides -Xfrontend -enable-ossa-modules -Xfrontend -enable-lexical-lifetimes=false -Xfrontend -disable-implicit-concurrency-module-import -Xfrontend -disable-implicit-string-processing-module-import -Xfrontend -prespecialize-generic-metadata -Xcc --gcc-toolchain=/usr -Xfrontend -define-availability -Xfrontend SwiftStdlib\ 9999:macOS\ 9999,\ iOS\ 9999,\ watchOS\ 9999,\ tvOS\ 9999 -Xfrontend -define-availability -Xfrontend SwiftStdlib\ 5.0:macOS\ 10.14.4,\ iOS\ 12.2,\ watchOS\ 5.2,\ tvOS\ 12.2 -Xfrontend -define-availability -Xfrontend SwiftStdlib\ 5.1:macOS\ 10.15,\ iOS\ 13.0,\ watchOS\ 6.0,\ tvOS\ 13.0 -Xfrontend -define-availability -Xfrontend SwiftStdlib\ 5.2:macOS\ 10.15.4,\ iOS\ 13.4,\ watchOS\ 6.2,\ tvOS\ 13.4 -Xfrontend -define-availability -Xfrontend SwiftStdlib\ 5.3:macOS\ 11.0,\ iOS\ 14.0,\ watchOS\ 7.0,\ tvOS\ 14.0 -Xfrontend -define-availability -Xfrontend SwiftStdlib\ 5.4:macOS\ 11.3,\ iOS\ 14.5,\ watchOS\ 7.4,\ tvOS\ 14.5 -Xfrontend -define-availability -Xfrontend SwiftStdlib\ 5.5:macOS\ 12.0,\ iOS\ 15.0,\ watchOS\ 8.0,\ tvOS\ 15.0 -Xfrontend -define-availability -Xfrontend SwiftStdlib\ 5.6:macOS\ 12.3,\ iOS\ 15.4,\ watchOS\ 8.5,\ tvOS\ 15.4 -Xfrontend -define-availability -Xfrontend SwiftStdlib\ 5.7:macOS\ 13.0,\ iOS\ 16.0,\ watchOS\ 9.0,\ tvOS\ 16.0 -Xfrontend -define-availability -Xfrontend SwiftStdlib\ 5.8:macOS\ 13.3,\ iOS\ 16.4,\ watchOS\ 9.4,\ tvOS\ 16.4 -Xfrontend -define-availability -Xfrontend SwiftStdlib\ 5.9:macOS\ 14.0,\ iOS\ 17.0,\ watchOS\ 10.0,\ tvOS\ 17.0 -Xfrontend -define-availability -Xfrontend SwiftStdlib\ 5.10:macOS\ 14.4,\ iOS\ 17.4,\ watchOS\ 10.4,\ tvOS\ 17.4,\ visionOS\ 1.1 -Xfrontend -define-availability -Xfrontend SwiftStdlib\ 6.0:macOS\ 9999,\ iOS\ 9999,\ watchOS\ 9999,\ tvOS\ 9999,\ visionOS\ 9999 -Xfrontend -target-min-inlining-version -Xfrontend min -Xcc -fno-omit-frame-pointer -whole-module-optimization -color-diagnostics -parse-as-library -resource-dir /home/finagolfin/build/Ninja-Release/swift-linux-x86_64/./lib/swift -I /home/finagolfin/build/Ninja-Release/swift-linux-x86_64/./lib/swift/linux -o /home/finagolfin/build/Ninja-Release/swift-linux-x86_64/stdlib/public/Cxx/std//LINUX/x86_64/CxxStdlib.o @/home/finagolfin/build/Ninja-Release/swift-linux-x86_64/stdlib/public/Cxx/std/e1a548a31bae2b871f7a1f12cec5037e80240ac6.txt
/home/finagolfin/swift/stdlib/public/Cxx/std/Chrono.swift:19:12: error: cannot find '__swift_interopMakeChronoSeconds' in scope
17 |   public init(_ duration: Duration) {
18 |     let (seconds, _) = duration.components                                                                                                                    19 |     self = __swift_interopMakeChronoSeconds(seconds)
   |            `- error: cannot find '__swift_interopMakeChronoSeconds' in scope
20 |   }
21 | }

/home/finagolfin/swift/stdlib/public/Cxx/std/Chrono.swift:27:12: error: cannot find '__swift_interopMakeChronoMilliseconds' in scope
25 |   public init(_ duration: Duration) {
26 |     let (seconds, attoseconds) = duration.components
27 |     self = __swift_interopMakeChronoMilliseconds(
   |            `- error: cannot find '__swift_interopMakeChronoMilliseconds' in scope
28 |       seconds * 1_000 +
29 |       attoseconds / 1_000_000_000_000_000)

/home/finagolfin/swift/stdlib/public/Cxx/std/Chrono.swift:37:12: error: cannot find '__swift_interopMakeChronoMicroseconds' in scope
35 |   public init(_ duration: Duration) {
36 |     let (seconds, attoseconds) = duration.components
37 |     self = __swift_interopMakeChronoMicroseconds(
   |            `- error: cannot find '__swift_interopMakeChronoMicroseconds' in scope
38 |       seconds * 1_000_000 +
39 |       attoseconds / 1_000_000_000_000)

/home/finagolfin/swift/stdlib/public/Cxx/std/Chrono.swift:47:12: error: cannot find '__swift_interopMakeChronoNanoseconds' in scope
45 |   public init(_ duration: Duration) {
46 |     let (seconds, attoseconds) = duration.components
47 |     self = __swift_interopMakeChronoNanoseconds(
   |            `- error: cannot find '__swift_interopMakeChronoNanoseconds' in scope
48 |       seconds * 1_000_000_000 +
49 |       attoseconds / 1_000_000_000)

/home/finagolfin/swift/stdlib/public/Cxx/std/String.swift:180:19: error: cannot find '__swift_interopHashOfU32String' in scope
178 |   public func hash(into hasher: inout Hasher) {
179 |     // Call std::hash<std::u32string>::operator()
180 |     let cxxHash = __swift_interopHashOfU32String().callAsFunction(self)                                                                                          |                   `- error: cannot find '__swift_interopHashOfU32String' in scope
181 |     hasher.combine(cxxHash)
182 |   }
[6/60][ 10%][1.275s] Building CXX object stdlib/public/Distributed/CMakeFiles/swiftDistributed-linux-x86_64.dir/DistributedActor.cpp.o
ninja: build stopped: subcommand failed.
ERROR: command terminated with a non-zero exit status 1, aborting

My compiler fix in #74814 got this same file building again on Android, and should do the same on Fedora and other platforms.