gcc: only disable aligned_alloc for darwin build/host/target platforms

[eliasnaur]

Before this change a Darwin gcc would output binaries that avoid
aligned_alloc, regardless of target platform. That's fine for Darwin
targets, but not for non-darwin targets such as pkgsCross.raspberryPi.

This change replaces the single configure flag with a flag for
each of build, host, target.

Idea by @trofi.

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:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• 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.05 Release Notes (or backporting 22.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.

[trofi]

I think it's a bit more tricky: aligned_alloc is gcc's builtin. As a result I expect it to be used both in gcc's binary and target libraries built by gcc. That means you likely want ac_cv_func_aligned_alloc=no for --host=darwin (and any --target=) to keep the properties of gcc binary. And for --target=darwin to keep the properties of target libraries like libgomp.

In theory it would be nice if gcc's build system provided hooks to affect only host or only target with ac_cv_func_aligned_alloc=no. But it's a bit tricky to get right. I think the canonical way to do it is to pass it via STAGE{2,3}_CONFIGURE_FLAGS to gcc.

Getting back to your issue you might get a reasonable outcome with lib.optionalString (hostPlatform.isDarwin || targetPlatform.isDarwin).

[eliasnaur]

Getting back to your issue you might get a reasonable outcome with lib.optionalString (hostPlatform.isDarwin || targetPlatform.isDarwin).

So

lib.optionalString (hostPlatform.isDarwin || targetPlatform.isDarwin) ''
      export ac_cv_func_aligned_alloc=no
''

?

Isn't your new condition a super-set of the old, because of the ||? My issue is that without this PR (or similar) cross-compiled binaries for Raspberry Pi don't reference aligned_alloc whereas cross-compiled binaries built on Linux do. It seems to me your suggestion strictly expands the number of configurations where aligned_alloc will be left out.

Did you perhaps mean lib.optionalString (hostPlatform.isDarwin && targetPlatform.isDarwin), with && instead of ||?

[trofi]

My suggestion was based on the following goals:

• when building on darwin build (either for linux or for darwin target) there should be no references to `aligned_allos)
• when building for target darwin (either on linux or darwin build) there should be no references to `aligned_allos)

Hence the ||.

[eliasnaur]

But I want to have darwin building for linux result in the same binaries as when linux builds for linux. That is, I want the aligned_alloc reference in the binaries because that's what linux building for linux have.

Do you know of a way to achieve that, without breaking darwin?

[trofi]

Reading through gcc/doc/install.texi I found this bit (TIL!):

@subsubheading Overriding @command{configure} test results

Sometimes, it might be necessary to override the result of some
@command{configure} test, for example in order to ease porting to a new
system or work around a bug in a test.  The toplevel @command{configure}
script provides three variables for this:

@table @code

@cindex @code{build_configargs}
@item build_configargs
The contents of this variable is passed to all build @command{configure}
scripts.

@cindex @code{host_configargs}
@item host_configargs
The contents of this variable is passed to all host @command{configure}
scripts.

@cindex @code{target_configargs}
@item target_configargs
The contents of this variable is passed to all target @command{configure}
scripts.

@end table

Thus most robust way to get that behavior might be the (untested):

lib.optionalString (buildPlatform.isDarwin) ''
      export build_configargs=ac_cv_func_aligned_alloc=no
'' ++ lib.optionalString (hostPlatform.isDarwin) ''
      export host_configargs=ac_cv_func_aligned_alloc=no
'' ++ lib.optionalString (targetPlatform.isDarwin) ''
      export target_configargs=ac_cv_func_aligned_alloc=no
''

[eliasnaur]

Thank you very much! Separate configure flags seem to be the correct fix, and achieves my goal of build platform independent binaries. PTAL.

[eliasnaur]

Excuse my ignorance, but what is the next step in getting this merged? More approvals?

[trofi]

Getting review from @NixOS/darwin-maintainers would be nice. Otherwise I support merging it tostaging.

[eliasnaur]

Time for staging, or wait a bit more?

[wegank]

I guess this can only take place after 2023-05-15?

[veprbl]

Looks like a sensible change.

[eliasnaur]

I guess this can only take place after 2023-05-15?

Does the ban apply to darwin-only changes as well?

[vcunat]

Well, perhaps we could say that gcc isn't a critical package on darwin (as clang is the default stdenv.cc), but any breaking changes have cutoff today (except desktop environment stuff).

[veprbl]

@eliasnaur Since this only affects cross, do you think you could massage this into a zero rebuild change?

[winterqt]

If it only affects cross, it should be enough to just do the inverse of this:

nixpkgs/pkgs/development/compilers/gcc/12/default.nix

Lines 72 to 74 in 120dd6a

	# We only apply this patch when building a native toolchain for aarch64-darwin, as it breaks building
	# a foreign one: https://github.com/iains/gcc-12-branch/issues/18
	++ optional (stdenv.isDarwin && stdenv.isAarch64 && buildPlatform == hostPlatform && hostPlatform == targetPlatform) (fetchpatch {

[eliasnaur]

If I make the PR zero-rebuild as you describe, won't it break cross-building a darwin gcc from linux? If so, I'd rather wait 2 weeks for this PR to get into staging. I have other mass-rebuild reproducibility PRs that need to wait for staging anyway.

[Et7f3]

cross-building a darwin gcc from linux

macOS/Darwin is a special case, as not the whole OS is open-source. It’s only possible to cross compile between aarch64-darwin and x86_64-darwin. aarch64-darwin support was recently added, so cross compilation is barely tested.

https://nix.dev/tutorials/cross-compilation#determining-the-host-platform-config

So I would say it is fine

[eliasnaur]

cross-building a darwin gcc from linux

macOS/Darwin is a special case, as not the whole OS is open-source. It’s only possible to cross compile between aarch64-darwin and x86_64-darwin. aarch64-darwin support was recently added, so cross compilation is barely tested.

https://nix.dev/tutorials/cross-compilation#determining-the-host-platform-config

So I would say it is fine

It may not work today, but won't we like to be able to cross-compile to darwin in the future?

If I seem overly concerned about adding these special cases, it's because I've spent quite some time finding and fixing reproducibility ruining special cases already[0]. I'd rather not leave more for future contributors to untangle.

[0] Apart from this PR, there are #226048, #223861, #225328, of which two trigger mass rebuilding.

EDIT: almost forgot #229377.

[uri-canva]

I think it's worth keeping it as is and wait to merge.

[eliasnaur]

The staging unrestricting date has come and passed, so I assume this can be merged?

[veprbl]

The staging unrestricting date has come and passed, so I assume this can be merged?

According to #232237, you are correct.
Let's do it.