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

gccCrossStageStatic: enable dynamic libraries, rename it #238154

Merged
6 commits merged into from Jul 12, 2023
Merged

gccCrossStageStatic: enable dynamic libraries, rename it #238154

6 commits merged into from Jul 12, 2023

Conversation

ghost
Copy link

@ghost ghost commented Jun 16, 2023

Best reviewed one commit at a time.

Motivation
  1. Fix a serious bug in our pkgsCross bootstrap: cross compiled binaries segfault on pthread_cancel(3) due to missing libgcc_s.so #240484. This is the reason for the backport request.
    a. Also: Closes libgcc_s.so is missing from glibc when cross compiling #40797

  2. Make our cross-compiler bootstrap look more like our stdenv bootstrap, as a step towards unifying them.

Summary

This PR allows gccCrossStageStatic to build dynamically-linked libraries. Since is no longer restricted to building static libraries its name is no longer appropriate, and this PR also renames it to the more-accurate gccWithoutTargetLibc.

Detailed description of changes

By default, you can't build a gcc that knows how to create dynamic libraries unless you have already built the targetPlatform libc.

Because of this, our gcc cross-compiler is built in three stages:

  1. Build a cross-compiler (gccCrossStageStatic) that can build only static libraries.

  2. Use gccCrossStageStatic to compile the targetPlatform libc.a.

  3. Use the targetPlatform libc.a to build a fully-capable cross compiler.

You might notice that this pattern looks very similar to what we do with xgcc and glibc in the stdenv bootstrap. Indeed it is! I would like to work towards getting the existing stdenv bootstrap to handle cross compilers as well. However we don't want to cripple stdenv.xgcc by taking away its ability to build dynamic libraries.

It turns out that the only thing gcc needs the targetPlatform libc for is to emit a DT_NEEDED for -lc into libgcc.so. That's it! And since we don't use gccCrossStageStatic to build anything other than libc, it's safe to omit the DT_NEEDED because that libgcc will never be loaded by anything other than libc. So libc will already be in the process's address space.

Other people have noticed this; crosstool-ng has been using this approach for a very long time:

https://github.com/crosstool-ng/crosstool-ng/blob/36ad0b17a732aaffe4701d5d8d410d6e3e3abba9/scripts/build/cc/gcc.sh#L638-L640

Closes #240484

Closes #240435

Things done
  • Built on platform(s)
    • x86_64-linux
    • powerpc64le-linux
    • aarch64-linux (cross from x86_64-linux)
    • mips64el-linuxabi64 (cross from x86_64-linux)
    • mips64el-linuxabin32 (cross from x86_64-linux)
    • arm-eabi gcc6 (cross from x86_64-linux)
  • Built to test gcc versions
    • pkgsCross.aarch64-multiplatform
    • .pkgsBuildTarget.gcc48
    • .pkgsBuildTarget.gcc49
    • .pkgsBuildTarget.gcc6
    • .pkgsBuildTarget.gcc7
    • .pkgsBuildTarget.gcc8
    • .pkgsBuildTarget.gcc9
    • .pkgsBuildTarget.gcc10
    • .pkgsBuildTarget.gcc11
    • .pkgsBuildTarget.gcc12
    • .pkgsBuildTarget.gcc13
  • Tested complete userspace rebuild on platform(s):
    • powerpc64le-linux
    • aarch64-linux (cross from x86_64-linux)
    • mips64el-linuxabi64+mips64el-linuxabin32 (mixed) (cross from x86_64-linux)

@ghost
Copy link
Author

ghost commented Jun 16, 2023

I'm testing builds on a pretty large matrix of target platforms and gcc versions.

In spite of this, I have the nagging feeling that this is going to break some obscure gcc version on an infrequently-used platform. I've tried to keep this PR master-eligible (i.e. no mass-rebuilds) so that if that kind of breakage does happen,

  1. People will be able to find the cause (this PR) without having to git bisect across a merge of staging into master
  2. The fix PR can go to master instead of staging.

@ghost

This comment was marked as outdated.

@ghost ghost marked this pull request as ready for review June 18, 2023 03:32
@ghost ghost requested review from matthewbauer and Ericson2314 as code owners June 18, 2023 03:32
@ghost
Copy link
Author

ghost commented Jun 19, 2023

Everything tests fine, rebooted the laptop with a complete system (bootloader, kernel, userspace) built this way.

However the change made in gcc commit e50084fa44cb68c447efc11e53ec16bf09a578c0 needs to be made to libstdc++ as well; I will send a patch to do that upstream and include it here.

Edit: what I wrote in the previous paragraph is true, but I will do that in a separate PR after this one. The only thing we need gccWithoutTargetLibc to do is compile libgcc_s.so and glibc, and neither of those need libstdc++.

Getting gccWithoutTargetLibc to produce a working libgcc_s.so is actually pretty urgent (see #240484), so I don't want it to wait.

@ghost ghost marked this pull request as draft June 19, 2023 06:40
@ghost ghost changed the title gccCrossStageStatic: enable dynamic libraries, rename to gccWithoutTargetLibc gccCrossStageStatic: enable dynamic libraries and rename it Jun 28, 2023
@ghost ghost changed the title gccCrossStageStatic: enable dynamic libraries and rename it gccCrossStageStatic: enable dynamic libraries, rename it Jun 28, 2023
@trofi
Copy link
Contributor

trofi commented Jul 13, 2023

Result of nixpkgs-review pr 238154 run on x86_64-linux 1

2 packages blacklisted:
  • nixos-install-tools
  • tests.nixos-functions.nixos-test
50 packages failed to build:
  • dxvk
  • dxvk.bin
  • dxvk.lib
  • exactaudiocopy
  • grapejuice
  • grapejuice.dist
  • klipper-firmware
  • klipper-flash
  • pipelight
  • pkgsLLVM.stdenv
  • playonlinux
  • q4wine
  • vkd3d
  • vkd3d-proton
  • winbox
  • wine (winePackages.full ,winePackages.stableFull)
  • wine-staging (winePackages.stagingFull)
  • wine-wayland (winePackages.waylandFull)
  • wine64 (wine64Packages.full ,wine64Packages.stableFull)
  • wine64Packages.base (wine64Packages.stable)
  • wine64Packages.staging
  • wine64Packages.stagingFull
  • wine64Packages.unstable
  • wine64Packages.unstableFull
  • wine64Packages.wayland
  • wine64Packages.waylandFull
  • winePackages.base (winePackages.stable)
  • winePackages.staging
  • winePackages.unstable
  • winePackages.unstableFull
  • winePackages.wayland
  • wineWow64Packages.base (wineWow64Packages.stable)
  • wineWow64Packages.full (wineWow64Packages.stableFull)
  • wineWow64Packages.staging
  • wineWow64Packages.stagingFull
  • wineWow64Packages.unstable
  • wineWow64Packages.unstableFull
  • wineWow64Packages.wayland
  • wineWow64Packages.waylandFull
  • wineWowPackages.base (wineWowPackages.stable)
  • wineWowPackages.full (wineWowPackages.stableFull)
  • wineWowPackages.staging
  • wineWowPackages.stagingFull
  • wineWowPackages.unstable
  • wineWowPackages.unstableFull
  • wineWowPackages.wayland
  • wineWowPackages.waylandFull
  • wineasio
  • yabridge
  • yabridgectl
10 packages built:
  • armTrustedFirmwareTools
  • fusee-launcher
  • gnuk
  • klipper-genconf
  • qmk
  • qmk.dist
  • simavr
  • spike
  • tests.makeBinaryWrapper
  • tinygo

@ghost
Copy link
Author

ghost commented Jul 13, 2023

Result of nixpkgs-review pr 238154 run on x86_64-linux 1

(Edit)

Most (maybe all) of this is fixed by these two PRs (mostly the second one):

(edit)

So this doesn't happen again:

@trofi
Copy link
Contributor

trofi commented Jul 14, 2023

Bisect claims that one of these commits also broke pkgsCross.m68k.stdenv in master:

$ nix build --no-link -f. pkgsCross.m68k.stdenv -L
...
m68k-unknown-linux-gnu-stage-static-gcc> make[1]: Leaving directory '/build/build/m68k-unknown-linux-gnu/libgcc'
m68k-unknown-linux-gnu-stage-static-gcc> Moving /nix/store/8658fc2zsh062ibn1l4vsvsapy4jppkz-m68k-unknown-linux-gnu-stage-static-gcc-12.3.0/m68k-unknown-linux-gnu/lib/libgcc_s.so to /nix/store/sf4g7jbpm502pp8lr2jh0qvxq2wqv0db-m68k-unknown-linux-gnu-stage-static-gcc-12.3.0-lib/m68k-unknown-linux-gnu/lib/libgcc_s.so
m68k-unknown-linux-gnu-stage-static-gcc> Moving /nix/store/8658fc2zsh062ibn1l4vsvsapy4jppkz-m68k-unknown-linux-gnu-stage-static-gcc-12.3.0/m68k-unknown-linux-gnu/lib/libgcc_s.so.2 to /nix/store/sf4g7jbpm502pp8lr2jh0qvxq2wqv0db-m68k-unknown-linux-gnu-stage-static-gcc-12.3.0-lib/m68k-unknown-linux-gnu/lib/libgcc_s.so.2
m68k-unknown-linux-gnu-stage-static-gcc> Removing empty /nix/store/8658fc2zsh062ibn1l4vsvsapy4jppkz-m68k-unknown-linux-gnu-stage-static-gcc-12.3.0/m68k-unknown-linux-gnu/lib/ and (possibly) its parents
m68k-unknown-linux-gnu-stage-static-gcc> /nix/store/qr60k7sz61hvv2baadlfv5qjr8q7vlg6-builder.sh: line 243: type: install_name_tool: not found
m68k-unknown-linux-gnu-stage-static-gcc> preFixupLibGccPhase
m68k-unknown-linux-gnu-stage-static-gcc> mv: cannot stat '/nix/store/sf4g7jbpm502pp8lr2jh0qvxq2wqv0db-m68k-unknown-linux-gnu-stage-static-gcc-12.3.0-lib/m68k-unknown-linux-gnu/lib/libgcc_s.so.1': No such file or directory

I think m68k defines libgcc_s.so.2, not libgcc_s.so.1.

@trofi
Copy link
Contributor

trofi commented Jul 14, 2023

And bisect claims that one of these commits also broke pkgsCross.s390.stdenv in master:

$ nix build --no-link -f. pkgsCross.s390.stdenv -L
...
s390-unknown-linux-gnu-stage-static-gcc> /build/build/./gcc/xgcc -B/build/build/./gcc/    -O2  -DIN_GCC  -DCROSS_DIRECTORY_STRUCTURE  -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition  -isystem ./include  -fPIC -mlong-double-128 -g -DIN_LIBGCC2 -fbuilding-libgcc -fno-stack-protector -Dinhibit_libc -fPIC -mlong-double-128 -I. -I. -I../.././gcc -I../../../gcc-12.3.0/libgcc -I../../../gcc-12.3.0/libgcc/. -I../../../gcc-12.3.0/libgcc/../gcc -I../../../gcc-12.3.0/libgcc/../include  -DHAVE_CC_TLS   -o morestack_s.o -MT morestack_s.o -MD -MP -MF morestack_s.dep -DSHARED -c -xassembler-with-cpp ../../../gcc-12.3.0/libgcc/config/s390/morestack.S
s390-unknown-linux-gnu-stage-static-gcc> make[1]: *** No rule to make target '../../../gcc-12.3.0/libgcc/config/s390/crti.S', needed by 'crti.o'.  Stop.
s390-unknown-linux-gnu-stage-static-gcc> make[1]: *** Waiting for unfinished jobs....
s390-unknown-linux-gnu-stage-static-gcc> /nix/store/51sszqz1d9kpx480scb1vllc00kxlx79-bash-5.2-p15/bin/bash ../../../gcc-12.3.0/libgcc/../move-if-change tmp-libgcc_tm.h libgcc_tm.h
s390-unknown-linux-gnu-stage-static-gcc> echo timestamp > libgcc_tm.stamp
s390-unknown-linux-gnu-stage-static-gcc> ../../../gcc-12.3.0/libgcc/config/s390/morestack.S: Assembler messages:
s390-unknown-linux-gnu-stage-static-gcc> ../../../gcc-12.3.0/libgcc/config/s390/morestack.S:600: Warning: ignoring incorrect section type for .init_array.00000
s390-unknown-linux-gnu-stage-static-gcc> make[1]: Leaving directory '/build/build/s390-unknown-linux-gnu/libgcc'
s390-unknown-linux-gnu-stage-static-gcc> make: *** [Makefile:12947: all-target-libgcc] Error 2
error: builder for '/nix/store/lffjd90i8pa7mbwjcxpwf8cxaa2xqljp-s390-unknown-linux-gnu-stage-static-gcc-12.3.0.drv' failed with exit code 2;

Don't see why immediately.

@ghost
Copy link
Author

ghost commented Jul 15, 2023

Bisect claims that one of these commits also broke pkgsCross.m68k.stdenv in master:

And bisect claims that one of these commits also broke pkgsCross.s390.stdenv in master:

Please add these to tests.cross.sanity so they are tested for; if there are no tests for something, it is certain to break.

Also, I'm pretty sure none of us can afford S390 hardware.

@trofi
Copy link
Contributor

trofi commented Jul 15, 2023

$ nix build -f.  tests.cross.sanity
error: attribute 'sanity' in selection path 'tests.cross.sanity' not found

@ghost
Copy link
Author

ghost commented Jul 15, 2023

@ofborg build tests.cross.sanity

@trofi
Copy link
Contributor

trofi commented Jul 17, 2023

Added assert against an attrset field caused the non-determinism regression: #244045 (a patch for nix to ease reproducibility: NixOS/nix#8711)

a-kenji pushed a commit to a-kenji/nixpkgs that referenced this pull request Aug 21, 2023
 ### Summary

This PR completely and finally solves the gcc<->glibc circular
`buildInputs` problem, for cross compilation.  The same technique
can be applied to native builds in the future.

Closes NixOS#213453

 ### Motivation

Prior to this PR, we had the following circular `buildInputs` problem:

1. gcc has glibc in its `buildInputs`

   - a compiled copy of glibc must be present before building gcc;
     if it isn't, gcc cripples itself (`inhibit_libc`) and refuses
     to build libgcc_s.so

2. glibc has libgcc_s.so in its `buildInputs`

   - glibc `dlopen()`s libgcc_s.so in order to implement POSIX
     thread cancellation.  For security reasons `glibc` requires
     that the path to `libgcc_s.so` is [hardwired] into `glibc` at
     compile time, so it's technically not a true dynamic link -- it
     just pretends to be one.

3. libgcc_s.so is built in the same derivation as gcc

   - libgcc_s.so is built as part of the gcc build process

We must cut one of these three links in the loop.

 ### Previous Attempts

Previously NixOS#238154 had
attempted to cut link (1) by building `gcc` without `glibc`, and
using the `libgcc_s` which emerges from that build.  Unfortunately
this just doesn't work.  GCC's configure script extracts quite a lot
of information from the glibc headers (which are a build artifact --
you can't just copy them out of the source tarball) and various
`./configure`-driven linking attempts.  If `glibc` isn't around at
build time you wind up with a `libgcc_s.so` that is missing various
unwinder features (see NixOS#213453
for the most problematic one).

Musl "cuts" link (2), or rather never creates it in the first place.
["Cancellation cleanup handling in musl has no relationship to C++
exceptions and unwinding... glibc implements cancellation as an
exception"](https://wiki.musl-libc.org/functional-differences-from-glibc.html#Thread-cancellation).
IMHO Musl made the smarter decision here.  It is incredibly rare to
find a codebase that uses both POSIX thread cancellation *and* C++
exceptions.  I have never seen a codebase that uses both *and*
expects them to be aware of each other, and I would be astonished if
one existed.  Glibc paid an immense cost in complexity for something
nobody has ever used.

 ### Changes Made

This PR cuts link (3): instead of building libgcc_s.so as part of
gcc, we build it separately from gcc.  Now there is a strict acyclic
graph of `buildInputs`:

```
 gccWithoutTargetLibc
 |
 +--->glibc-nolibgcc
 |    |
 |    v
 +--->libgcc
 |    |
 |    v
 +--->glibc
 |    |
 |    v
 +--->gcc
```

In other words, there's a simple linear `buildInputs` chain
`glibc-nolibgcc` `->` `libgcc` `->` `glibc` `->` `gcc` where all
four packages are compiled by (and therefore have as a
`(native)BuildInput`) `gccWithoutTargetLibc`.

`gccWithoutTargetLibc` and `glibc-nolibgcc` are strictly
bootstrapping artifacts; nothing else has them as a `buildInput` and
they shouldn't appear in the closure of any final deployment
packages.  `glibc-nolibgcc` lacks `libgcc_s.so`, so it will segfault
if you try to use it with POSIX thread cancellation.  Fortunately
all we need from it is (a) its headers (`lib.getDev`) and (b) to use
it in the `./configure` script for `libgcc`.

When translated over to the native bootstrap, `xgcc` takes the place
of `gccWithoutTargetLibc`, and the "first `glibc`" (we build two of
them) takes the place of `glibc-nolibgcc`.  At that point our native
and cross bootstrap have the same overall architecture, and it
becomes possible to merge them (at last!)

[213453]: NixOS#213453
[238154]: NixOS#238154
[hardwired]: https://github.com/NixOS/nixpkgs/blob/7553d0fe29801938bcb280bb324b579ef9016aea/pkgs/development/libraries/glibc/default.nix#L69-L88
@trofi
Copy link
Contributor

trofi commented Sep 1, 2023

Another regression is pkgCross.loongarch64-linux.stdenv reported by jiegec: #252590

@mikatammi
Copy link
Contributor

I'm guessing the automatic backport failed and we never got backport to 23.05? Could this be backported, it would be nice, as I'm seeing the cross-compiled Wayland failing on runtime, and this PR fixes the issue

@github-actions
Copy link
Contributor

github-actions bot commented Oct 2, 2023

Backport failed for release-23.05, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git fetch origin release-23.05
git worktree add -d .worktree/backport-238154-to-release-23.05 origin/release-23.05
cd .worktree/backport-238154-to-release-23.05
git checkout -b backport-238154-to-release-23.05
ancref=$(git merge-base 931b8a4979df477c0baffc2544f593d7a9b32dcb 96a2f1b4e1315251f1916f64c4632dbf7eb40522)
git cherry-pick -x $ancref..96a2f1b4e1315251f1916f64c4632dbf7eb40522

@alyssais
Copy link
Member

alyssais commented Oct 2, 2023

This PR caused some regressions, so any potential backport should probably be okayed by the release managers before being merged.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants