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

patchelf: increase the page size alignment on Aarch64 to support 64K pages #89317

Closed
wants to merge 1 commit into from

Conversation

delroth
Copy link
Contributor

@delroth delroth commented Jun 1, 2020

Motivation for this change

NixOS built binaries currently do not work on ARMv8 systems configured for 64K wide pages (the more common default is 4K pages, like x86). This is because even though the usual compiler toolchains properly generate ELF files with 64K segment alignment, patchelf will sometimes rewrite segments and align them to 4K instead. The 4K value comes from the fact that the current Hydra ARMv8 builder uses 4K pages, which in itself is kind of undesirable (leaking details of the build infrastructure into packages).

I currently lack enough system diversity to test this (my only ARMv8 box is the one that's broken...), so feedback welcome.

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.

@delroth
Copy link
Contributor Author

delroth commented Jun 1, 2020

Note that this change leaves cross-compilation to aarch64 as "broken" as it is now -- patchelf doesn't know about the target architecture being patched, so it will just use the 4K value from the host instead.

The better solution would be something like NixOS/patchelf#80 .

@delroth
Copy link
Contributor Author

delroth commented Jun 2, 2020

After a lot of rebuilding: I can confirm this leads to a minimal system which boots on both 4K pages and 64K pages configs, and the userland works well enough to build itself on either of the two configurations.

@domenkozar
Copy link
Member

@delroth do you have an idea how to fix this in patchelf itself? The existing patch doesn't seem finished.

@delroth
Copy link
Contributor Author

delroth commented Jun 3, 2020

I think there are basically 2 approaches for a long term fix in patchelf.

  1. What's done in patchelf#80, which is adding some minimal alignment per architecture in patchelf. I don't think there is much work to do on that patch, it probably just needs to be rebased, but it seems fine.
  2. Changing patchelf to just never decrease the alignment when relocating segments -- basically, if a segment has alignment 0x10000, don't pick a new alignment, just reuse the 0x10000. I think this would be a better solution, but not knowing about patchelf I don't know how involved this is.

(1) will still require changes to patchelf for each new architecture with page size != 4K. (2) is theoretically better IMO by just following whatever the toolchain generated.

Both of these options would fix the cross-compilation problem and the reproducibility problem.

@matthewbauer
Copy link
Member

Could patchelf also check for aarch64 and set the proper with-page-size value?

@delroth
Copy link
Contributor Author

delroth commented Jun 20, 2020

I've made an attempt at a patchelf fix in NixOS/patchelf#216

@infinisil
Copy link
Member

I guess this PR isn't relevant anymore with the patchelf PR merged?

@delroth
Copy link
Contributor Author

delroth commented Aug 27, 2020

Yeah, though I'm starting to lose hope of ever seeing a new patchelf release...

@delroth delroth closed this Aug 27, 2020
@delroth delroth deleted the patchelf-64k branch August 27, 2020 02:22
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

4 participants