Skip to content

Conversation

@RossComputerGuy
Copy link
Member

@RossComputerGuy RossComputerGuy commented Jun 5, 2025

Things done

GCC built like LLVM where we've split things into their individual components.

  • Built on platform(s)
    • x86_64-linux
    • 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 all binary files (usually in ./result/bin/)
  • Nixpkgs 25.11 Release Notes (or backporting 24.11 and 25.05 Nixpkgs Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
  • NixOS 25.11 Release Notes (or backporting 24.11 and 25.05 NixOS Release notes)
    • (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.

@github-actions github-actions bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Jun 5, 2025
@philiptaron
Copy link
Contributor

Instant subscribe. Wishing you incredibly well on this journey.

@RossComputerGuy RossComputerGuy force-pushed the feat/gcc-ng branch 2 times, most recently from d7286b9 to dc2bb16 Compare June 27, 2025 03:27
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. and removed 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. labels Jun 27, 2025
@RossComputerGuy RossComputerGuy marked this pull request as ready for review June 27, 2025 03:57
@RossComputerGuy
Copy link
Member Author

Cross compilation and native compilation of GNU hello works.

@nixpkgs-ci nixpkgs-ci bot added the 9.needs: reviewer This PR currently has no reviewers requested and needs attention. label Jul 4, 2025
Copy link
Member

Choose a reason for hiding this comment

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

Now that we are close to merging, can we do my original three commits (and anything else you need on top) rather than one squashed one?

(Note that the previous GCC NG PRs also had one squashed. The originals come from https://github.com/Ericson2314/gcc/tree/prog-target-14 as I mentioned before.)

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure what exactly you're meaning here. Like 3 separate patches from that branch?

@nixpkgs-ci nixpkgs-ci bot removed the 9.needs: reviewer This PR currently has no reviewers requested and needs attention. label Jul 7, 2025
Copy link
Member

Choose a reason for hiding this comment

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

We might want to do that before merging?

Copy link
Member

Choose a reason for hiding this comment

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

Actually find waiting on some of the other patches, as we might get feedback on how the exe lookup ought to work in general one one patch that would affect the other patches

Comment on lines +131 to +150
Copy link
Member

Choose a reason for hiding this comment

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

Did we have this before? What exactly is going on? Seems like this should be not buildGccPackages but the same package set as GCC itself? At least if GCC is going to link these?

Is this the top-level configure you are using? can we use a GCC subdir instead to avoid some of this?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a new thing to help minimize build times, I was able to save about 3 minutes by using a Nix built set of libraries. My hope is that later on, we can explicitly get GCC to not build it without forcing it like I did.

Copy link
Member

Choose a reason for hiding this comment

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

OK that sounds good. I see comparing to the earlier versions of this that this is not a different approach but just added stuff to the same approach -- I was worried you might be doing this instead of just CD'ing to a subproject, but I see that is not the case.

Copy link
Member

@Ericson2314 Ericson2314 left a comment

Choose a reason for hiding this comment

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

Just a few things, almost there!

Copy link
Member

@Ericson2314 Ericson2314 left a comment

Choose a reason for hiding this comment

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

OK yes just squash out my commit (I didn't want to force push to your branch) and this is ready to go! So excited!!!

@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jul 11, 2025
Copy link
Member

@Ericson2314 Ericson2314 left a comment

Choose a reason for hiding this comment

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

Let's go!

@Ericson2314 Ericson2314 merged commit 2deaafc into NixOS:master Jul 11, 2025
23 of 27 checks passed
@RossComputerGuy RossComputerGuy deleted the feat/gcc-ng branch July 11, 2025 14:52
@philiptaron
Copy link
Contributor

Congratulations!!!

@Ericson2314
Copy link
Member

https://gcc.gnu.org/pipermail/gcc-patches/2025-July/689354.html for the record, email about upstreaming one of the patches that I just sent.

}:
let
inherit (stdenv) targetPlatform hostPlatform;
targetPrefix = lib.optionalString (targetPlatform != hostPlatform) "${targetPlatform.config}-";
Copy link
Member

Choose a reason for hiding this comment

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

@RossComputerGuy A bit late to the party, but in #389469 I fixed a somewhat obscure cross-compilation bug by using lib.systems.equals consistently everywhere for gcc, but in this ng version some direct equality checks are present again. I think it would be good to keep things consistent from the start. I can make PR for this, but maybe it's easier if you do it while you're still fixing things?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think this is something we can change without messing with the CC wrapper and stdenv. When you use pkgsLLVM or any of the variants, you create a cross environment. This usage is simple enough it won't create issues.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/blog-post-compiler-bootstrapping-in-nixpkgs/66791/1

@trofi
Copy link
Contributor

trofi commented Jul 15, 2025

Looks like libelf was brought in back as a dependency. It was removed in 2022 in #187234 . If this branch is based on even older gcc expression code you might need to revisit more changes done to gcc in nixpkgs if you plain to maintain some parity.

@Ericson2314
Copy link
Member

Good catch, @trofi. Yes, I imagine we'll need many rounds of syncing things up.

@trofi
Copy link
Contributor

trofi commented Jul 16, 2025

Proposed libelf removal as:

+ c_file_name = p;
+
+ if (c_file_name) {
+ is_cross_compiler = strncmp(basename(c_file_name), target_machine, strlen(target_machine)) == 0;
Copy link
Contributor

@LordGrimmauld LordGrimmauld Nov 30, 2025

Choose a reason for hiding this comment

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

Uh oh - there is issues here!

for one, basename requires #include <libgen.h>. On glibc, you are lucky and this is implicitly already included. On musl, well, its completely broken.

According to the manpage:

basename(3)                                                  Library Functions Manual                                                  basename(3)

NAME
       basename, dirname - parse pathname components

LIBRARY
       Standard C library (libc, -lc)

SYNOPSIS
       #include <libgen.h>

       char *dirname(char *path);
       char *basename(char *path);

Another issue:
https://github.com/gcc-mirror/gcc/blob/1b306039ac49f8ad91ca71d3de3150a3c9fa792a/gcc/collect2.cc#L221

const char *c_file_name;		/* pathname of gcc */

c_file_name is const, meaning you can't just call basename and expect things to work. Ordinarily i would suggest just adding a strdup, but turns out strdup is poisoned here.

As it stands, pkgsMusl.gccNGPackages_15.libssp (what i actually wanted to build) is broken indirectly, i suspect pkgsMusl.gccNGPackages_15.gccNoLibgcc is the attrpath to the smallest reproducer.

Copy link
Contributor

Choose a reason for hiding this comment

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

(I don't know anything about gccNGPackages). gcc uses lbasename() from local libiberty.a in a few places. Might work here as is.

I wonder why you need libssp. Just to make sure it builds? Otherwise musl should use it's facility to implement ssp primitives: https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/configure.ac;h=b6d9608d5994f559fe7b1ccd1deb86a530195615;hb=HEAD#l6949

        *-*-musl*)
          # All versions of musl provide stack protector
          gcc_cv_libc_provides_ssp=yes;;

But there are probably some target-specific caveats like 32-bit x86, 32-bit ppc and mips.

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder why you need libssp. Just to make sure it builds? Otherwise musl should use it's facility to implement ssp primitives:

I was building netbsd.fts on llvm-native musl against #463395, and that failed to include ssp/ssp.h. There might be more things going on, but i figured i'd just try and see what happens if i give it ssp.

But even then, gcc 15 ng should work on musl, even if my use case here turns out to be bogus and some other issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

diff --git a/pkgs/development/compilers/gcc/ng/15/gcc/fix-collect2-paths.diff b/pkgs/development/compilers/gcc/ng/15/gcc/fix-collect2-paths.diff
index 1fac49766044..eed303ae4a0b 100644
--- a/pkgs/development/compilers/gcc/ng/15/gcc/fix-collect2-paths.diff
+++ b/pkgs/development/compilers/gcc/ng/15/gcc/fix-collect2-paths.diff
@@ -155,7 +155,7 @@ index 268ac378b9c..8a5c606075a 100644
 +    c_file_name = p;
 +
 +  if (c_file_name) {
-+    is_cross_compiler = strncmp(basename(c_file_name), target_machine, strlen(target_machine)) == 0;
++    is_cross_compiler = strncmp(lbasename(c_file_name), target_machine, strlen(target_machine)) == 0;
 +  }
 +
 +  for (i = 0; i < USE_LD_MAX; i++)

this does indeed fix the build fwiw

Copy link
Member Author

Choose a reason for hiding this comment

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

I was building netbsd.fts on llvm-native musl against #463395, and that failed to include ssp/ssp.h. There might be more things going on, but i figured i'd just try and see what happens if i give it ssp.

I don't think we even use GCC NG currently by default so this would probably be a problem with regular GCC.

Copy link
Contributor

Choose a reason for hiding this comment

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

oh yeah no, regular musl (built with regular gcc) breaks and can't build gcc ng. That was just motivation for why i was even trying in the first place, but just musl gccng reproduces.

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

Labels

10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants