treewide: migrate to -fno-common

[Gaelan]

Motivation for this change

GCC 10 defaults to -fno-common, a change that broke some packages. To ease the transition when migrating to GCC 10, we changed the default back in our build of GCC. This commit undoes that, bringing us closer to upstream and reducing our technical debt. (Clang, which made the same change, also cites "performance and code-size benefits"; it's unclear if this is the case for GCC.)

In an effort to reduce the potential breakage from this PR, I've made no effort to patch or upgrade packages broken by this. Instead, for any package that fails to build, I fix it by setting NIX_CFLAGS_COMPILE = "-fcommon", i.e. returning to the status quo. This brings the vast majority of packages onto the new default, and has no risk of introducing new bugs due to incorrect patches. As the rest of the world migrates towards GCC 10, most of these will probably get fixed by upstream, and we can remove the flag; for the rest, we can decide whether to write/find a patch or keep the flag indefinitely.

This PR is not ready to merge at this point. Before doing so, I'd like to try building at least these things:

• The configuration.nix of the laptop I'm testing this on
• Some (or all) of the configurations in release.nix
• Packages listed as broken (or previously broken) in the Gentoo and Fedora issues for migrating to GCC 10.

It might also be a good idea to set up a hydra job.

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.

[Gaelan]

Huh, it's weird that this causes an stdenv rebuild on darwin. It should only affect GCC.

[SuperSandro2000]

https://gcc.gnu.org/onlinedocs/gcc/Code-Gen-Options.html

-fcommon
In C code, this option controls the placement of global variables defined without an initializer, known as tentative definitions in the C standard. Tentative definitions are distinct from declarations of a variable with the extern keyword, which do not allocate storage.

The default is -fno-common, which specifies that the compiler places uninitialized global variables in the BSS section of the object file. This inhibits the merging of tentative definitions by the linker so you get a multiple-definition error if the same variable is accidentally defined in more than one compilation unit.

The -fcommon places uninitialized global variables in a common block. This allows the linker to resolve all tentative definitions of the same variable in different compilation units to the same object, or to a non-tentative definition. This behavior is inconsistent with C++, and on many targets implies a speed and code size penalty on global variable references. It is mainly useful to enable legacy code to link without errors.

[r-burns]

I wonder if this would warrant a hydra job. Since this primarily affects linux, it could be a good task for spare x86_64-linux builders.

[r-burns]

The darwin stdenv contains cpio, but we can avoid the rebuild:

Suggested change

	NIX_CFLAGS_COMPILE = "-fcommon";
	NIX_CFLAGS_COMPILE = if stdenv.cc.isGNU then "-fcommon" else null;

[r-burns]

Also it looks like gentoo has a fix for this: https://bugs.gentoo.org/705900

[SuperSandro2000]

It goes to staging anyway so we don't really need to worry about stdenv rebuilds.

[Gaelan]

Aha, that solves the Darwin mystery.

My inclination is to use CFLAGS instead of patches initially, because it keeps the PR simple and seems less likely to break things; we can always switch to patches on a per-package basis later. But I can probably be convinced otherwise.

[toonn]

I ran into this for #126411 too. I opted for patches in the cases I came across because I figured they'd break sooner and trigger people to reevaluate them. For cpio in particular I've also sent an email to upstream (no response so far). So I'm in favor of having this apply for Darwin and clang too : )

[Gaelan]

iso_minimal now builds. I can upload to cachix if it helps anyone.

[Gaelan]

Bah, there's a merge conflict, but if I try to merge in aa8868c (the output of git merge-base origin/master origin/staging), the PR pulls in a bunch of unrelated commits for some reason.

[ruuda]

There’s a conflict in squashfsTools because lz4 support recently became unconditional, and it touched related lines.

[thefloweringash]

Clang 11 is making the same change (changelog, D75056) so these changes shouldn't be restricted to GCC. I ran into this when working on aarch64-darwin which has a hydra job. It could be useful to pick through the set of new failures in the change from clang 10 to clang 11: https://hydra.nixos.org/eval/1646384?compare=1646102#tabs-now-fail.

[Gaelan]

Sorry this PR is languishing (and for all the force pushes) - I can't manage to fix the merge conflict without GitHub pulling in thousands of unrelated commits. I thought git merge xyz, where xyz is a commit appearing in both master and staging, would do the trick, but it doesn't seem to work.

[trofi]

All failing packages noticed on hydra are now fixed in master or staging. I'd like to do one extra run once staging trickles down into master to check if we have any extra darwin fallout: #177171

After that we'll be ready for merge.

[trofi]

Failures extracted from this iteration:

• gmtp: gmtp: apply -fcommon workaround for real #178536
• uim: uim: pull upstream fix for -fno-common toolchains #178538
• vscode-extensions.ms-python.python (lttng-ust-2.10.5): vscode-extensions.ms-python.python: pull upstream fix for -fno-common… #178370
• ocaml-ng.ocamlPackages_4_05.lablgtk: ocaml-ng.ocamlPackages_4_05.lablgtk: add -fcommon workaround #178373
• a52dec (darwin): ended up being not -fcommon-specific
  • a52dec aarch64-darwin fix: a52dec: fix build on aarch64-darwin #180194
• bs-platform: bs-platform: workaround -fno-common build failure on aarch64 #179253
• sbcl (darwin): sbcl: pull darwin fix pending upstream inclusion for -fno-common tool… #179934
• texlive.bin.xdvi (darwin): texlive.bin.xdvi: pull upstream darwin fix for -fno-common toolchains #179943

[vcunat]

Jobset: as x86_64-darwin is now catching up about a week of outage on Hydra, I canceled it here and put in aarch64-darwin instead (was idling right now).

[trofi]

I hereby declare all (~240) the knows failures to be fixed. Should be ready for staging.

[nixos-discourse]

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

https://discourse.nixos.org/t/help-disabling-fno-common-hack/19031/3

[Artturin]

@ofborg build nixosTests.gnome

[vcunat]

Thanks. This must've taken really lots of work. I'm convinced it's in good enough shape for staging. Minor remaining issues can always get fixed during staging-next.

[trofi]

Recent package additions that fail now:

• decoder: decoder: pull patch pending upstream inclusion for -fno-common toolch… #180820

[Artturin]

Thank you for your efforts trofi