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

glibc: copy libgcc_s.so from .lib output if it exists #209055

Merged
merged 1 commit into from Jan 14, 2023

Conversation

trofi
Copy link
Contributor

@trofi trofi commented Jan 4, 2023

Otherwise copy it from the default output. The difference is visible when we build glibc with:

  • bootstrapTools gcc: ${stdenv.cc.cc.out}/lib/libgcc_s.so.1 is used
  • nixpkgs gcc: ${stdenv.cc.cc.lib}/lib/libgcc_s.so.1 is used

Noticed when experimented with multiple gcc rebuilds in bootstrap.

While at it killing RUNPATH reference to bootstrap glibc. We expect libgcc_s.so.1 to be used with currently installed glibc anyway.

Description of changes
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.05 Release Notes (or backporting 22.11 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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@ghost
Copy link

ghost commented Jan 9, 2023

@ofborg eval

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

I'm okay with this PR minus the three lines I suggest deleting below. They defeat an important sanity check in stdenv. Do we really need to nuke the references?

pkgs/development/libraries/glibc/default.nix Outdated Show resolved Hide resolved
@ghost ghost mentioned this pull request Jan 9, 2023
4 tasks
@trofi
Copy link
Contributor Author

trofi commented Jan 9, 2023

I'm okay with this PR minus the three lines I suggest deleting below. They defeat an important sanity check in stdenv. Do we really need to nuke the references?

Yes, we have to do it: libgcc_s.so.1 is linked against bootstrap glibc (as in, different glibc than the one we just built). It's usually a bootstrapTools glibc.

If we don't remove references then stdenv will still directly refer bootstrapTools and fail at reference check. What is worse: final stdenv will then have 2 glibc versions in it's closure (and RUNPATHs): one for just built glibc and another - for libgcc_s.so.1 right within /lib directory of just built glibc. Loading such a libgcc_s.so.1 would be unsafe as it would try to pull in libraries of second glibc into an address space.

Currently (where we copy libgcc_s.so.1 from bootstrapTools) it happens to work because we already nuked references at bootstrapTools tarball creation time.

Ideal solution would be to get rid of libgcc_s.so.1 copy. I'd like to do it at some point as well, but I don't have concrete picture how it would look like.

@trofi trofi requested a review from Ericson2314 January 9, 2023 21:20
@ghost
Copy link

ghost commented Jan 10, 2023

If --shrink-rpath isn't enough to solve the problem, that is a very worrying sign.

I'm still waiting for my x86 builders to catch up so I can try that.

@trofi
Copy link
Contributor Author

trofi commented Jan 10, 2023

It's not enough. We copy file already built against a different glibc. We have to mangle it to repin it against newer glibc.

@ghost ghost mentioned this pull request Jan 10, 2023
3 tasks
@ghost
Copy link

ghost commented Jan 12, 2023

Do we really need to nuke the references?

Yes, we have to do it: libgcc_s.so.1

You're right. I was wrong about this; I get it now.

Could you please mark this resolved?

@ghost
Copy link

ghost commented Jan 12, 2023

@ofborg eval

@ghost
Copy link

ghost commented Jan 12, 2023

@trofi trofi force-pushed the glibc-consistent-copy-of-libgcc_s.so.1 branch from e87c7f0 to 1d93e69 Compare January 14, 2023 08:15
@trofi
Copy link
Contributor Author

trofi commented Jan 14, 2023

gmp-with-cxx-6.2.1 crashes when $ nix build -f lib/tests/release.nix is ran. Looking at the test details.

@trofi trofi marked this pull request as draft January 14, 2023 08:50
@trofi
Copy link
Contributor Author

trofi commented Jan 14, 2023

Oh, it even breaks stdenv build. That's not good. Switching to draft to poke a bit more.

@trofi
Copy link
Contributor Author

trofi commented Jan 14, 2023

Crashes in gmp-with-cxx as a dependency of gcc:

$ nix build --no-link -f. stdenv --keep-going
error: builder for '/nix/store/nsk8abj7kbfh07234b2ry84kmawccc99-gmp-with-cxx-6.2.1.drv' failed with exit code 2;
       last 10 log lines:
       > make[5]: Leaving directory '/build/gmp-6.2.1/tests/cxx'
       > make[4]: *** [Makefile:1097: check-TESTS] Error 2
       > make[4]: Leaving directory '/build/gmp-6.2.1/tests/cxx'
       > make[3]: *** [Makefile:1315: check-am] Error 2
       > make[3]: Leaving directory '/build/gmp-6.2.1/tests/cxx'
       > make[2]: *** [Makefile:823: check-recursive] Error 1
       > make[2]: Leaving directory '/build/gmp-6.2.1/tests'
       > make[1]: *** [Makefile:997: check-recursive] Error 1
       > make[1]: Leaving directory '/build/gmp-6.2.1'
       > make: *** [Makefile:1289: check] Error 2
       For full logs, run 'nix log /nix/store/nsk8abj7kbfh07234b2ry84kmawccc99-gmp-with-cxx-6.2.1.drv'.
error: 1 dependencies of derivation '/nix/store/vz6xhj7s1dvqhn2kv59cvgwbb1czc0k9-isl-stage3-0.20.drv' failed to build
error: 1 dependencies of derivation '/nix/store/8fvr0ija739lafp0x8lz8l0fyvk43qlx-mpfr-4.1.1.drv' failed to build
error: 1 dependencies of derivation '/nix/store/py2x1dvgkvdz7bic84xrr1kni50kd8z4-mpfr-stage3-4.1.1.drv' failed to build
error: 2 dependencies of derivation '/nix/store/rqq52r9w558sfk3nyhy6nbfcp12f3g9d-libmpc-stage3-1.2.1.drv' failed to build

Looking at the crash libstdc++.so.6 is loaded from bootstrapTools's lib directory (would probably use bootstrapTools glibc) while glibc used is already from nixpkgs:

   1831407:     calling init: /nix/store/ys9d0j18y9zfjx2fmmb2cjdivp2nv4z4-glibc-2.35-224/lib/ld-linux-x86-64.so.2
   1831407:     calling init: /nix/store/ys9d0j18y9zfjx2fmmb2cjdivp2nv4z4-glibc-2.35-224/lib/libc.so.6
   1831407:     calling init: /nix/store/ys9d0j18y9zfjx2fmmb2cjdivp2nv4z4-glibc-2.35-224/lib/libgcc_s.so.1
   1831407:     calling init: /nix/store/ys9d0j18y9zfjx2fmmb2cjdivp2nv4z4-glibc-2.35-224/lib/libm.so.6
   1831407:     calling init: /nix/store/p4s4jf7aq6v6z9iazll1aiqwb34aqxq9-bootstrap-tools/lib/libstdc++.so.6

Current staging does not seem broken. Cherry-picking b470a6b
linux/bootstrap-tools: move libstdc++ out of default library search path alone does not make the crash go away.

I'll try to bisect the fix to see if it's intentional and rebase against staging.

@trofi trofi force-pushed the glibc-consistent-copy-of-libgcc_s.so.1 branch from 1d93e69 to e420544 Compare January 14, 2023 11:19
@trofi
Copy link
Contributor Author

trofi commented Jan 14, 2023

Even rebase against staging keeps gmp-with-cxx crashing, which makes some sense given it mixes in unexpected libstdc++. Need to explore a bit more.

Otherwise copy it from the default output. The difference is visible
when we build `glibc` with:

- `bootstrapTools` `gcc`: ${stdenv.cc.cc.out}/lib/libgcc_s.so.1 is used
- `nixpkgs` `gcc`: ${stdenv.cc.cc.lib}/lib/libgcc_s.so.1 is used

Noticed when experimented with multiple `gcc` rebuilds in bootstrap.

While at it killing `RUNPATH` reference to bootstrap `glibc`.
@trofi trofi force-pushed the glibc-consistent-copy-of-libgcc_s.so.1 branch from e420544 to 76f5618 Compare January 14, 2023 11:59
@trofi
Copy link
Contributor Author

trofi commented Jan 14, 2023

Instead of force-pulling new libc's RPATH we can maintain status quo and use binary's default:

--- a/pkgs/development/libraries/glibc/default.nix
+++ b/pkgs/development/libraries/glibc/default.nix
@@ -83,7 +83,9 @@ in
           cp -a ${lib.getLib stdenv.cc.cc}/lib/libgcc_s.so $out/lib/libgcc_s.so
           # wipe out reference to previous libc it was built against
           chmod +w $out/lib/libgcc_s.so.1
-          patchelf --set-rpath $out/lib $out/lib/libgcc_s.so.1
+          # rely on default RUNPATHs of the binary and other libraries
+          # Do no force-pull wrong glibc.
+          patchelf --remove-rpath $out/lib/libgcc_s.so.1
       fi
     '';

It's more forgiving and should not cause more breakage, than the variant with nuked reference.

@trofi trofi merged commit d4d82a6 into NixOS:staging Jan 14, 2023
@trofi trofi deleted the glibc-consistent-copy-of-libgcc_s.so.1 branch January 14, 2023 15:53
@Ericson2314
Copy link
Member

Looks good!

chmod +w $out/lib/libgcc_s.so.1
# rely on default RUNPATHs of the binary and other libraries
# Do no force-pull wrong glibc.
patchelf --remove-rpath $out/lib/libgcc_s.so.1
Copy link

Choose a reason for hiding this comment

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

Note that patchelf --remove-rpath doesn't quite completely remove the RPATH. Unfortunately the fix won't be in the bootstrap-files. I've been using --set-rpath "" as a workaround.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I had to slap nuke-refs on top: #210741

@ghost
Copy link

ghost commented Jan 15, 2023

Even rebase against staging keeps gmp-with-cxx crashing, which makes some sense given it mixes in unexpected libstdc++. Need to explore a bit more.

The way gcc deals with libstdc++ is a big headache. For #209870 I build the first copy of gcc using a c-only (no c++) copy of gmp.

Long term, once we have control of the gcc bootstrap process again I would like build libstdc++ separately from gcc. I think that will put us in control of the decisions that are causing these issues. Right now we sort of just have to accept whatever gcc's Makefile feels like doing, and I suspect in a few cases it's not doing the right thing (or at least not doing what's right for nixpkgs' needs).

@wegank wegank mentioned this pull request Jan 16, 2023
13 tasks
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.

None yet

2 participants