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

stdenv/darwin: put compiler-rt into bootstrap tarball #94426

Conversation

thefloweringash
Copy link
Member

@thefloweringash thefloweringash commented Aug 1, 2020

compiler-rt is no longer included in clang, so the current result of
building bootstrap tools doesn't include compiler-rt at all.

Motivation for this change

I want to update bootstrap tools soon as part of #19906, but just doing the update results in a subtly broken early toolchain. It looks like the bootstrap tools builder needs a small update. This is the first part that puts the required parts into the tarball, there's a follow up that fixes the unpacking stage.

This defines a new set of bootstrap tools, see #94427 for the "other half" that uses the updated bootstrap tools.

Possibly related #39743

cc @matthewbauer and @LnL7 who seem to work on the bootstrap files

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

compiler-rt is no longer included in clang, so the current result of
building bootstrap tools doesn't include compiler-rt at all.
@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Aug 1, 2020
@ofborg ofborg bot added 6.topic: stdenv Standard environment 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Aug 1, 2020
@thefloweringash
Copy link
Member Author

The main problem I'm seeing with trying to update the bootstrap tools without both of these changes is

$ nix-build -A perlPackages.LocaleGettext
[...]
running tests
check flags: SHELL=/nix/store/2zpjcwvc73cmirzvw7g461s96ri6dr55-bash-4.4-p23/bin/bash VERBOSE=y test
"/nix/store/z72xsh2ks2w337q2mr8kanhb47hln76l-perl-5.30.3/bin/perl" -MExtUtils::Command::MM -e 'cp_nonempty' -- gettext.bs blib/arch/auto/Locale/gettext/gettext.bs 644
PERL_DL_NONLAZY=1 "/nix/store/z72xsh2ks2w337q2mr8kanhb47hln76l-perl-5.30.3/bin/perl" "-MExtUtils::Command::MM" "-MTest::Harness" "-e" "undef *Test::Harness::Switches; test_harness(0, 'blib/lib', 'blib/arch')" t/*.t
t/bind.t ....... Can't load '/private/tmp/nix-build-perl5.30.3-gettext-1.07.drv-1/Locale-gettext-1.07/blib/arch/auto/Locale/gettext/gettext.bundle' for module Locale::gettext: dlopen(/private/tmp/nix-build-perl5.30.3-gettext-1.07.drv-1/Locale-gettext-1.07/blib/arch/auto/Locale/gettext/gettext.bundle, 2): Symbol not found: ___isOSVersionAtLeast
  Referenced from: /nix/store/p4s4w4q1lwvcla9p88d9ikvvwfxl66fg-curl-7.71.1/lib/libcurl.4.dylib
  Expected in: flat namespace
 in /nix/store/p4s4w4q1lwvcla9p88d9ikvvwfxl66fg-curl-7.71.1/lib/libcurl.4.dylib at /nix/store/z72xsh2ks2w337q2mr8kanhb47hln76l-perl-5.30.3/lib/perl5/5.30.3/darwin-thread-multi-2level/DynaLoader.pm line 197.
 at t/bind.t line 6.
Compilation failed in require at t/bind.t line 6.
BEGIN failed--compilation aborted at t/bind.t line 6.
t/bind.t ....... Dubious, test returned 2 (wstat 512, 0x200)
Failed 1/1 subtests
[...]

Full log: https://gist.github.com/thefloweringash/10f236a3d9fee59298bdee2ec47480e8


The curl with the undefined ___isOSVersionAtLeast symbol is built from stage 4. Maybe the most significant change is the cc-wrapper setup in #94427.

$ nix show-derivation /nix/store/p4s4w4q1lwvcla9p88d9ikvvwfxl66fg-curl-7.71.1/lib/libcurl.4.dylib | head -2
{
  "/nix/store/cy80vkg7r3m4gi0c3lhlhk4a5dshlrnw-curl-7.71.1.drv": {

$ nix-store -q --references /nix/store/cy80vkg7r3m4gi0c3lhlhk4a5dshlrnw-curl-7.71.1.drv | grep stdenv
/nix/store/2p866jj5z5l1qgaz6h4a63pb1azakxs7-bootstrap-stage4-stdenv-darwin.drv

@thefloweringash thefloweringash marked this pull request as ready for review August 17, 2020 11:45
@thefloweringash
Copy link
Member Author

Quick sanity check:

libclang_rt differences from current bootstrap tools to currently generated bootstrap tools

-lib/clang/CLANG_VERSION/lib/darwin/libclang_rt.10.4.a
-lib/clang/CLANG_VERSION/lib/darwin/libclang_rt.asan_osx_dynamic.dylib
-lib/clang/CLANG_VERSION/lib/darwin/libclang_rt.cc_kext.a
-lib/clang/CLANG_VERSION/lib/darwin/libclang_rt.osx.a
-lib/clang/CLANG_VERSION/lib/darwin/libclang_rt.profile_osx.a
-lib/clang/CLANG_VERSION/lib/darwin/libclang_rt.safestack_osx.a
-lib/clang/CLANG_VERSION/lib/darwin/libclang_rt.stats_client_osx.a
-lib/clang/CLANG_VERSION/lib/darwin/libclang_rt.stats_osx_dynamic.dylib
-lib/clang/CLANG_VERSION/lib/darwin/libclang_rt.ubsan_osx_dynamic.dylib

libclang_rt differences from currently generated bootstrap tools (HEAD~), to generated bootstrap tools as of this PR (HEAD)

+lib/darwin/libclang_rt.10.4.a
+lib/darwin/libclang_rt.asan_osx_dynamic.dylib
+lib/darwin/libclang_rt.cc_kext.a
+lib/darwin/libclang_rt.fuzzer_no_main_osx.a
+lib/darwin/libclang_rt.fuzzer_osx.a
+lib/darwin/libclang_rt.lsan_osx_dynamic.dylib
+lib/darwin/libclang_rt.osx.a
+lib/darwin/libclang_rt.profile_osx.a
+lib/darwin/libclang_rt.safestack_osx.a
+lib/darwin/libclang_rt.stats_client_osx.a
+lib/darwin/libclang_rt.stats_osx_dynamic.dylib
+lib/darwin/libclang_rt.ubsan_minimal_osx.a
+lib/darwin/libclang_rt.ubsan_minimal_osx_dynamic.dylib
+lib/darwin/libclang_rt.ubsan_osx.a
+lib/darwin/libclang_rt.ubsan_osx_dynamic.dylib
+lib/darwin/libclang_rt.xray-basic_osx.a
+lib/darwin/libclang_rt.xray-fdr_osx.a
+lib/darwin/libclang_rt.xray-profiling_osx.a
+lib/darwin/libclang_rt.xray_osx.a
+lib/libclang_rt.10.4.a
+lib/libclang_rt.asan_osx_dynamic.dylib
+lib/libclang_rt.cc_kext.a
+lib/libclang_rt.fuzzer_no_main_osx.a
+lib/libclang_rt.fuzzer_osx.a
+lib/libclang_rt.lsan_osx_dynamic.dylib
+lib/libclang_rt.osx.a
+lib/libclang_rt.profile_osx.a
+lib/libclang_rt.safestack_osx.a
+lib/libclang_rt.stats_client_osx.a
+lib/libclang_rt.stats_osx_dynamic.dylib
+lib/libclang_rt.ubsan_minimal_osx.a
+lib/libclang_rt.ubsan_minimal_osx_dynamic.dylib
+lib/libclang_rt.ubsan_osx.a
+lib/libclang_rt.ubsan_osx_dynamic.dylib
+lib/libclang_rt.xray-basic_osx.a
+lib/libclang_rt.xray-fdr_osx.a
+lib/libclang_rt.xray-profiling_osx.a
+lib/libclang_rt.xray_osx.a

In the above the lib/libclang_rt* files are symlinks to their darwin/ counterparts.

@matthewbauer matthewbauer requested a review from LnL7 October 6, 2020 19:08
@matthewbauer
Copy link
Member

/cc @LnL7

Copy link
Member

@LnL7 LnL7 left a comment

Choose a reason for hiding this comment

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

I mentioned this before on irc, but I think this is more of a bootstrapping issue. Keeping as much as possible out of the bootstrap tarball makes bootstrapping more flexible and simplifies introducing a new architecture.

All that said I don't see a particularly big problem with adding compiler-rt.

@thefloweringash
Copy link
Member Author

I mentioned this before on irc, but I think this is more of a bootstrapping issue

As I understand it, clang_rt is a required component of a clang toolchain providing "builtins", and clang may choose to compile code to call a function defined in clang_rt. In practice, we have two instances (assuming #94427 is merged): one from bootstrap tools, and one rebuilt at stage 3. We might be able to omit the bootstrap tools version until we build a new one, but I'm not in favor of this, since it means using a partial clang toolchain in the early stages. As for how it works currently, the cc value for the entire bootstrap process comes from the bootstrap tarball, so the bootstrap clang v4 with integrated compiler-rt is used from beginning to end

All that said I don't see a particularly big problem with adding compiler-rt.

Not sure if I made this clear, but the current bootstrap tools contain compiler-rt. I think it was removed from the generator by #39743 as part of a clang refactoring. This change is to intended to keep the structure of the current bootstrap tools.

@PetarKirov
Copy link

What is the status of the PR? I'm not an expert in this area, but the change (including more of the already build llvmPackages code) doesn't seem like it can introduce issues (except for increasing the surface area of the bootstrap tarball), so I think it's worth including as it solves real issues.

@SuperSandro2000
Copy link
Member

@ofborg eval

@thefloweringash
Copy link
Member Author

This was subsumed by bootstrap changes in #105026.

@thefloweringash thefloweringash deleted the darwin-bootstrap-compiler-rt branch July 10, 2021 02:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: darwin Running or building packages on Darwin 6.topic: stdenv Standard environment 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
Status: Big Sur
Development

Successfully merging this pull request may close these issues.

6 participants