SR-7154: swiftpm tests fail if --debug-foundation option is used. #1544

spevans · 2018-03-25T14:20:48Z

Add a --swift-lib-dir so that tests can find libswiftSwiftOnone.so
which is required by libFoundation.so
The path to the libraries in the swift build directory are passed
to the test programs via the environment using LD_LIBRARY_PATH.

When testing using ./utils/build-script -R -T --llbuild --swiftpm --xctest --debug-foundation -- --reconfigure --lit-args=-v to build a debug version of Foundation, the libFoundation.so in the Foundation build directory is linked to libswiftSwiftOnone.so in the swift build directory. However libFoundation.so cannot find libswiftSwiftOnone.so in its default RUNPATH. This adds an LD_LIBRARY_PATH to the environment so that libswiftSwiftOnone.so can be found ok.

This PR works in conjunction with swiftlang/swift#15498 for swift's main build script so it can pass the directory.

- Add a --swift-lib-dir so that tests can find libswiftSwiftOnone.so which is required by libFoundation.so - The path to the libraries in the swift build directory are passed to the test programs via the environment using LD_LIBRARY_PATH.

aciidgh · 2018-03-26T16:41:17Z

Utilities/bootstrap

@@ -1052,6 +1055,9 @@ def main():

    env_cmd = ["env", "SWIFT_EXEC=" + os.path.join(bindir, "swiftc"),
                      "SWIFTPM_BUILD_DIR=" + build_path]
+
+    if args.swift_lib_dir:
+        env_cmd.append("LD_LIBRARY_PATH=" + args.swift_lib_dir)


Can we put a symlink in the fake toolchain instead of using LD_LIBRARY_PATH or that doesn't work?

You can do it that way and it was the original way I had it working, however it also requires adding a symlink in Foundation's build directory as well. The other downside is that if in the future more libraries get built by the main swift repo in addition to libswiftCore.so and libswiftSwiftOnone.so then symlinks will also be required there.

However thinking about it, this LD_LIBRARY_PATH could possibly be exported in the swift/utils/build-script-impl script the same way it is setup when running TestFoundation (see https://github.com/spevans/swift/blob/ce45ea3eeaca9a30e257be7fbe5746a67d5b2788/utils/build-script-impl#L2950) and then this change wouldnt be required.

We have several tests in SwiftPM that build Swift packages in debug mode. I don't understand why we don't hit the same problem there. @jrose-apple said these libraries can be considered part of stdlib.

I don't think it matters whether the packages that SwiftPM builds are in debug or release mode but whether the libFoundation.so they link to is in release or debug mode. In debug mode it links to libswiftSwiftOnone.so which it can't find in the build directories without this path.
The --debug-foundation flag controls whether the libFoundation.so is built in debug mode.

@jrose-apple said these libraries are required if something is being built with -Onone and we have several test packages that are built with -Onone. Presence of libFoundation.so shouldn't matter.

Can we have an investigation on this behavior and why and how it affects SwiftPM tests? It is blocking Foundation adoption of @testable import.

When you build with -Onone, you gain a dependency on libswiftSwiftOnoneSupport.so/.a (yes, it's silly that there are two "swift"s in there). It shouldn't have anything to do with @testable import.

I'll take a closer look tomm...

spevans · 2018-03-28T06:58:52Z

So swiftpm runs two binaries as part of its testing, build/Ninja-ReleaseAssert/swiftpm-linux-x86_64/x86_64-unknown-linux/release/swift-build-stage1 and build/Ninja-ReleaseAssert/swiftpm-linux-x86_64/x86_64-unknown-linux/release/swift-test.

The first one, swift-build-stage-1 runs without issue. Using readelf we can see that its runpath and dependant shared libraries are as follows:

$ readelf -aW ../build/Ninja-ReleaseAssert/swiftpm-linux-x86_64/x86_64-unknown-linux/release/swift-build-stage1|egrep 'Library runpath|Shared library'
 0x0000000000000001 (NEEDED)             Shared library: [libswiftCore.so]
 0x0000000000000001 (NEEDED)             Shared library: [libswiftSwiftOnoneSupport.so]
 0x0000000000000001 (NEEDED)             Shared library: [libFoundation.so]
 0x0000000000000001 (NEEDED)             Shared library: [libswiftGlibc.so]
 0x0000000000000001 (NEEDED)             Shared library: [libpthread.so.0]
 0x0000000000000001 (NEEDED)             Shared library: [libutil.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libdl.so.2]
 0x0000000000000001 (NEEDED)             Shared library: [libdispatch.so]
 0x0000000000000001 (NEEDED)             Shared library: [libstdc++.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libm.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libgcc_s.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
 0x000000000000001d (RUNPATH)            Library runpath: [/home/spse/src/swift/build/Ninja-ReleaseAssert/swift-linux-x86_64/lib/swift/linux:$ORIGIN/../lib/swift/linux]

The RUNPATH is the search path for libraries. So when swift-build-stage1 is run, it dynamically loads libswiftSwiftOnoneSupport.so which it can find in /home/spse/src/swift/build/Ninja-ReleaseAssert/swift-linux-x86_64/lib/swift/linux. Later when libFoundation.so is needed, it is loaded from the second path $ORIGIN/../lib/swift/linux. $ORIGIN refers to the directory of the loading ELF file, in this case the directory containing ../build/Ninja-ReleaseAssert/swiftpm-linux-x86_64/x86_64-unknown-linux/release/swift-build-stage1 which gives a resultant path of ../build/Ninja-ReleaseAssert/swiftpm-linux-x86_64/x86_64-unknown-linux/release/../lib/swift/linux, the contents of which are:

$ ls -l ../build/Ninja-ReleaseAssert/swiftpm-linux-x86_64/x86_64-unknown-linux/release/../lib/swift/linux
total 8
lrwxr-xr-x 1 501 dialout 96 Mar 27 22:32 libdispatch.so -> /home/spse/src/swift/build/Ninja-ReleaseAssert/libdispatch-linux-x86_64/src/.libs/libdispatch.so
lrwxr-xr-x 1 501 dialout 98 Mar 27 22:32 libFoundation.so -> /home/spse/src/swift/build/Ninja-ReleaseAssert/foundation-linux-x86_64/Foundation/libFoundation.so

So it finds libFoundation.so here. When libFoundation.so later needs libswiftSwiftOnone.so, it is already loaded and there is no issue.

The second binary that is run is build/Ninja-ReleaseAssert/swiftpm-linux-x86_64/x86_64-unknown-linux/release/swift-test which has the following runpath and dependancies:

$ readelf -aW ../build/Ninja-ReleaseAssert/swiftpm-linux-x86_64/x86_64-unknown-linux/release/swift-test|egrep 'Library runpath|Shared library'
 0x0000000000000001 (NEEDED)             Shared library: [libswiftCore.so]
 0x0000000000000001 (NEEDED)             Shared library: [libpthread.so.0]
 0x0000000000000001 (NEEDED)             Shared library: [libutil.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libdl.so.2]
 0x0000000000000001 (NEEDED)             Shared library: [libdispatch.so]
 0x0000000000000001 (NEEDED)             Shared library: [libswiftGlibc.so]
 0x0000000000000001 (NEEDED)             Shared library: [libFoundation.so]
 0x0000000000000001 (NEEDED)             Shared library: [libstdc++.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libm.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libgcc_s.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
 0x000000000000001d (RUNPATH)            Library runpath: [/home/spse/src/swift/build/Ninja-ReleaseAssert/swift-linux-x86_64/lib/swift/linux:$ORIGIN:/../lib/swift/linux:/home/spse/src/swift/build/Ninja-ReleaseAssert/foundation-linux-x86_64/Foundation:/home/spse/src/swift/build/Ninja-ReleaseAssert/xctest-linux-x86_64]

This time swift-test loads libFoundation.so first which it finds via the /home/spse/src/swift/build/Ninja-ReleaseAssert/foundation-linux-x86_64/Foundation entry in the runpath. Looking at libFoundation.so, readelf shows the following:

$ readelf -aW ../build/Ninja-ReleaseAssert/foundation-linux-x86_64/Foundation/libFoundation.so|egrep 'Library rpath|Shared library'
 0x0000000000000001 (NEEDED)             Shared library: [libswiftGlibc.so]
 0x0000000000000001 (NEEDED)             Shared library: [libswiftSwiftOnoneSupport.so]
 0x0000000000000001 (NEEDED)             Shared library: [libicui18n.so.55]
 0x0000000000000001 (NEEDED)             Shared library: [libicuuc.so.55]
 0x0000000000000001 (NEEDED)             Shared library: [libicudata.so.55]
 0x0000000000000001 (NEEDED)             Shared library: [libcurl.so.4]
 0x0000000000000001 (NEEDED)             Shared library: [libxml2.so.2]
 0x0000000000000001 (NEEDED)             Shared library: [libpthread.so.0]
 0x0000000000000001 (NEEDED)             Shared library: [libdl.so.2]
 0x0000000000000001 (NEEDED)             Shared library: [libm.so.6]
 0x0000000000000001 (NEEDED)             Shared library: [libswiftCore.so]
 0x0000000000000001 (NEEDED)             Shared library: [libdispatch.so]
 0x0000000000000001 (NEEDED)             Shared library: [libgcc_s.so.1]
 0x0000000000000001 (NEEDED)             Shared library: [libc.so.6]
 0x000000000000000f (RPATH)              Library rpath: [$ORIGIN]

This time libFoundation.so is the library requesting libswiftSwiftOnoneSupport.so be dynamically loaded however libFoundation.so only has an RPATH of $ORIGIN which resolves to its own build directory of /home/spse/src/swift/build/Ninja-ReleaseAssert/foundation-linux-x86_64/Foundation/. This directory does not contain libswiftSwiftOnone so this is where the loading fails and the error message swift-test: error while loading shared libraries: libswiftSwiftOnoneSupport.so: cannot open shared object file: No such file or directory appears.

The addition of the LD_LIBRARY_PATH environment variable, which is considered after the RPATH, is added to point to /home/spse/src/swift/build/Ninja-ReleaseAssert/swift-linux-x86_64/lib/swift/linux/x86_64 which contains libswiftSwiftOnoneSupport.so and thus resolves the issue.

The following could be achieved through the use of extra symlinks and this is actually the case for the loading of libdispatch.so which is loaded via a symlink made by swift-corelibs-xctest's build-script.py adding a symlink into swift-corelibs-foundation's build directory however there are a couple of issues with this.

If swift-corelib-foundation's build directory is deleted, it wont know to add the symlink back.
Adding symlinks in multiple places in the build scripts is more inconvienent than providing a simple LD_LIBRARY_PATH environment variable (which is also the method used when invoking TestFoundation from build-script-impl)
If swift ever vends more libraries as part of its runtime / stdlib, extra symlinks would need to be created for those as well.

Extra information about ELF library loading that may be useful: How to write shared libraries

jrose-apple · 2018-03-28T15:50:30Z

More options:

When libFoundation.so is built with -Onone it gets a runpath to the current location of the stdlib it was built against. It's not like we're going to distribute a debug Foundation anyway.
We all actually build into a common toolchain location somehow, instead of just putting products in each project's build directory. Unfortunate CMake changes, most likely.

millenomi · 2018-03-28T18:18:23Z

Thus far, all development on Linux that I've done has required setting a correct LD_LIBRARY_PATH. Is it something that we don't want to have to occur?

aciidgh · 2018-03-29T00:35:45Z

@spevans Thanks for the detailed analysis. I like @jrose-apple's first suggestion about Foundation getting rpath to swift stdlibs when its built with -Onone. This currently happens automatically in swift-test because we use Swift compiler as the linker and it always embeds the stdlib's rpath. We manually remove the embedded rpath during the install step.

aciidgh · 2018-03-29T02:03:43Z

@millenomi If I remember correctly, LD_LIBRARY_PATH is usually considered anti-pattern and should be avoided but I don't really remember the reasons. (But maybe I dreamed that up.)

spevans · 2018-03-30T16:36:02Z

I reworked this by simply passing an LD_LIBRARY_PATH when running the tests:

call env LD_LIBRARY_PATH="${SWIFT_BUILD_PATH}/lib/swift/${SWIFT_HOST_VARIANT}/${SWIFT_HOST_VARIANT_ARCH}" "${swiftpm_bootstrap_command[@]}" test --test-parallel

This also means that this PR is not now required as it can all be done in utils/build-script-impl. I will update swiftlang/swift#15498 to use this method.

I think the issue with adding in runpaths into the libraries is that it makes the build process more complicated and will cause issues if not properly removed. Debugging is also made more complicated as this is now a non-obvious path added into the linking, as opposed to an explicit LD_LIBRARY_PATH on the command line.

@millenomi I do have one question with the @testable import which is that if debugging needs to be enabled for the tests to run, how are the tests run on the snapshot builds which are built in release mode?

aciidgh · 2018-03-30T16:45:55Z

I still don't think we should use LD_LIBRARY_PATH. It will make things difficult when using SwiftPM's bootstrap command independently of the Swift's build-script. You can pass the runpath to foundation from the build-script. I am not sure why debugging becomes complicated. Is that some know issue?

jrose-apple · 2018-03-30T18:05:34Z

Note that we don't want to provide an rpath for release builds, because we might make toolchains out of those and ship them to people.

spevans · 2018-03-31T15:38:15Z

Ive done an RPATH version for swift-corelibs-foundation in swiftlang/swift-corelibs-foundation#1495

spevans mentioned this pull request Mar 25, 2018

SR-7154: Pass the build directory containing swift libs to swiftpm swiftlang/swift#15498

Closed

spevans force-pushed the pr_sr_7154 branch from 97272dc to 53446da Compare March 25, 2018 17:20

spevans changed the title ~~SR-7154: swiftpm tests fail if --debug-foundation optios it used.~~ SR-7154: swiftpm tests fail if --debug-foundation option is used. Mar 25, 2018

aciidgh reviewed Mar 26, 2018

View reviewed changes

spevans mentioned this pull request Mar 31, 2018

SR-7154: Make swiftpm tests work when building with --debug-foundation swiftlang/swift-corelibs-foundation#1495

Merged

spevans closed this Mar 31, 2018

spevans mentioned this pull request Mar 25, 2018

[SR-7154] Foundation PR testing broken swiftlang/swift#49702

Closed

SR-7154: swiftpm tests fail if --debug-foundation option is used. #1544

SR-7154: swiftpm tests fail if --debug-foundation option is used. #1544

Uh oh!

Conversation

spevans commented Mar 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spevans commented Mar 28, 2018

Uh oh!

jrose-apple commented Mar 28, 2018

Uh oh!

millenomi commented Mar 28, 2018

Uh oh!

aciidgh commented Mar 29, 2018

Uh oh!

aciidgh commented Mar 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

spevans commented Mar 30, 2018

Uh oh!

aciidgh commented Mar 30, 2018

Uh oh!

jrose-apple commented Mar 30, 2018

Uh oh!

spevans commented Mar 31, 2018

Uh oh!

Uh oh!

spevans commented Mar 25, 2018 •

edited

Loading

aciidgh commented Mar 29, 2018 •

edited

Loading