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

gcc12: mark broken for MIPS64 with unspecified ABI #237780

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alyssais
Copy link
Member

Description of changes

Seems like there's a bug in this version of GCC that prevents it from building unless the ABI is explicitly specified in the triple or -mabi. No other GCC versions in Nixpkgs are affected.

We might want to consider bumping MIPS to GCC 13 early to get around this.

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:
  • 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 all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.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
  • Fits CONTRIBUTING.md.

Seems like there's a bug in this version of GCC that prevents it from
building unless the ABI is explicitly specified in the triple or -mabi.
No other GCC versions in Nixpkgs are affected.
@alyssais alyssais added the 6.topic: exotic Exotic hardware or software platform label Jun 14, 2023
@alyssais alyssais requested review from trofi and a user June 14, 2023 15:44
@trofi
Copy link
Contributor

trofi commented Jun 14, 2023

Do you know by chance if it was explicitly fixed in newer gcc versions?

My worry is that nixpkgs might not build correctly gcc versions different from the default attribute (and thus update to gcc-13 might expose the same failure).

While at it: which attribute do you build (if we have one in nixpkgs)? Looking at lib/examples all mips64* ones have gnuabi* suffix.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild labels Jun 14, 2023
@alyssais
Copy link
Member Author

Do you know by chance if it was explicitly fixed in newer gcc versions?

I don't. I suppose I could possibly bisect and find out, but I don't know how bisectable GCC is.

My worry is that nixpkgs might not build correctly gcc versions different from the default attribute (and thus update to gcc-13 might expose the same failure).

I don't undersand what you mean here. I tested all other GCC versions for the same MIPS doubles, and I regularly test builds for all other Linux architectures in lib.systems.doubles.

While at it: which attribute do you build (if we have one in nixpkgs)? Looking at lib/examples all mips64* ones have gnuabi* suffix.

nix-build -A buildPackages.gcc12 --argstr crossSystem mips64-linux

@trofi
Copy link
Contributor

trofi commented Jun 14, 2023

Do you know by chance if it was explicitly fixed in newer gcc versions?

I don't. I suppose I could possibly bisect and find out, but I don't know how bisectable GCC is.

Aha, no problem. I can look at it as well.

My worry is that nixpkgs might not build correctly gcc versions different from the default attribute (and thus update to gcc-13 might expose the same failure).

I don't undersand what you mean here. I tested all other GCC versions for the same MIPS doubles, and I regularly test builds for all other Linux architectures in lib.systems.doubles.

Aha. My minor worry was that buildPackages.gcc13 is not exactly equivalent to buildPackages.gcc (when default is gcc=13). My apologies for the confusion.

While at it: which attribute do you build (if we have one in nixpkgs)? Looking at lib/examples all mips64* ones have gnuabi* suffix.

nix-build -A buildPackages.gcc12 --argstr crossSystem mips64-linux

Thank you! Did not know you can do it that way :) I tried a few pkgsCross.*.stdenv.cc variants and failed. With your command I see the following libsanitizer failure clashing on stat64 definition:

In file included from /nix/store/hb8458w3wgpqp9kcbshv2zvhvjjkij0b-glibc-mips64-unknown-linux-gnu-2.37-8-dev/include/bits/stat.h:25,
                 from /nix/store/hb8458w3wgpqp9kcbshv2zvhvjjkij0b-glibc-mips64-unknown-linux-gnu-2.37-8-dev/include/fcntl.h:78,
                 from ../../../../gcc-12.3.0/libsanitizer/sanitizer_common/sanitizer_linux.cpp:55:
/nix/store/hb8458w3wgpqp9kcbshv2zvhvjjkij0b-glibc-mips64-unknown-linux-gnu-2.37-8-dev/include/bits/struct_stat.h:190:8: error: redefinition of 'struct stat64'
  190 | struct stat64
      |        ^~~~~~

Is it the same thing you observe? That one should be easy to track down in gcc/master.

@trofi
Copy link
Contributor

trofi commented Jun 14, 2023

https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=e08835fd3a4a6559b79c26db9b18df0e838d943e looks like a good candidate, added in gcc-13+ (did not test yet).

@alyssais
Copy link
Member Author

Thank you! Did not know you can do it that way :)

It should really be the default way people use, so we don't proliferate rarely-used "examples", and to make it more obvious that the platforms can be customised. In future, I'd like to clear out pkgsCross and point people towards this way instead for most purposes.

Is it the same thing you observe? That one should be easy to track down in gcc/master.

Yes

@trofi
Copy link
Contributor

trofi commented Jun 14, 2023

https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=e08835fd3a4a6559b79c26db9b18df0e838d943e looks like a good candidate, added in gcc-13+ (did not test yet).

Was not it. libsanitizer has a bunch of changes between gcc-12 and gcc-13 including libsanitizer upstream syncs as a single commit. I did not find a nice small change to pull in.

WDYT of disabling just sanitizer instead: #237815

@trofi trofi removed their request for review June 15, 2023 06:17
@ghost
Copy link

ghost commented Jun 20, 2023

Hey thanks for tracking this down.

I was able to reproduce the problem (both mips64 and mips64el). I think @trofi had the right solution with 6cec03027a07162b1fe1d5bfd61c98e98eb5c316. I also was able to verify that his 6cec03027a07162b1fe1d5bfd61c98e98eb5c316 fixes it.

Could we do that instead? I've opened #238716 to do that.

@alyssais alyssais added the 2.status: merge conflict This PR has merge conflicts with the target branch label Oct 21, 2023
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 19, 2024
@wegank wegank marked this pull request as draft March 20, 2024 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: exotic Exotic hardware or software platform 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants