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
Member

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 15:59
@lovesegfault
Copy link
Member Author

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 May 9, 2020 13:56
- 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 NixOS#78197
@lovesegfault
Copy link
Member Author

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
Member Author

@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
@flokli flokli mentioned this pull request May 14, 2020
10 tasks
@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.

@thefloweringash
Copy link
Member

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
Member Author

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
29 tasks
luc65r added a commit to luc65r/nixpkgs that referenced this pull request Dec 27, 2020
FRidh pushed a commit that referenced this pull request Dec 28, 2020
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

7 participants