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

gccWithoutTargetLibc: if glibc, enable threads and pass headers #241208

Closed
wants to merge 9 commits into from
Closed

gccWithoutTargetLibc: if glibc, enable threads and pass headers #241208

wants to merge 9 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Jul 3, 2023

⚠️ see update below ⚠️

Description of changes

If ${targetPackages.glibc}/include/stdio.h does not exist during the configurePhase of pkgsCross.*.gcc.libgcc, the stack unwinder will be silently disabled.

The resulting libgcc_s.so will deliberately crash any process which attempts to use pthread_cleanup_push() or similar calls.

This PR ensures that ${targetPackages.glibc}/include/stdio.h does exist during the configurePhase of pkgsCross.*.gcc.libgcc, so we get an unwinder-capable libgcc_s.so.

This also adds a new package glibcHeaders, which allows to build the glibc headers without first having built a targetPlatform compiler. This is needed in order to break the dependency cycle between glibc and libgcc. Additional details are in the commit message for that particular commit

Closes:
Includes
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Adam Joseph added 8 commits July 1, 2023 13:12
This commit adds `hasSharedLibraries` to `lib.systems`.

We need `plat.hasSharedLibraries` in order to know whether or not to
expect `gcc` (and many other tools) to emit shared libraries (like
`libgcc_s.so`).  Many of the GNU build scripts are smart enough that
if you configure them with `--enable-shared` on a platform (such as
`arm-none-eabi`) that doesn't support dynamic linking, they will
simply skip the shared libraries instead of aborting the
`configurePhase`.  Unfortunately the missing shared libraries in the
final build product cause very hard-to-troubleshoot problems later
on.

The alternative to introducing `hasSharedLibraries` would be to set
`isStatic` in these situations.  However doing so causes
`make-derivation.nix` to insert `-static` between the `pname` and
`hostPlatform` suffix, which is undesirable.

If at some point in the future we eliminate the `-static` suffix,
then `hasSharedLibraries` can be made equal to `!isStatic`.
…rgetLibc

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

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 two stages:

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

  2. Use gccCrossStageStatic to compile the targetPlatform libc.

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

You might notice that this pattern looks very similar to what we do
with `xgcc` 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
This commit renames the `crossStageStatic` argument to the `gcc`
expression to `withoutTargetLibc`.  See previous commit for details.
We want a `libgcc_s.so` to be built by the first stage
cross-compiler (withoutTargetLibc), since that is the compiler which
will compile the target libc.

This commit accomplishes that, by making three changes:

1. Replacing the `targetPlatform.libc == "msvcrt" &&` conditional
   with `enableShared`, so that the code which cross-build
   `libgcc_s.so` is used for all cross compilers capable of emitting
   shared libraries.

2. Removing the `targetPlatform == hostPlatform` guard from the code
   which produces the `libgcc` output.

3. Looking for build products in in "lib/${targetPlatform.config}/"
   rather than "lib/", so we will find them when cross compiling.
This commit is reverted in #240596 (which must go to staging).
The situation described in the comment preceding `export
inhibit_libc=true` does not match the conditional which follows it.
Specifically, the comment says that this line is meant for "clang
builds gcc" situations, yet it is enabled even when
`!stdenv.cc.isClang`.

This commit tightens the conditional to make it match the situation
described in the comment.
This expression allows to build the glibc headers without first
having built a targetPlatform compiler.  It is needed in order to
break the dependency cycle between glibc and libgcc:

1. glibc must have the exact path to libgcc embedded into it via
   user-defined-trusted-dirs; see nixpkgs commit
   7553d0f.  This reference is not
   part of the ELF structure so we can't use `patchelf` to update it.

2. libgcc must be able to see the glibc headers in order to compile
   unwinder support.  Otherwise, it will deliberately `assert()`
   when a thread which has called `pthread_cleanup_push()` exits:

   https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libgcc/unwind-dw2.c;h=d0afce7a9ea9f5b12a5a01ef1e940e1452b48cab;hb=HEAD#l1336

Unfortunately due to how GNU autoconf works, you can't *usually* get
the glibc headers without a targetPlatform compiler, because the
headers are partially generated based on autoconf results, and
autoconf is based on running the targetPlatform C compiler on
various inputs and seeing if they fail.

Fortunately the glibc developers are at least partially aware of
this being problematic, which is why they have avoided using any
targetPlatform-compiler-derived data when producing the
targetPlatform headers, and have provided the `--enable-hacker-mode`
flag for `./configure`, which runs just enough of the process to
allow `make install-headers` to work (but `make` certainly *won't*).
@ghost ghost mentioned this pull request Jul 3, 2023
12 tasks
@ghost ghost changed the title gcc: for glibc platforms, enable threads and pass headers gccWithoutTargetLibc: for glibc platforms, enable threads and pass headers Jul 3, 2023
@ghost ghost changed the title gccWithoutTargetLibc: for glibc platforms, enable threads and pass headers gccWithoutTargetLibc: if glibc, enable threads and pass headers Jul 3, 2023
@ghost ghost marked this pull request as ready for review July 3, 2023 03:22
If ${targetPackages.glibc}/include/stdio.h does not exist during the
configurePhase of pkgsCross.*.gcc.libgcc, the stack unwinder will be
silently disabled.  The resulting libgcc_s.so will deliberately
crash any process which attempts to use `pthread_cleanup_push()` or
similar calls.

Closes #213453
@ghost ghost marked this pull request as draft July 3, 2023 06:12
@ghost
Copy link
Author

ghost commented Jul 6, 2023

Update: so, this does fix #213453, and it does work on x86_64 and aarch64. Unfortunately it pretty much doesn't work on any other architecture.

Apparently being able to ./configure without a targetPlatform compiler is a fairly new feature for glibc and it has only been implemented on the popular platforms. So I don't think this PR will ever be undraftified (and, because of that, I am unlikely to hunt down the OfBorg eval failure although I can rebuild my whole userspace without hitting it).

After much digging around in the guts of glibc I think I've found a better solution: right now we use the libgcc from the first-stage gcc (aka gccWithoutTargetLibc). That libgcc is missing the unwinder. All we need to do is use the libgcc from the final gcc instead.

There are two ways to do this:

  1. The easy way: add an extra recompile of glibc for pkgsCross.*.stdenv. This is simple enough that it might be backport-eligible.

  2. The better way: use a technique similar to patchelf --add-rpath to update the pointer in glibc. Of course the pointer-updated glibc would be a new derivation with a new outpath, but unlike in the "easy way" it wouldn't require any compiling. Most importantly, it is a change which could be applied to both pkgsCross.*.stdenv and also to native-stdenv. This will take time, testing, and have to go through staging. We can't use patchelf since the "pointer" isn't an ELF DT_NEEDED, but it is a simple string in the static-data section of ld.so.

So to summarize:

  1. This PR is 99%-likely to be abandoned.
  2. I will write a PR for strategy 1 above in the next week or two, and if merged propose it for backporting.
  3. I will write another PR which reverts strategy 1 and implements strategy 2 sometime in the next few months. This PR will not be proposed for backporting to 23.05.

@alyssais alyssais added the 2.status: merge conflict This PR has merge conflicts with the target branch label Oct 10, 2023
@ghost ghost closed this Oct 22, 2023
@ghost ghost deleted the cross-libgcc-build-with-unwinder branch October 22, 2023 07:34
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant