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

pkgsStatic: Pass hostPlatform.gcc attribute #283460

Merged
merged 1 commit into from
May 16, 2024

Conversation

rodarima
Copy link
Contributor

To build the security wrappers1 the pkgsStatic stdenv is used, so the binaries are static. However, the hostPlatform may have gcc attributes that are required to build binaries so they can run on the host platform. In particular, this is the case when using gcc.arch, which ends up injecting -march=... in the gcc wrapper. Those attributes are not contained in hostPlatform.parsed.

This change sets the same gcc attributes found in the hostPlatform for the pkgsStatic cross system, so it can build binaries with the same gcc flags.


This problem has ocurred to me while cross compiling a RISC-V NixOS system from an x86 machine to run in QEMU with the compressed instructions disabled (using -cpu rv64,c=false). What happens when the -march flags are not propagated to the gcc wrapper is that the binaries end up being compiled with compressed instructions which cause a SIGILL like the following:

$ mount
[52637.699560] mount[2334]: unhandled signal 4 code 0x1 at 0x000000000001048c in mount[10000+e000]
[52637.702177] CPU: 19 PID: 2334 Comm: mount Not tainted 6.1.72 #1-NixOS
[52637.703517] Hardware name: riscv-virtio,qemu (DT)
[52637.705091] epc : 000000000001048c ra : 000000000004a0dc sp : 00ffffffc5588bf0
[52637.706789]  gp : 000000000001f800 tp : 00ffffffabed63a0 t0 : ffffffffffffffff
[52637.708603]  t1 : 0000000000030a8c t2 : 00000000001952a8 s0 : 00000000001f29c0
[52637.710191]  s1 : 00000000001f29c0 a0 : 0000000000000000 a1 : 00000000001807c0
[52637.711965]  a2 : 0000000000180af0 a3 : 0000000000000000 a4 : 0000000000116010
[52637.713687]  a5 : 0000000000000000 a6 : 00000000001807c0 a7 : 00000000000000dd
[52637.715348]  s2 : 000000000010cefc s3 : 00000000001805c0 s4 : 00000000001807c0
[52637.717137]  s5 : 00ffffffac1a8d50 s6 : 0000000000180af0 s7 : 00000000001807c0
[52637.718808]  s8 : 0000000000117890 s9 : 0000000000000000 s10: 00000000001134c0
[52637.720686]  s11: 00000000001134cc t3 : 00ffffffabf9f348 t4 : 918a9092d091969d
[52637.722370]  t5 : 0000000000000010 t6 : 0000000000000007
[52637.723562] status: 0000000200004020 badaddr: 000000000000850a cause: 0000000000000002
Illegal instruction (core dumped)

Description of changes

Things done

  • Built on platform(s)
    • x86_64-linux (cross compilation to RISC-V)
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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 some binary files (usually in ./result/bin/)
    • I tested mount and sudo which run okay now.
  • 24.05 Release Notes (or backporting 23.05 and 23.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
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@rodarima rodarima requested a review from Ericson2314 as a code owner January 24, 2024 10:19
@@ -272,6 +272,7 @@ let
if stdenv.isLinux
then makeMuslParsedPlatform stdenv.hostPlatform.parsed
else stdenv.hostPlatform.parsed;
gcc = lib.optionalAttrs (stdenv.hostPlatform ? gcc) stdenv.hostPlatform.gcc;
} // lib.optionalAttrs (stdenv.hostPlatform.system == "powerpc64-linux") {
gcc.abi = "elfv2";
Copy link
Contributor

Choose a reason for hiding this comment

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

This will not merge correctly on ppc. Also, maybe we should inherit all the values of the platform, and only change isStatic and parsed?

Copy link
Contributor Author

@rodarima rodarima Jan 24, 2024

Choose a reason for hiding this comment

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

This will not merge correctly on ppc

Ups, I'll move the // to the gcc attribute instead.

maybe we should inherit all the values of the platform

I don't know the consecuences of that, so I only wanted to do the minimal changes that fix the current problem. I assume for other pkgs* sets some similar fix should be done.

@rodarima rodarima force-pushed the fix-pkgs-static-gcc-march branch from c379a0d to 57e7c8f Compare January 24, 2024 10:30
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Jan 24, 2024
Copy link
Member

@alyssais alyssais left a comment

Choose a reason for hiding this comment

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

Makes sense to me.

pkgs/top-level/stage.nix Outdated Show resolved Hide resolved
pkgs/top-level/stage.nix Outdated Show resolved Hide resolved
@rodarima
Copy link
Contributor Author

Can we merge this? NixOS continues to be broken when cross-compiling with specific -march flags.

Otherwise, can you provide workaround so we can switch to upstream nixpkgs? AFAIK, I cannot patch nixpkgs when using it as an input flake, so I cannot take that route either.

To build the security wrappers[1] the pkgsStatic stdenv is used, so the
binaries are static. However, the hostPlatform may have gcc attributes
that are *required* to build binaries so they can run on the host
platform. In particular, this is the case when using gcc.arch, which
ends up injecting -march=... in the gcc wrapper. Those attributes are
not contained in hostPlatform.parsed.

This change sets the same gcc attributes found in the hostPlatform for
the pkgsStatic cross system, so it can build binaries with the same gcc
flags.

[1]: nixos/modules/security/wrappers/default.nix
@rodarima rodarima force-pushed the fix-pkgs-static-gcc-march branch from 02002b1 to e4ee77f Compare May 16, 2024 12:11
@Aleksanaa Aleksanaa merged commit cab94ab into NixOS:master May 16, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants