glibc: add enableCopyLibGccHack #210112

ghost · 2023-01-10T21:44:17Z

Description of changes

There has been much wailing and gnashing of teeth over the "libgcc copying hack":

To facilitate the excision of this foulness, let's provide an option (on by default) for it. Turning this option off will be a first step towards removing the hack.

Things done

Built on platform(s)
- x86_64-linux
- aarch64-linux
Fits CONTRIBUTING.md.

tpwrules · 2023-01-11T15:43:40Z

What is the plan to turn this flag off? I am pretty sure it cannot be turned off conditionally based on architecture, as right now that copy is the one loaded dynamically by glibc at runtime for certain pthread functions. It will abort if it can't be found. The fact that it's from the wrong GCC and causes linker errors is sort of a separate issue.

But once that issue is fixed then we can just remove the preInstall. I don't see a situation where this flag is on in some cases and off in others.

ghost · 2023-01-11T18:40:13Z

What is the plan to turn this flag off?

This PR turns off enableCopyLibGccHack when you turn on gcc.enableExternalBootstrap:

stdenv: Nix-driven bootstrap of gcc #209870

tpwrules · 2023-01-11T20:13:13Z

I think the hack is no longer necessary for bootstrap, but it is still necessary (or at least currently supports) for programs which uses pthreads. Please try to build pkgs.dejagnu, I wonder if it aborts during the testing phase.

ghost · 2023-01-11T21:22:07Z

Please try to build pkgs.dejagnu, I wonder if it aborts during the testing phase.

Indeed, I have observed the failure you describe. I will investigate.

ghost · 2023-01-11T21:51:49Z

This seems to be a ~~libtcl~~pthread_cancel() problem. ~~I'm going to see if we can put a hack into libtcl in order to remove a hack from basically everything else.~~

tpwrules · 2023-01-11T22:00:35Z

Yes, I provided additional context in #208412, particularly in option 2. glibc seems to need to dynamically load it in support of pthreads.

To me the crime is not that there is a libgcc_s.so in glibc (as keeping a copy in glibc is a convenient way to make sure glibc always has it and avoid increasing everybody's closure size by referencing GCC's libraries), but that it is currently the wrong version. I'm excited by all the work around cleaning up the bootstrap sequence and maybe the hack can be removed long-term, but I don't want to focus on it unless it helps upgrade GCC. The failure mode is particularly egregious too, with random programs randomly aborting under certain circumstances.

ghost · 2023-01-11T22:02:47Z

Root cause:

glibc commit 9d79e0377b08773ec4f7ec38479b1563606f7ef7
- moved the stack unwinder out of glibc
- replaced the compiler-agnostic implementation of pthread_cancel() with one that uses the stack unwinder
- added a dlopen("libgcc_s") to access the unwinder

If killing threads needs compiler-specific information (which it probably does), what's supposed to happen when you compile two .o files, one with clang and one with gcc, link them together, and call pthread_kill()? Which compiler's unwinder should it use?

If libgcc_s's unwinder works on any compiler's binaries (using dwarf2 data?) then it ought to be its own library, separate from gcc.

ghost · 2023-01-11T22:08:45Z

To me the crime is not that there is a libgcc_s.so

The crime is that pthread_cancel() exists. Killing threads, if it is even allowed at all, is a language feature not a quasi-system-call.

(as keeping a copy in glibc is a convenient way to make sure glibc always has it and avoid increasing everybody's closure size by referencing GCC's libraries),

The solution to the closure size concern is to move libgcc into its own output of gcc and reference it. Copying like this defeats a lot of Nix's sanity checks.

ghost · 2023-01-11T22:55:37Z

Added an explanation of the real reason why this hack was put in place.

tpwrules · 2023-01-11T23:19:17Z

This is not quite the correct explanation, glibc itself does the abort and the application cannot stop it from happening in any way. You can see expect printing the message in strace to a presumably-already-closed fd. This also all applies to pthread_exit() which is a more reasonable call, and some stack unwinding support logic.

If you simply Google libgcc_s.so.1 must be installed for pthread_exit to work then you see this error has popped up in quite a wide variety of programs and scenarios, so I don't believe it's true that these pthread calls are used "very infrequently". I just provided that example as an easy trigger within nixpkgs. But the fact that it can show up anywhere is scary and I don't have a good solution. Nor to what happens if you mix compilers.

I don't think adding the extra output is a bad solution, but it will then require a hack to GCC to link it into every program which uses pthreads or even glibc to be safe. It is also probably smart to update the comment in nixpkgs to the current state of the problem.

Mic92 · 2023-01-11T23:23:13Z

guix have their gcc patched so it will add libgcc to all libraries they build btw: https://github.com/guix-mirror/guix/blob/5e4ec8218142eee8e6e148e787381a5ef891c5b1/gnu/packages/gcc.scm#L243

ghost · 2023-01-11T23:40:42Z

This is not quite the correct explanation, glibc itself does the abort and the application cannot stop it from happening in any way.

Thanks, fixed

I just provided that example as an easy trigger within nixpkgs.

You provided the same example I did -- libtcl (via expect). Is there another? Asynchronous thread cancellation is generally considered to be a language misfeature -- Java deprecated it 20 years ago and Rust won't let you do it without unsafe.

I don't think adding the extra output is a bad solution,

I have an implementation of that and am testing it now.

but it will then require a hack to GCC

I don't believe so. The gcc.libgcc output becomes a dependency of glibc, which makes it a dependency of everything that could need it for pthread_cancel().

tpwrules · 2023-01-12T00:11:01Z

Asynchronous thread cancellation is generally considered to be a language misfeature -- Java deprecated it 20 years ago and Rust won't let you do it without unsafe.

As explained in my previous comments

pthread_exit() is not asynchronous and is completely safe
pthread_exit() will also abort if libgcc_s.so cannot be found
Plenty of real-world code uses both functions, as evidenced by the reports of glibc's error message. Therefore we cannot break it.

The gcc.libgcc output becomes a dependency of glibc

How does this happen? It is not currently as libgcc_s.so is loaded dynamically. And making it so would be an essentially identical situation to this hack where glibc pulls the wrong libgcc_s.so into other processes, it would just be stored in a slightly different place.

ghost · 2023-01-12T00:39:08Z

As explained in my previous comments and commits

pthread_**cancel**() is asynchronous if PTHREAD_CANCEL_ASYNCHRONOUS is used.
if PTHREAD_CANCEL_ASYNCHRONOUS is not used, there is no need for stack unwinding, and no need for libgcc_s.
Real-world code rarely, if ever, uses PTHREAD_CANCEL_ASYNCHRONOUS. No examples have been presented in this thread.

We seem to be talking past each other.

tpwrules · 2023-01-12T01:00:00Z

My point is I don't understand why it matters what glibc could actually do, the problem today is that it does need libgcc_s in common situations encountered by popular programs. For example, CPython (though they are trying to reduce its usage as this keeps biting them).

Do you propose changing (or at least make a note that it could be changed in the future) glibc to need libgcc_s in fewer circumstances, i.e. those which are unlikely to be encountered in real-world code? What about for older versions?

ghost · 2023-01-12T02:11:19Z

Do you propose changing (or at least make a note that it could be changed in the future) glibc to need libgcc_s in fewer circumstances, i.e. those which are unlikely to be encountered in real-world code? What about for older versions?

That would be nice. But the politicking required to get it merged is not something I have time for. It would also be nice if they had not deleted the old implementation that did not need libgcc_s.

ghost · 2023-01-12T02:13:06Z

guix have their gcc patched so it will add libgcc to all libraries they build btw: https://github.com/guix-mirror/guix/blob/5e4ec8218142eee8e6e148e787381a5ef891c5b1/gnu/packages/gcc.scm#L243

@Mic92, thank you so much for this link.

The most important part of that link is that it links to this mailing list posting. That was incredibly helpful.

I bet that explains why all previous attempts to pull libgcc_s from anywhere other than the exact same directory that contains glibc had failed. It also explains how to make them not fail.

tpwrules · 2023-01-12T02:44:29Z

Proof of concept of removing the hack completely here. Can you point me to the commit which puts libgcc_s.so in a separate output?

Artturin · 2023-04-28T14:44:56Z

Can this be closed?

ghost · 2023-05-01T10:08:36Z

Yep.

@Ericson2314

#### Summary By default, when you type `make`, GCC will compile itself three times. This PR inhibits that behavior by configuring GCC with `--disable-bootstrap`, and reimplements the triple-rebuild using Nix rather than `make`/`sh`. #### Immediate Benefits - Allow `gcc11` and `gcc12` on `aarch64` (without needing new `bootstrapFiles`) - Faster stdenv rebuilds: the third compilation of gcc (i.e. stageCompare) is no longer a `drvInput` of the final stdenv. This allows Nix to build stageCompare in parallel with the rest of nixpkgs instead of in series. - No more copying `libgcc_s` out of the bootstrap-files or other derivations - No more Frankenstein compiler: the final gcc and the libraries it links against (mpfr, mpc, isl, glibc) are all built by the same compiler (xgcc) instead of a mixture of the bootstrapFiles' compiler and xgcc. - No more [static lib{mpfr,mpc,gmp,isl}.a hack] - Many other small `stdenv` hacks eliminated - `gcc` and `clang` share the same codepath for more of `cc-wrapper`. #### Future Benefits - This should allow using a [foreign] `bootstrap-files` so long as `hostPlatform.canExecute bootstrapFiles`. - This should allow each of the libraries that ship with `gcc` (lib{backtrace, atomic, cc1, decnumber, ffi, gomp, iberty, offloadatomic, quadmath, sanitizer, ssp, stdc++-v3, vtv}) to be built in separate (one-liner) derivations which `inherit src;` from `gcc`, much like NixOS#132343 #### Incorporates - NixOS#210004 - NixOS#36948 (unreverted) - NixOS#210325 - NixOS#210118 - NixOS#210132 - NixOS#210109 - NixOS#213909 - NixOS#216136 - NixOS#216237 - NixOS#210019 - NixOS#216232 - NixOS#216016 - NixOS#217977 - NixOS#217995 #### Closes - Closes NixOS#108305 - Closes NixOS#108111 - Closes NixOS#201254 - Closes NixOS#208412 #### Credits This project was made possible by three important insights, none of which were mine: 1. @Ericson2314 was the first to advocate for this change, and probably the first to appreciate its advantages. Nix-driven (external) bootstrap is "cross by default". 2. @trofi has figured out a lot about how to get gcc to not mix up the copy of `libstdc++` that it depends on with the copy that it builds, by moving the `bootstrapFiles`' `libstdc++` into a [versioned directory]. This allows a Nix-driven bootstrap of gcc without the final gcc would still having references to the `bootstrapFiles`. 3. Using the undocumented variable [`user-defined-trusted-dirs`] when building glibc. When glibc `dlopen()`s `libgcc_s.so`, it uses a completely different and totally special set of rules for finding `libgcc_s.so`. This trick is the only way we can put `libgcc_s.so` in its own separate outpath without creating circular dependencies or dependencies on the bootstrapFiles. I would never have guessed to use this (or that it existed!) if it were not for a [comment in guix] which @Mic92 [mentioned]. My own role in this PR was basically: being available to go on a coding binge at an opportune moment, so we wouldn't waste a [crisis]. [aarch64-compare-ofborg]: https://github.com/NixOS/nixpkgs/pull/209870/checks?check_run_id=10662822938 [amd64-compare-ofborg]: https://github.com/NixOS/nixpkgs/pull/209870/checks?check_run_id=10662825857 [nonexistent sysroot]: NixOS#210004 [versioned directory]: NixOS#209054 [`user-defined-trusted-dirs`]: https://sourceware.org/legacy-ml/libc-help/2013-11/msg00026.html [comment in guix]: https://github.com/guix-mirror/guix/blob/5e4ec8218142eee8e6e148e787381a5ef891c5b1/gnu/packages/gcc.scm#L253 [mentioned]: NixOS#210112 (comment) [crisis]: NixOS#108305 [foreign]: NixOS#170857 (comment) [static lib{mpfr,mpc,gmp,isl}.a hack]: https://github.com/NixOS/nixpkgs/blob/2f1948af9c984ebb82dfd618e67dc949755823e2/pkgs/stdenv/linux/default.nix#L380

ghost mentioned this pull request Jan 10, 2023

stdenv: Nix-driven bootstrap of gcc #209870

Merged

4 tasks

ofborg bot added 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Jan 10, 2023

ghost marked this pull request as ready for review January 10, 2023 23:10

ghost mentioned this pull request Jan 11, 2023

glibc: don't use bootstrap libgcc_s #36948

Merged

8 tasks

ghost requested review from kalbasit, Mic92, zowoq, ajs124, lukegb, mweinelt and matthewbauer as code owners January 15, 2023 00:14

ofborg bot added 2.status: merge conflict 10.rebuild-linux: 0 and removed 2.status: merge conflict 10.rebuild-linux: 1-10 labels Feb 13, 2023

ghost closed this May 1, 2023

ghost deleted the pr/glibc/enableCopyLibGccHack branch May 1, 2023 10:08

Artturin mentioned this pull request Jan 29, 2024

List of @amjoseph-nixpkgs Pull requests before account removal #284796

Open

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

glibc: add enableCopyLibGccHack #210112

glibc: add enableCopyLibGccHack #210112

ghost commented Jan 10, 2023

tpwrules commented Jan 11, 2023

ghost commented Jan 11, 2023 •

edited by ghost

tpwrules commented Jan 11, 2023

ghost commented Jan 11, 2023

ghost commented Jan 11, 2023 •

edited by ghost

tpwrules commented Jan 11, 2023 •

edited

ghost commented Jan 11, 2023 •

edited by ghost

ghost commented Jan 11, 2023 •

edited by ghost

ghost commented Jan 11, 2023

tpwrules commented Jan 11, 2023 •

edited

Mic92 commented Jan 11, 2023 •

edited

ghost commented Jan 11, 2023 •

edited by ghost

tpwrules commented Jan 12, 2023

ghost commented Jan 12, 2023

tpwrules commented Jan 12, 2023

ghost commented Jan 12, 2023

ghost commented Jan 12, 2023 •

edited by ghost

tpwrules commented Jan 12, 2023

Artturin commented Apr 28, 2023

ghost commented May 1, 2023

glibc: add enableCopyLibGccHack #210112

glibc: add enableCopyLibGccHack #210112

Conversation

ghost commented Jan 10, 2023

Description of changes

Things done

tpwrules commented Jan 11, 2023

ghost commented Jan 11, 2023 • edited by ghost

tpwrules commented Jan 11, 2023

ghost commented Jan 11, 2023

ghost commented Jan 11, 2023 • edited by ghost

tpwrules commented Jan 11, 2023 • edited

ghost commented Jan 11, 2023 • edited by ghost

ghost commented Jan 11, 2023 • edited by ghost

ghost commented Jan 11, 2023

tpwrules commented Jan 11, 2023 • edited

Mic92 commented Jan 11, 2023 • edited

ghost commented Jan 11, 2023 • edited by ghost

tpwrules commented Jan 12, 2023

ghost commented Jan 12, 2023

tpwrules commented Jan 12, 2023

ghost commented Jan 12, 2023

ghost commented Jan 12, 2023 • edited by ghost

tpwrules commented Jan 12, 2023

Artturin commented Apr 28, 2023

ghost commented May 1, 2023

ghost commented Jan 11, 2023 •

edited by ghost

ghost commented Jan 11, 2023 •

edited by ghost

tpwrules commented Jan 11, 2023 •

edited

ghost commented Jan 11, 2023 •

edited by ghost

ghost commented Jan 11, 2023 •

edited by ghost

tpwrules commented Jan 11, 2023 •

edited

Mic92 commented Jan 11, 2023 •

edited

ghost commented Jan 11, 2023 •

edited by ghost

ghost commented Jan 12, 2023 •

edited by ghost