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.

Sign up for GitHub

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

Jump to bottom

Add ELFvx ABIs for PowerPC64 #182807

Merged
merged 6 commits into from
Aug 28, 2022
Merged

Add ELFvx ABIs for PowerPC64 #182807

merged 6 commits into from
Aug 28, 2022

Conversation

minijackson
Copy link
Member

@minijackson minijackson commented Jul 25, 2022 •
edited

Description of changes

Little context:

Today the PowerPC ISA has access to several ABIs, notably ELFv1 and ELFv2. If I got the history right, the ELFv1 architecture and PowerPC big-endian came first. Then, ELFv2 was created for PowerPC little-endian, but intentionally in a way that is compatible with PowerPC big-endian.1

In this situation, GCC made the sensible choice of defaulting to ELFv1 for big-endian, and ELFv2 for little-endian in order not to break ABI compatibility with existing compiled code.

Today ELFv1 is less and less used, and some distributions (Gentoo, Void Linux, FreeBSD) are using PowerPC64 big-endian with the ELFv2 ABI by default. I've understood that in NixOS we would want ELFv2 as a default.

However, some packages wrongfully make the assumption that PowerPC64 big-endian implies ELFv1. On the other hand, some packages just don't work with ELFv1, for example the whole musl libc.

---

The -elfv1 and -elfv2 system suffixes were added in #111345, but removed in #116495. This PR adds them back, as gnuabielfv1, gnuabielfv2, and muslabielfv2.

This PR also makes gnuabielfv2 the default for PowerPC64 big endian, which users may find less confusing, if they try crossSystem.system = "powerpc64-linux"; instead of crossSystem = lib.systems.examples.ppc64;.

The GNU ELFv1 ABI is available again, although unless someone is dlopening something outside of Nix, I don't think there are may usecases.

It also adds a convenience isPower64BeElfv2 function, since a few downstream packages needs patches for this specific case.

Note that LLVM is still an issue.

Pinging @r-burns as the author of both aforementioned PRs.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
    • The gcc attribute of pkgs/test's cross tests
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

Also managed to compile a NixOS image with some other patches that I'll be upstreaming.

@minijackson minijackson added the 6.topic: cross-compilation Building packages on a different sort platform than than they will be run on label Jul 25, 2022
@Mindavi Mindavi added the 6.topic: exotic Exotic hardware or software platform label Jul 25, 2022
ghost
ghost reviewed Jul 28, 2022
View reviewed changes
Copy link

@ghost ghost left a comment •
edited by ghost

Choose a reason for hiding this comment

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

LGTM except for trying to find a way we can end up with two ABI identifiers instead of three. If it's not clear what I'm proposing I can write up something more concrete.

lib/systems/doubles.nix Show resolved Hide resolved
pkgs/top-level/stage.nix Outdated Show resolved Hide resolved
@r-burns
Copy link
Contributor

r-burns commented Jul 28, 2022

Nice, I really liked the idea of being able to build for either ABI. The only reason I removed that functionality is that gnu-config seemed to be breaking when passed the nonstandard suffixes, but this doesn't seem to be a problem anymore, so I'm +1 on trying it out again.

@ghost ghost mentioned this pull request Jul 28, 2022
Closed
13 tasks
ghost
ghost reviewed Jul 28, 2022
View reviewed changes
lib/systems/parse.nix Outdated Show resolved Hide resolved
ghost
ghost reviewed Jul 28, 2022
View reviewed changes
lib/systems/examples.nix Outdated Show resolved Hide resolved
ghost
ghost reviewed Jul 28, 2022
View reviewed changes
lib/systems/inspect.nix Outdated Show resolved Hide resolved
ghost
ghost reviewed Jul 28, 2022
View reviewed changes
pkgs/top-level/stage.nix Outdated Show resolved Hide resolved
ghost
ghost reviewed Jul 28, 2022
View reviewed changes
lib/systems/inspect.nix Outdated Show resolved Hide resolved
@ghost
Copy link

ghost commented Jul 28, 2022

gnu-config seemed to be breaking when passed the nonstandard suffixes, but this doesn't seem to be a problem anymore

gnu-config treats anything after -gnuabi or -eabi as a comment:

$ result/config.sub powerpc64-linux-gnuabicrazypants
powerpc64-unknown-linux-gnuabicrazypants

@minijackson minijackson requested a review from a user August 3, 2022 08:51
ghost
ghost approved these changes Aug 3, 2022
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Awesome!

@Mindavi
Copy link
Contributor

Mindavi commented Aug 24, 2022

Can you rebase so the fixups are squashed? I think then this is good to go.

@minijackson
Copy link
Member Author

@Mindavi: done!

@Mindavi
Copy link
Contributor

Mindavi commented Aug 26, 2022

@ofborg build pkgsCross.ppc64.zlib pkgsCross.ppc64-musl.zlib pkgsCross.ppc64.bash

Not sure if this will be done before the 1h time, but let's see.

@Mindavi Mindavi merged commit b2190a3 into NixOS:master Aug 28, 2022
@minijackson minijackson deleted the ppc64-elfv2-abi branch August 28, 2022 20:49
@OPNA2608 OPNA2608 mentioned this pull request Sep 1, 2022
Closed
14 tasks
@OPNA2608
Copy link
Contributor

This seems to have broken pkgsStatic for ppc64, it did seem to work on 4442087 just before this. musl complains about being built for ELFv1 now, relevant part from pkgsCross.ppc64.pkgsStatic.hello:

checking preprocessor condition _CALL_ELF == 2... false
./configure: error: unsupported powerpc64 ABI
error: builder for '/nix/store/zgas30jqgi23s0gkk1vr9xhmbpmxzsry-musl-static-powerpc64-unknown-linux-musl-1.2.3.drv' failed with exit code 1;

ghost
ghost reviewed Sep 16, 2022
View reviewed changes
pkgs/top-level/stage.nix
@@ -257,8 +259,6 @@ let
crossSystem = {
isStatic = true;
parsed = makeMuslParsedPlatform stdenv.hostPlatform.parsed;
} // lib.optionalAttrs (stdenv.hostPlatform.system == "powerpc64-linux") {
gcc.abi = "elfv2";
Copy link

Choose a reason for hiding this comment

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

@OPNA2608, if you need a quick fix, the root cause is the deletion of these two lines in 82ff1f5.

This was referenced Sep 16, 2022
Merged
Merged
Mindavi pushed a commit that referenced this pull request Sep 27, 2022
@Mindavi
stage.nix: revert deletion of gcc.abi="elfv2" from 82ff1f5
33bc076 
Commit 82ff1f5 in #182807 removed two lines
from stage.nix which were responsible for making sure the `gcc` for `pkgsStatic`
on powerpc64 was built with the `--with-abi=elfv2` flag.

Unfortunately this causes build failures for `pkgsCross.ppc64.pkgsStatic`, as
reported here:

  #182807 (comment)

This commit reverts the deletion.

Unfortunately ugly kludges like this are necessary because nixpkgs'
`lib/systems/` doesn't understand the difference between a libc and an abi.  So
we have no clean way to tell nixpkgs "musl on big-endian powerpc64 always uses
the ELFv2 ABI" -- it thinks that musl is an ABI.  Until that gets fixed there is
no better way to add the flag.
@ghost
Copy link

ghost commented Feb 21, 2023

Possibly of interest, Linux 6.2 supposedly lets you build big endian kernels (not just userspace) with -mabi=elfv2: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5017b45946722bdd20ac255c9ae7273b78d1f12e

ghost
ghost reviewed Jun 20, 2023
View reviewed changes
lib/systems/parse.nix
Comment on lines +475 to +476
# Default ppc64 BE to ELFv2
else if isPower64 parsed && isBigEndian parsed then abis.gnuabielfv2
Copy link

Choose a reason for hiding this comment

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

Were these two lines really essential?

They cause a problem: now if we parse powerpc64-apple-linux-gnu and then unparse it, we get back powerpc64-apple-linux-gnuabielfv2. That disagrees with gnu-config:

$ /nix/store/rvxxihfvp9z7h4lw6w60lv7mvsn97dwn-gnu-config-2023-01-21/config.sub powerpc64-apple-linux-gnu
powerpc64-apple-linux-gnu

I still agree with nixpkgs PowerPC64-BE using elfv2, I just don't think the triple parsing routine is the right place to impose that default...

I've included a commit to revert these two lines as part of #235230; if that's a problem, or if there's a different way we should be handling this, please let me know.

Copy link

Choose a reason for hiding this comment

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

428c545

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nbp nbp Awaiting requested review from nbp

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314 Ericson2314 is a code owner

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

@alyssais alyssais Awaiting requested review from alyssais alyssais is a code owner

Assignees
No one assigned
Labels
6.topic: cross-compilation Building packages on a different sort platform than than they will be run on 6.topic: exotic Exotic hardware or software platform 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@minijackson @r-burns @Mindavi @OPNA2608 @bobby285271