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

Force linux builds off of base toolchain #66398

Conversation

etcwilde
Copy link
Contributor

@etcwilde etcwilde commented Jun 7, 2023

UBI9 installs the gcc-12 aligned toolchain as a dependency for clang. Clang picks up the gcc-12 aligned toolchain and symbols and adds a call to glibcxx::__assert_fail, which is not in the gcc-11 aligned libstdc++. To work with this when dynamically linking, the gcc-12 toolset contains a libstdc++_noshared.a static archive, which gets linked into the shared libraries and executables much like our compatibility libraries, and provides the missing symbols on top of the gcc-11 runtime that is already on UBI9 systems.

Of course, folks want to be able to statically link the stdlib into their binaries, and we avoid overlinking and symbol collisions by not linking the libstdc++_noshared.a static archive into the libswiftCore.a. Of course, this means that there is no definition for the symbol when it comes time to link the static stdlib on a base UBI9 image.

I've decided to just build the library against what is installed on the base system, so that we don't need to install anything additional to build and copy the binary to other systems running the same OS.

This should fix the linking issue in #65097
In theory, this should actually work for all of the Linux images and ensure that we're actually building the Linux toolchains based on what is actually available on the base system and not what is installed with the toolchain-build-dependencies.

@etcwilde etcwilde requested a review from shahmishal June 7, 2023 00:24
@etcwilde
Copy link
Contributor Author

etcwilde commented Jun 7, 2023

CC: @FranzBusch

@etcwilde
Copy link
Contributor Author

etcwilde commented Jun 7, 2023

@swift-ci please test

@etcwilde
Copy link
Contributor Author

etcwilde commented Jun 7, 2023

@swift-ci please build linux toolchain

@etcwilde
Copy link
Contributor Author

etcwilde commented Jun 7, 2023

Linux failure:

    clang: error: unknown argument '-gcc-toolchain=/usr'; did you mean '--gcc-toolchain=/usr'?
    clang: error: unknown argument '-gcc-toolchain=/usr'; did you mean '--gcc-toolchain=/usr'?
    <unknown>:0: error: link command failed with exit code 1 (use -v to see invocation)
    ninja: build stopped: subcommand failed.

UBI9 installs the gcc-12 aligned toolchain as a dependency for clang.
Clang picks up the gcc-12 aligned toolchain and symbols and adds a call
to `glibcxx::__assert_fail`, which is not in the gcc-11 aligned
libstdc++. To work with this when dynamically linking, the gcc-12
toolset contains a `libstdc++_noshared.a` static archive, which gets
linked into the shared libraries and executables much like our
compatibility libraries, and provides the missing symbols on top of the
gcc-11 runtime that is already on UBI9 systems.

Of course, folks want to be able to statically link the stdlib into
their binaries, and we avoid overlinking and symbol collisions by not
linking the libstdc++_noshared.a static archive into the libswiftCore.a.
Of course, this means that there is no definition for the symbol when it
comes time to link the static stdlib on a base UBI9 image.

I've decided to just build the library against what is installed on the
base system, so that we don't need to install anything additional to
build and copy the binary to other systems running the same OS.
@etcwilde etcwilde force-pushed the ewilde/all-your-bases-are-belong-to-us branch from b3417f2 to e36a308 Compare June 7, 2023 02:19
@etcwilde
Copy link
Contributor Author

etcwilde commented Jun 7, 2023

@swift-ci please test

@etcwilde
Copy link
Contributor Author

etcwilde commented Jun 7, 2023

Windows failure:

FAILED: dispatch_timer.exe 
cmd.exe /C "cd . && "C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.exe" -E vs_link_exe --intdir=tests\CMakeFiles\dispatch_timer.dir --rc=C:\PROGRA~2\WI3CF2~1\10\bin\100177~1.0\x64\rc.exe --mt=C:\PROGRA~2\WI3CF2~1\10\bin\100177~1.0\x64\mt.exe --manifests  -- T:\1\bin\lld-link.exe /nologo tests\CMakeFiles\dispatch_timer.dir\dispatch_timer.c.obj  /out:dispatch_timer.exe /implib:tests\dispatch_timer.lib /pdb:dispatch_timer.pdb /version:0.0 /INCREMENTAL:NO /INCREMENTAL:NO /subsystem:console -LIBPATH:C:\PROGRA~2\MICROS~1\2019\COMMUN~1\VC\Tools\MSVC\1429~1.300\Lib\x64   -LIBPATH:C:\PROGRA~2\WI3CF2~1\10\Lib\100177~1.0\ucrt\x64   -LIBPATH:C:\PROGRA~2\WI3CF2~1\10\Lib\100177~1.0\um\x64 src\dispatch.lib  src\BlocksRuntime\BlocksRuntime.lib  tests\bsdtests.lib  bcrypt.lib  kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib && cd ."
MT: command "C:\PROGRA~2\WI3CF2~1\10\bin\100177~1.0\x64\mt.exe /nologo /manifest dispatch_timer.exe.manifest /outputresource:dispatch_timer.exe;#1" failed (exit code 0x1f) with the following output:

mt.exe : general error c101008d: Failed to write the updated manifest to the resource of file "dispatch_timer.exe". Access is denied.

@FranzBusch
Copy link
Member

Amazing investigation @etcwilde! Thanks for doing this. I agree with your sentiment that we should be doing this for all Linux platforms to make sure we are not picking up any gcc that is installed after the fact and might break on the base images.

@etcwilde
Copy link
Contributor Author

etcwilde commented Jun 9, 2023

@swift-ci please test windows

@etcwilde
Copy link
Contributor Author

etcwilde commented Jun 9, 2023

Alright, on a fresh ubi9 with the binutils (for the linker) and gcc-c++, we can statically link with this change in place;

[root@9892eb36d904 devtools]# ./usr/bin/swiftc -static-stdlib test.swift 
[root@9892eb36d904 devtools]# ./test
Hello World
[root@9892eb36d904 devtools]# ldd test
        linux-vdso.so.1 (0x00007ffe355af000)
        libstdc++.so.6 => /lib64/libstdc++.so.6 (0x00007f0f3f8b7000)
        libm.so.6 => /lib64/libm.so.6 (0x00007f0f3f7dc000)
        libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007f0f3f7c1000)
        libc.so.6 => /lib64/libc.so.6 (0x00007f0f3f5b8000)
        /lib64/ld-linux-x86-64.so.2 (0x00007f0f400ab000)
[root@9892eb36d904 devtools]# nm -a test | grep 'glibcxx'

@MaxDesiatov
Copy link
Contributor

@swift-ci build toolchain linux

@shahmishal
Copy link
Member

@swift-ci build toolchain ubi9

@shahmishal
Copy link
Member

@swift-ci build toolchain CentOS 7

@shahmishal
Copy link
Member

@swift-ci build toolchain Ubuntu 18.04

@shahmishal
Copy link
Member

@swift-ci build toolchain Ubuntu 22.04

@shahmishal
Copy link
Member

@swift-ci build toolchain Amazon Linux 2

@etcwilde
Copy link
Contributor Author

CentOS 7 failure:

CMake Error at cmake/modules/CheckCompilerVersion.cmake:88 (message):
  libstdc++ version must be at least 7.1.

@etcwilde
Copy link
Contributor Author

So yes, it looks like we'll need different configurations to handle building the different distros. The C++ stdlib on centOS 7 is too old.

@FranzBusch
Copy link
Member

We definitely need to line up the installed dependencies for the toolchains here with the installed dependencies in the docker images then. @shahmishal can we come up with a way so this doesn’t get out of sync

@shahmishal
Copy link
Member

shahmishal commented Jun 10, 2023

Do we know why it's out of sync? Is it because CentOS does not support newer version? @FranzBusch

@FranzBusch
Copy link
Member

@etcwilde pointed out that we need to install newer gcc versions for some toolchain builds. We also need these to be installed in the respective docker images right?
We should probably wait how the other toolchains turn out since CentOS7 is also nearing EOL.

@FranzBusch
Copy link
Member

Just saw everything works except CentOS7 and UBI9. Need to check why UBI9 failed

@shahmishal
Copy link
Member

Just saw everything works except CentOS7 and UBI9. Need to check why UBI9 failed

You can ignore the check of Swift Build Toolchain UBI9 Platform and check the results of Swift Build Toolchain UBI9 (x86_64)

Swift Build Toolchain UBI9 Platform was when I was testing the PR setup.

@shahmishal
Copy link
Member

All of the platforms passed except for CentOS 7.

Copy link
Contributor

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be ok to merge this as is and deal with the CentOS 7 failure separately.

@etcwilde
Copy link
Contributor Author

etcwilde commented Jun 16, 2023

I think it would be ok to merge this as is and deal with the CentOS 7 failure separately.

I'm waiting for discussions on swiftlang/swift-docker#341 to be resolved. CentOS 7 is a little odd since the base is so old, so it will need its own configuration and will need to build against the special devtoolset. In that case we'll want to ensure that the build images have the extra toolset as well, but I want to make sure that the clang that we're installing from our toolchain actually behaves correctly too.

@etcwilde
Copy link
Contributor Author

Checked static linking with 5.9.2 and 5.10.1, and bot are working. Closing this.

@etcwilde etcwilde closed this Jun 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants