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

binutils: 2.31.1 -> 2.34 #86954

Merged
merged 3 commits into from May 11, 2020
Merged

binutils: 2.31.1 -> 2.34 #86954

merged 3 commits into from May 11, 2020

Conversation

@lovesegfault
Copy link
Contributor

lovesegfault commented May 5, 2020

Motivation for this change

Trying #85951 again as it had to be reverted.

cc. @flokli @pbogdan @thefloweringash

Closes #78204

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.
@lovesegfault lovesegfault requested review from flokli, cleverca22 and vcunat and removed request for flokli May 5, 2020
@lovesegfault
Copy link
Contributor Author

lovesegfault commented May 5, 2020

Please see #85951 (comment) and #85951 (comment).

Still unclear on how to solve the (apparent) bootstrapping issue.

@pbogdan
Copy link
Member

pbogdan commented May 5, 2020

I don't feel confident enough to say if it's a good / bad idea but this patch:

diff --git a/pkgs/development/libraries/glibc/common.nix b/pkgs/development/libraries/glibc/common.nix
index 0429c7295fb..52fa7191cb7 100644
--- a/pkgs/development/libraries/glibc/common.nix
+++ b/pkgs/development/libraries/glibc/common.nix
@@ -203,7 +203,7 @@ stdenv.mkDerivation ({
     configureScript="`pwd`/../$sourceRoot/configure"
 
     ${lib.optionalString (stdenv.cc.libc != null)
-      ''makeFlags="$makeFlags BUILD_LDFLAGS=-Wl,-rpath,${stdenv.cc.libc}/lib"''
+      ''makeFlags="$makeFlags BUILD_LDFLAGS=-Wl,-rpath,${stdenv.cc.libc}/lib OBJDUMP=${stdenv.cc.bintools.bintools}/bin/objdump"''
     }

does fix the issue with the generation of the stub functions and at least confirms what @thefloweringash found. I don't know if there's a better way to somehow line up the binutils tools that are included in the stdenv with other tools like objdump which at that stage get pulled from the bootstrap tools as I understand it.

guibou and others added 3 commits Jan 21, 2020
- I've removed the stack of patch linked to
https://sourceware.org/bugzilla/show_bug.cgi?id=23428 . The associated
issue says it is closed and targeted for 2.32.

- I've ugraded the "no_plugin" patch. The logic changed in
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commitdiff;h=41f37a6fb71f2a3de388108f5cdfca9cbe6e9d51
and I tried to keep the same logic by disabling everything.

It closes #78197
@lovesegfault lovesegfault force-pushed the lovesegfault:binutils-2.34 branch from 469c61c to b83fb95 May 9, 2020
@lovesegfault
Copy link
Contributor Author

lovesegfault commented May 9, 2020

Alright, @pbogdan's patch solved the issue and the full bootstrap seems to work now. Managed to build opencv, linux_latest, and twelf, all of which failed previously.

@flokli
Copy link
Contributor

flokli commented May 10, 2020

@lovesegfault just to confirm - with "bootstrap", you meant building opencv, linux_latest and twelf, not creating new bootstrap binaries, right?

@lovesegfault
Copy link
Contributor Author

lovesegfault commented May 10, 2020

@flokli That's correct.

@flokli
Copy link
Contributor

flokli commented May 11, 2020

I'd say let's cook this in staging :-)

@flokli flokli merged commit 629fa8a into NixOS:staging May 11, 2020
16 checks passed
16 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
binutils, binutils.passthru.tests, glibc, glibc.passthru.tests on aarch64-linux Success
Details
binutils, binutils.passthru.tests, glibc, glibc.passthru.tests on x86_64-linux Success
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="b83fb95"; rev="b83fb95a41c94920b89db9f9bf6859e4f0d3a10d"; } ./pkgs/t
Details
grahamcofborg-eval-lib-tests nix-build --arg pkgs import ./. {} ./lib/tests/release.nix
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="b83fb95"; rev="b83fb95a41c94920b89db9f9bf6859e4f0d3a10d"; } ./nixos/
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="b83fb95"; rev="b83fb95a41c94920b89db9f9bf6859e4f0d3a10d"; } ./nixos/
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="b83fb95"; rev="b83fb95a41c94920b89db9f9bf6859e4f0d3a10d"; } ./nixos/
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="b83fb95"; rev="b83fb95a41c94920b89db9f9bf6859e4f0d3a10d"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="b83fb95"; rev="b83fb95a41c94920b89db9f9bf6859e4f0d3a10d"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="b83fb95"; rev="b83fb95a41c94920b89db9f9bf6859e4f0d3a10d"; } ./pkgs/t
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@flokli flokli mentioned this pull request May 14, 2020
0 of 10 tasks complete
@FRidh
Copy link
Member

FRidh commented May 17, 2020

Fix for a missing change c10f0a3

Note Python's cffi's tests fail with this bump.

@flokli
Copy link
Contributor

flokli commented May 17, 2020

@FRidh I'm not sure I really understand some of these test failures:

______________________________ test_load_library _______________________________

    def test_load_library():
>       x = find_and_load_library('c')

c/test_c.py:73: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

name = 'c', flags = 2

    def find_and_load_library(name, flags=RTLD_NOW):
        import ctypes.util
        if name is None:
            path = None
        else:
            path = ctypes.util.find_library(name)
            if path is None and name == 'c':
>               assert sys.platform == 'win32'
E               AssertionError: assert 'linux' == 'win32'
E                 - win32
E                 + linux

Why does this code think the platform should be win32? 😕

@FRidh
Copy link
Member

FRidh commented May 18, 2020

Most likely because find_library returned None, and that is something that absolutely should not happen.

$ nix-shell -p python3 --pure --run "python3 -c 'import ctypes.util; print(ctypes.util.find_library(\"c\"))'"

functions fine though.

@flokli
Copy link
Contributor

flokli commented May 22, 2020

@lovesegfault any idea what's going on here?

@lovesegfault
Copy link
Contributor Author

lovesegfault commented May 22, 2020

I do not, maybe @pbogdan or @thefloweringash do.

We may need to revert this again :(

@flokli flokli mentioned this pull request May 22, 2020
7 of 14 tasks complete
@pbogdan
Copy link
Member

pbogdan commented May 22, 2020

I don't know either, sorry :-(. Agreed it might need another revert to not block other important changes in staging.

@thefloweringash
Copy link
Member

thefloweringash commented May 23, 2020

find_library("c") is calling ld -t -o /dev/null -lc in _findLib_ld. In current master, this returns only shared objects while on staging this is returning scripts and archives too. Python takes the first result that matches a simple filter and tries to use it as a shared object. The first returned item is now libc.so, instead of libc.so.6. libc.so is a plain-text script, so this fails and ultimately returns None. There was a change in binutils (727a29b) that looks suspicious:

Report scripts and libraries searched for ld --trace

It seems like python's _findLib_ld doesn't handle this change. This probably isn't a widespread problem since it's a fallback method for find_library.


# recent unstable
$ nix-shell --pure -p hello --run 'ld -t -o /dev/null -lc'
/nix/store/a57856fs4m8ir6vlv14h3gq3sv9aq2lb-binutils-2.31.1/bin/ld: mode elf_x86_64
/nix/store/nwsn18fysga1n5s0bj4jp4wfwvlbx8b1-glibc-2.30/lib/libc.so.6
/nix/store/nwsn18fysga1n5s0bj4jp4wfwvlbx8b1-glibc-2.30/lib/ld-linux-x86-64.so.2
/nix/store/nwsn18fysga1n5s0bj4jp4wfwvlbx8b1-glibc-2.30/lib/ld-linux-x86-64.so.2
/nix/store/a57856fs4m8ir6vlv14h3gq3sv9aq2lb-binutils-2.31.1/bin/ld: warning: cannot find entry symbol _start; not setting start address
# current staging
$ nix-shell --pure -p hello --run 'ld -t -o /dev/null -lc'
/nix/store/j1l6ds4mhm97nqw965w9sg07i9fric4d-glibc-2.30/lib/libc.so
/nix/store/j1l6ds4mhm97nqw965w9sg07i9fric4d-glibc-2.30/lib/libc.so.6
/nix/store/j1l6ds4mhm97nqw965w9sg07i9fric4d-glibc-2.30/lib/libc_nonshared.a
/nix/store/j1l6ds4mhm97nqw965w9sg07i9fric4d-glibc-2.30/lib/ld-linux-x86-64.so.2
/nix/store/j1l6ds4mhm97nqw965w9sg07i9fric4d-glibc-2.30/lib/libc_nonshared.a
/nix/store/j1l6ds4mhm97nqw965w9sg07i9fric4d-glibc-2.30/lib/ld-linux-x86-64.so.2
/nix/store/j284mk8hdv9ccxfdlhcdk1lg2jml4fhk-binutils-2.34/bin/ld: warning: cannot find entry symbol _start; not setting start address
@FRidh
Copy link
Member

FRidh commented May 23, 2020

Thanks for investigating @thefloweringash . I did not realize libc.so was a text file. So, in my test I forgot to set nixpkgs=., sigh. Indeed, it will return None as expected to be the problem.

Using _findLib_ld is actually the only method we use for finding libraries, considering the other methods (ldconfig, gcc) have been patched out.

To me this seems now like an upstream Python/binutils issue. Either we can try and fix the regex ourselves, or we need to revert the binutils update and try yet again later.

Created the following upstream Python issue https://bugs.python.org/issue40739.

@thefloweringash
Copy link
Member

thefloweringash commented May 23, 2020

Either we can try and fix the regex ourselves

I don't think this can be fixed by filtering the list by filename. A *.so file is an input for GNU ld which might be a shared library, symlink to a shared library, or a linker script as seen here. ld is now reporting all files and revealing the presence of the linker script. It's likely sufficient to return the first thing that's actually a shared library, but I don't see a way to do that without opening each file and inspecting the contents. An easy way would be to go through the list and return the first one that returns a value from _get_soname.

@FRidh
Copy link
Member

FRidh commented May 23, 2020

Actually, we may not have to fix this. In principle we do not support find_library. Packages are required to include hardcoded paths to libraries, thereby circumventing load_library.

@FRidh
Copy link
Member

FRidh commented May 23, 2020

numpy also fails, but with "ELF load command address/offset not properly aligned". Too much stripped? https://hydra.nixos.org/build/118901023

$ ldd /nix/store/riqyzfjpskh0qf46f5fjc8ldqbp8aj85-python3.7-numpy-1.18.4/lib/python3.7/site-packages/numpy/core/_multiarray_umath.cpython-37m-x86_64-linux-gnu.so 
/nix/store/riqyzfjpskh0qf46f5fjc8ldqbp8aj85-python3.7-numpy-1.18.4/lib/python3.7/site-packages/numpy/core/_multiarray_umath.cpython-37m-x86_64-linux-gnu.so: error while loading shared libraries: liblapack.so.3: ELF load command address/offset not properly aligned

Reverting numpy to 1.18.3 did not fix it.

@vcunat
Copy link
Member

vcunat commented May 23, 2020

So... we revert binutils for a while again? So far I haven't noticed anything that needs the new version. I'd hope that upstream helps to resolve this within a couple weeks.

@FRidh
Copy link
Member

FRidh commented May 23, 2020

Yep, already doing the revert.

FRidh added a commit to FRidh/nixpkgs that referenced this pull request May 23, 2020
Pythons find_library is broken with binutils 2.34, and numpy could not import libraries because of not properly aligned ELF's.

This is the second time binutils 2.34 got reverted. Next time, we should have a dedicated Hydra job for it.

This reverts commit 629fa8a, reversing
changes made to 4ddd080.
FRidh added a commit to FRidh/nixpkgs that referenced this pull request May 23, 2020
Reverting this change again because we're going back to binutils 2.31.

NixOS#86954 (comment)

This reverts commit ade7fae.
@vcunat
Copy link
Member

vcunat commented May 23, 2020

Hmm, CVEs are reported for 2.31.1: #63061 so I wonder if there's a version in-between that fixes them but doesn't break python yet.

EDIT: well, we haven't dealt with these in over a year, so perhaps we can just delay a couple weeks longer 😈

@lovesegfault
Copy link
Contributor Author

lovesegfault commented May 23, 2020

The battle to bump binutils rages on! See you guys in the next episode of the binutils Iliad

@luc65r luc65r mentioned this pull request Jun 8, 2020
10 of 29 tasks complete
luc65r added a commit to luc65r/nixpkgs that referenced this pull request Jun 8, 2020
luc65r added a commit to luc65r/nixpkgs that referenced this pull request Jun 11, 2020
luc65r added a commit to luc65r/nixpkgs that referenced this pull request Jun 12, 2020
luc65r added a commit to luc65r/nixpkgs that referenced this pull request Jun 16, 2020
luc65r added a commit to luc65r/nixpkgs that referenced this pull request Jul 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
You can’t perform that action at this time.