autoPatchelfHook: fix detection under crossSystem #137351

impl · 2021-09-11T06:47:55Z

Motivation for this change

In #84415, autoPatchelfHook was taught to use the correct path to the readelf binary when a crossSystem is specified. Unfortunately, the remainder of the functionality in the script depended on ldd, which only reads ELF files of its own architecture. It has the further unfortunate quality of not reporting any useful error, but rather that the file is not a dynamic executable.

This change uses patchelf to directly analyze the DT_NEEDED tags in the target files instead, which correctly works across architectures. It also updates the use of objdump to be prefix-aware $OBJDUMP (which would have been required in the PR mentioned above, but we never made it that far into the script execution).

Here is an example of a package that can now be properly cross-patched (?, not sure what the terminology for this is):

let
  pkgs = import <nixpkgs> {
    crossSystem = {
      config = "armv6l-unknown-linux-gnueabihf";
      gcc = {
        arch = "armv6kz";
        fpu = "vfp";
      };
    };
  };
in pkgs.callPackage ({ lib, stdenv, fetchFromGitHub, autoPatchelfHook, glibc, gccForLibs }: stdenv.mkDerivation rec {
  pname = "vcdbg";
  version = "1.20210831";

  src = fetchFromGitHub {
    owner = "raspberrypi";
    repo = "firmware";
    rev = version;
    sha256 = "sha256-NhQwg6DRGSwsULxXWJAOUPIW+JvOnZVDTTvIBZnjzgA=";
  };

  nativeBuildInputs = [ autoPatchelfHook ];
  buildInputs = [ gccForLibs.lib ];

  installPhase = ''
    install -m 644 -D -t $out/lib opt/vc/lib/*.so
    install -m 755 -D opt/vc/bin/vcdbg $out/bin/vcdbg
  '';

  meta = with lib; {
    description = "vcdbg";
    homepage = "https://github.com/raspberrypi/firmware";
    license = licenses.unfreeRedistributableFirmware; # See https://github.com/raspberrypi/firmware/blob/master/boot/LICENCE.broadcom
  };
}) {}

This binary runs correctly even under aarch64 on the Raspberry Pi when built.

Things done

impl · 2021-09-11T07:37:13Z

pkgs/build-support/setup-hooks/auto-patchelf.sh

 }

 populateCacheWithRecursiveDeps() {
    local so found foundso
    for so in "${autoPatchelfCachedDeps[@]}"; do
-        for found in $(getDepsFromSo "$so"); do
+        for found in $(getDepsFromElfBinary "$so"); do


I don't think this change is doing the right thing because the library path isn't evaluated yet. I'll take another pass at this.

according to man ldd ldd prints all recursive dependencies of the executable while readelf/objdump only print direct dependencies, so that looks like a regression.

OK, I think I've addressed both of these now. Let me know what you think!

pkgs/build-support/setup-hooks/auto-patchelf.sh

r-rmcgibbo · 2021-09-11T14:40:38Z

Result of nixpkgs-review pr 137351 at fd5670af run on aarch64-linux 1

50 packages marked as broken and skipped:

Literate
adoptopenjdk-hotspot-bin-13
adoptopenjdk-hotspot-bin-14
adoptopenjdk-jre-hotspot-bin-13
adoptopenjdk-jre-hotspot-bin-14
cassandra_2_1
cassandra_2_2
collectd
collectd-data
dtools
echidna
gtkd
impressive
isabelle
jboss
odpdown
onedrive
openems
opentsdb
ovftool
pig
polymake
processing
python38Packages.cnvkit
python38Packages.foundationdb51
python38Packages.foundationdb52
python38Packages.foundationdb60
python38Packages.foundationdb61
python38Packages.python-csxcad
python38Packages.python-openems
python38Packages.tensorflowWithCuda
python39Packages.cnvkit
python39Packages.foundationdb51
python39Packages.foundationdb52
python39Packages.foundationdb60
python39Packages.foundationdb61
python39Packages.jax
python39Packages.python-csxcad
python39Packages.python-openems
python39Packages.tensorflowWithCuda
qemu_xen
qemu_xen-light
qemu_xen_4_10
qemu_xen_4_10-light
rund
spark
spring
springLobby
tilix
tlaps

51 packages failed to build:

adoptopenjdk-bin (adoptopenjdk-hotspot-bin-11 ,openjdk11-bootstrap)
openjdk8-bootstrap (adoptopenjdk-hotspot-bin-8)
apacheKafka (apacheKafka_2_8)
apacheKafka_2_7
bazel (bazel_3)
bazel_0 (bazel_0_26)
bluej
charles (charles4)
clojure
cytoscape
dbeaver
dbus_java
erlang_javac
filebot
freeplane
gradle_5
gradle_6
graylog
greenfoot
gremlin-console
igv
jdk11 (openjdk11)
jdk11_headless (openjdk11_headless)
jetbrains.clion
jetbrains.datagrip
jetbrains.goland
jetbrains.jdk
jetbrains.mps
jetbrains.phpstorm
jetbrains.pycharm-community
jetbrains.pycharm-professional
jetbrains.rider
jetbrains.ruby-mine
jetbrains.webstorm
jre8 (jre8_headless)
libmatthew_java
logstash (logstash6)
logstash6-oss
metabase
mucommander
netbeans
nzbhydra2
pdfsam-basic
python38Packages.adb-enhanced
python39Packages.adb-enhanced
scenebuilder
signal-cli
smartgithg
soapui
stm32cubemx
tlaplus

479 packages skipped due to time constraints:

DisnixWebService
R
_1password
_7zz
abcl
adapta-gtk-theme
openjdk16-bootstrap (adoptopenjdk-hotspot-bin-15)
adoptopenjdk-hotspot-bin-16
adoptopenjdk-icedtea-web
adoptopenjdk-jre-bin (adoptopenjdk-jre-hotspot-bin-11)
...

Note that build failures may predate this PR, and could be nondeterministic or hardware dependent.
Please exercise your independent judgement. Does something look off? Please file an issue or reach out on IRC.

In NixOS#84415, autoPatchelfHook was taught to use the correct path to the readelf binary when a crossSystem is specified. Unfortunately, the remainder of the functionality in the script depended on ldd, which only reads ELF files of its own architecture. It has the further unfortunate quality of not reporting any useful error, but rather that the file is not a dynamic executable. This change uses patchelf to directly analyze the DT_NEEDED tags in the target files instead, which correctly works across architectures. It also updates the use of objdump to be prefix-aware $OBJDUMP (which would have been required in the PR mentioned above, but we never made it that far into the script execution).

symphorien

could build julia_16-bin natively and cross to aarch64. native julia passes its tests, cross julia can run --version in qemu.

impl · 2021-09-14T20:11:51Z

Just to let you know, I have a few more fixes I'll put into a follow-up PR (I rented a big ol' AWS instance and manually reviewed all of the output of nixpkgs-review pr... :) that should improve stability across existing packages as well. Coming shortly!

impl · 2021-09-15T16:05:59Z

That PR is #137886, for future onlookers!

impl requested a review from aszlig as a code owner September 11, 2021 06:47

ofborg bot added 10.rebuild-darwin: 11-100 10.rebuild-linux: 501+ 10.rebuild-linux: 1001-2500 labels Sep 11, 2021

impl commented Sep 11, 2021

View reviewed changes

SuperSandro2000 requested changes Sep 11, 2021

View reviewed changes

pkgs/build-support/setup-hooks/auto-patchelf.sh Outdated Show resolved Hide resolved

Mindavi added the 6.topic: cross-compilation Building packages on a different platform than they will be used on label Sep 11, 2021

impl force-pushed the autopatchelfhook-cross-fix branch from fd5670a to 77afa34 Compare September 12, 2021 00:14

impl changed the title ~~autoPatchelfHook: Fix detection under crossSystem~~ autoPatchelfHook: fix detection under crossSystem Sep 12, 2021

impl force-pushed the autopatchelfhook-cross-fix branch from 77afa34 to 61733d1 Compare September 12, 2021 00:16

impl force-pushed the autopatchelfhook-cross-fix branch from 61733d1 to b79483d Compare September 12, 2021 22:07

symphorien approved these changes Sep 14, 2021

View reviewed changes

symphorien merged commit 3413d61 into NixOS:master Sep 14, 2021

Janik-Haag added the 12. first-time contribution label Jun 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

autoPatchelfHook: fix detection under crossSystem #137351

autoPatchelfHook: fix detection under crossSystem #137351

impl commented Sep 11, 2021 •

edited

Loading

impl Sep 11, 2021

symphorien Sep 11, 2021

impl Sep 12, 2021

r-rmcgibbo commented Sep 11, 2021

symphorien left a comment

impl commented Sep 14, 2021

impl commented Sep 15, 2021

autoPatchelfHook: fix detection under crossSystem #137351

autoPatchelfHook: fix detection under crossSystem #137351

Conversation

impl commented Sep 11, 2021 • edited Loading

Motivation for this change

Things done

impl Sep 11, 2021

Choose a reason for hiding this comment

symphorien Sep 11, 2021

Choose a reason for hiding this comment

impl Sep 12, 2021

Choose a reason for hiding this comment

r-rmcgibbo commented Sep 11, 2021

symphorien left a comment

Choose a reason for hiding this comment

impl commented Sep 14, 2021

impl commented Sep 15, 2021

impl commented Sep 11, 2021 •

edited

Loading