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 #85951

Merged
merged 3 commits into from Apr 27, 2020
Merged

binutils: 2.31.1 -> 2.34 #85951

merged 3 commits into from Apr 27, 2020

Conversation

lovesegfault
Copy link
Member

Motivation for this change

Noticed it was out of date and that #78204 was out of date.

cc. @guibou who did the original PR
cc. @flokli for review
c.f. https://sourceware.org/pipermail/binutils/2020-April/110818.html for discussion on the coreutils test failures reported in #78204 in the binutils ML.

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 mentioned this pull request Apr 24, 2020
10 tasks
@flokli flokli mentioned this pull request Apr 24, 2020
10 tasks
@cole-h
Copy link
Member

cole-h commented Apr 24, 2020

@ofborg eval

Seems to have stalled. Retry!!

@zimbatm
Copy link
Member

zimbatm commented Apr 25, 2020

nixpkgs-review is evaluating fine here. Is it possible that ofborg is running out of memory since the updated graph is so large?

@grahamc
Copy link
Member

grahamc commented Apr 25, 2020

It is unfortunate I didn't notice at the time, but the evaluation tests should never build anything because of precisely this problem: the evaluators are supposed to succeed even in the case of mass rebuilds. @infinisil, maybe you have ideas on ways the lib tests could be evaluation only? Otherwise, I think we'll have to drop that check until it can be made a builder check.

@lovesegfault
Copy link
Member Author

This causes the build of rhash to fail with the weirdest issue I've seen in Nix so far. I'd appreciate someone with more experience taking a look:

❯ nix build -f. --arg overlays '[]' -L rhash
rhash> unpacking sources
rhash> unpacking source archive /nix/store/ljybrllcm43yyabb7dqb768y92zknsmx-source
rhash> cp: setting permissions for 'source': Function not implemented
rhash> do not know how to unpack source archive /nix/store/ljybrllcm43yyabb7dqb768y92zknsmx-source
builder for '/nix/store/sylw6x6r4ph2hdjlzqhfzs4ficy6jxam-rhash-1.3.9.drv' failed with exit code 1; last 4 log lines:
  unpacking sources
  unpacking source archive /nix/store/ljybrllcm43yyabb7dqb768y92zknsmx-source
  cp: setting permissions for 'source': Function not implemented
  do not know how to unpack source archive /nix/store/ljybrllcm43yyabb7dqb768y92zknsmx-source
[0 built (1 failed), 0.0 MiB DL]
error: build of '/nix/store/sylw6x6r4ph2hdjlzqhfzs4ficy6jxam-rhash-1.3.9.drv' failed

@infinisil
Copy link
Member

Oh yeah that's a problem.. Lemme think about that..

@infinisil
Copy link
Member

See above two PR's which should fix the eval problem for ofborg

@flokli
Copy link
Contributor

flokli commented Apr 26, 2020

Regarding #85951 (comment), I spent some time with @LnL7 and @lovesegfault yesterday to debug this.

The failing command here is https://github.com/NixOS/nixpkgs/blob/master/pkgs/stdenv/generic/setup.sh#L813, which handles the case when src is a directory. It's not specific to rhash, but to all packages where src is provided as a directory.

Apparently, with this binutils bump, cp -pr suddenly forgets how to copy over files inside directories that don't have the w chmod set.

I don't really understand why it's the binutils bump that's breaking this, and not coreutils

@pbogdan
Copy link
Member

pbogdan commented Apr 26, 2020

I also don't understand why it's specifically binutils that triggers it but it seems like instance of https://bugzilla.redhat.com/show_bug.cgi?id=1811038 Indeed applying https://src.fedoraproject.org/rpms/coreutils/c/acfa9e81ec1baeea27c2c12f04b5b56c260a3af7?branch=master does fix the issue on the rhash archive.

The difference can be seen in the config.log of coreutils

  • master
    configure:8523: checking for lchmod
    configure:8523: gcc -o conftest -g -O2   conftest.c  >&5
    conftest.c:84:1: error: unknown type name 'choke'
       84 | choke me
          | ^~~~~
    conftest.c:84:9: error: expected ';' before 'int'
       84 | choke me
          |         ^
          |         ;
    ......
       87 | int
          | ~~~      
    configure:8523: $? = 1
    
  • this branch:
    configure:8523: checking for lchmod
    configure:8523: gcc -o conftest -g -O2   conftest.c  >&5
    /nix/store/1hf9d291zalp1c5ywx41ij09b88fdv54-binutils-2.34/bin/ld: /build/ccqVNKSd.o: in function `main':
    /build/coreutils-8.31/conftest.c:90: warning: lchmod is not implemented and will always fail
    configure:8523: $? = 0
    configure:8523: result: yes
    

I also guess something similar may affect other packages as well
but again no clue how this is related to binutils upgrade ¯_(ツ)_/¯ or why the problem only shows up now.

EDIT: the configure check program is:

#define PACKAGE_NAME "GNU coreutils"
#define PACKAGE_TARNAME "coreutils"
#define PACKAGE_VERSION "8.31"
#define PACKAGE_STRING "GNU coreutils 8.31"
#define PACKAGE_BUGREPORT "bug-coreutils@gnu.org"
#define PACKAGE_URL "https://www.gnu.org/software/coreutils/"
#define PACKAGE "coreutils"
#define VERSION "8.31"
#define STDC_HEADERS 1
#define HAVE_SYS_TYPES_H 1
#define HAVE_SYS_STAT_H 1
#define HAVE_STDLIB_H 1
#define HAVE_STRING_H 1
#define HAVE_MEMORY_H 1
#define HAVE_STRINGS_H 1
#define HAVE_INTTYPES_H 1
#define HAVE_STDINT_H 1
#define HAVE_UNISTD_H 1
#define __EXTENSIONS__ 1
#define _ALL_SOURCE 1
#define _DARWIN_C_SOURCE 1
#define _GNU_SOURCE 1
#define _NETBSD_SOURCE 1
#define _OPENBSD_SOURCE 1
#define _POSIX_PTHREAD_SEMANTICS 1
#define __STDC_WANT_IEC_60559_ATTRIBS_EXT__ 1
#define __STDC_WANT_IEC_60559_BFP_EXT__ 1
#define __STDC_WANT_IEC_60559_DFP_EXT__ 1
#define __STDC_WANT_IEC_60559_FUNCS_EXT__ 1
#define __STDC_WANT_IEC_60559_TYPES_EXT__ 1
#define __STDC_WANT_LIB_EXT2__ 1
#define __STDC_WANT_MATH_SPEC_FUNCS__ 1
#define _TANDEM_SOURCE 1
#define _HPUX_ALT_XOPEN_SOCKET_API 1
#define HAVE_FSEEKO 1
#define _DARWIN_USE_64_BIT_INODE 1
#define _REENTRANT 1
#define _THREAD_SAFE 1
#define HAVE_FCHMOD 1
#define HAVE_PATHCONF 1
#define HAVE_BTOWC 1
#define HAVE_USELOCALE 1
#define HAVE_CANONICALIZE_FILE_NAME 1
#define HAVE_REALPATH 1
#define HAVE_READLINKAT 1
#define HAVE_CHOWN 1
#define HAVE_FCHOWN 1
#define HAVE_FCHDIR 1
#define HAVE_FDOPENDIR 1
#define HAVE_FACCESSAT 1
#define HAVE_EXPLICIT_BZERO 1
#define HAVE_POSIX_FADVISE 1
#define HAVE_FCHMODAT 1
/* end confdefs.h.  */
/* Define lchmod to an innocuous variant, in case <limits.h> declares lchmod.
   For example, HP-UX 11i <limits.h> declares gettimeofday.  */
#define lchmod innocuous_lchmod

/* System header to define __stub macros and hopefully few prototypes,
    which can conflict with char lchmod (); below.
    Prefer <limits.h> to <assert.h> if __STDC__ is defined, since
    <limits.h> exists even on freestanding compilers.  */

#ifdef __STDC__
# include <limits.h>
#else
# include <assert.h>
#endif

#undef lchmod

/* Override any GCC internal prototype to avoid an error.
   Use char because int might match the return type of a GCC
   builtin and then its argument prototype would still apply.  */
#ifdef __cplusplus
extern "C"
#endif
char lchmod ();
/* The GNU C library defines this for functions which it implements
    to always fail with ENOSYS.  Some functions are actually named
    something starting with __ and the normal name is an alias.  */
#if defined __stub_lchmod || defined __stub___lchmod
choke me
#endif

int
main ()
{
return lchmod ();
  ;
  return 0;
}

EDIT: Hmm, looking at why the test program fails / succeeds seems to come down to whether __stub_lchmod is defined by glibc - on master I see a definition in include/gnu/stubs-64.h which is absent from glibc built from this PR...

@lovesegfault
Copy link
Member Author

@pbogdan you did it! That was the issue! me and @flokli just verified that setting ac_cv_func_lchmod=no fixes the issues we had seen.

@cole-h
Copy link
Member

cole-h commented Apr 26, 2020

Pinged on IRC, but posting here for prosperity: the build-components-separately.patch is used by libopcodes and libbfd and as such should not be removed from the tree.

EDIT:

../../tools/misc/binutils/build-components-separately.patch

../../tools/misc/binutils/build-components-separately.patch

lovesegfault and others added 3 commits April 26, 2020 15:17
Bumping binutils to 2.32 broke coreutils for unknown reasons[1]. Upon
further investigation we found that there was some issue with mknod
inside a chroot[2][3], which setting ac_cv_func_lchmod to "no" _somehow_
fixes.

We apply this fix and hope to never have to debug this again.

Thanks to @flokli and @LnL7 who helped me chase after this and @pbogdan
who found the fix in the fedora build.

[1]: NixOS#85951
[2]: NixOS#85951 (comment)
[3]: https://bugzilla.redhat.com/show_bug.cgi?id=1811038
- 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

Good catch, thanks @cole-h!

I reverted that deletion

@lovesegfault lovesegfault deleted the binutils-2.34 branch April 27, 2020 16:54
@lovesegfault lovesegfault mentioned this pull request Apr 27, 2020
@Ma27 Ma27 mentioned this pull request Apr 28, 2020
10 tasks
Ma27 added a commit to Ma27/nixpkgs that referenced this pull request Apr 28, 2020
The patch to separate it from `binutils` didn't apply anymore after
updating `binutils` to 2.34[1].

[1] NixOS#85951
@flokli
Copy link
Contributor

flokli commented Apr 29, 2020

This seems to have broken the kernel builds:

nix-build -A linux_5_5
[…]
DEPMOD  5.5.19
rsync: failed to set permissions on "/nix/store/v1q8m9vsvjhc5zp0rdrzq62lhbqyddz5-linux-5.5.19-dev/lib/modules/5.5.19/source/.COPYING.5uJvZC": Function not implemented (38)
rsync: failed to set permissions on "/nix/store/v1q8m9vsvjhc5zp0rdrzq62lhbqyddz5-linux-5.5.19-dev/lib/modules/5.5.19/source/.CREDITS.ZjYYtD": Function not implemented (38)

After a git revert c5f602f5b764ac6ee8e1e0c1b1b3ead88c381da7 c5e3472e2072ed0998571ce3961b8df65e2862cc efdb29597a76c526dc0d9a55f8adf2ec5c33b0ee, it works again.

@thefloweringash
Copy link
Member

thefloweringash commented May 5, 2020

I looked in to what's going on here. I think the root cause is mixing binutils 2.31 with binutils >= 2.32.

Some details:

Various downstream builds seem to fail due to using lchmod, and can be fixed by specifying set ac_cv_func_lchmod=no as in c5e3472. In the version of glibc we use, lchmod is not implemented and should always result in checking for lchmod... no. Even though glibc doesn't implement lchmod, it does provide a stub for it. When a function is stubbed, an entry is written into stubs.h that autoconf uses to exclude stubs from its results. This file lives at ${glibc.dev}/include/gnu/stubs-64.h (on x86_64, it differs per platform). In this branch this file contains no stubs and the incorrect yes result is returned.

So why does this file contain no stubs? The build log for a failed glibc shows:

(cd /build/build/csu/.; objdump -h init-first.o libc-start.o sysdep.o version.o [...]
gawk '/\.gnu\.glibc-stub\./ { \
          sub(/\.gnu\.glibc-stub\./, "", $2); \
          stubs[$2] = 1; } \
        END { for (s in stubs) print "#define __stub_" s }' > /build/build/csu/stubsT
objdump: init-first.o: unable to initialize decompress status for section .debug_info
objdump: init-first.o: unable to initialize decompress status for section .debug_info
objdump: init-first.o: file format not recognized
objdump: libc-start.o: unable to initialize decompress status for section .debug_info
objdump: libc-start.o: unable to initialize decompress status for section .debug_info
objdump: libc-start.o: file format not recognized
objdump: sysdep.o: unable to initialize decompress status for section .debug_info
objdump: sysdep.o: unable to initialize decompress status for section .debug_info
objdump: sysdep.o: file format not recognized
objdump: version.o: unable to initialize decompress status for section .debug_info
objdump: version.o: unable to initialize decompress status for section .debug_info
objdump: version.o: file format not recognized
[...]

A shell pipeline has masked a critical error in the glibc build.

The internet says this is a mutual incompatibility between binutils 2.31 and binutils >= 2.32 due to a bug fix appearing in binutils 2.32.

I'm not yet familiar with how stdenv bootstrapping transitions from bootstrap binary binutils (ie, 2.31, I assume) to the in-tree version (2.34, as of this PR). A simple resolution would be a synchronized update of the bootstrap binaries and the in-tree version such that old and new never mix. I'm hoping there's a simpler resolution from tweaking the bootstrapping.

@lovesegfault
Copy link
Member Author

cc. @matthewbauer for bootstrapping questions

@lovesegfault lovesegfault mentioned this pull request May 5, 2020
10 tasks
@lovesegfault
Copy link
Member Author

Continued in #86954

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

9 participants