cc-wrapper: omit adding -D_FORTIFY_SOURCE if it would cause redefinition

[risicle]

Description of changes

See discussion in #224822 regarding annoying redefinition warnings it caused. This is the simplest of my proposed options.

This shouldn't actually change behaviour as the way hardeningCFlags are currently prepended to supplied params, the caller-supplied value will win anyway, but in omitting our definition, we'll avoid annoying (and potentially fatal) warnings.

Tested against #217390 successfully, nixos x86_64.

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.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.

[ghost]

I'm not too excited about this. Trying to parse the flags that upstream's build process passes to the wrapper, and then change our behavior based on that... I mean down that path lies madness.

It's a long established principle that conflicting C compiler flags are okay, and later flags supersede earlier flags. Nixpkgs has always exploited and depended upon this, extensively.

It sounds like all this extra complexity is simply because:

annoying redefinition warnings it caused

Frankly I think the complexity this adds to cc-wrapper is infinitely more annoying.

Better solutions would be:

1. If a package unilaterally sets the hardening flags, get it to not do that (possibly by upstreaming a patch creating an option that says "don't set these flags for me").
2. Turn off the warning.

I think 2 is a good idea in any case. We use and rely upon the ability to pass flags that override other flags. It's not a defect; we don't need to be warned when this happens.

[risicle]

FWIW we already inspect params in add-hardening.sh for the -pie option, and in cc-wrapper.sh we quite extensively parse the params and change our behaviour based on it.

As for it "being annoying" it's a bit of an understatement - there are some packages where it doubles the size of the build log.

But failing that, a more comprehensive solution may be to move -D_FORTIFY_SOURCE additions to a suffixed hardeningCFlags-alike, though at that point we're overriding the build system's choice. Perhaps that's fine, but there's bound to be at least a few packages that causes problems with.

Anyway, let's keep more general discussion of other possible solutions on #224822

[emilazy]

I think I would generally support overriding upstream for hardening, especially since I expect most of these in practice are opting in to hardening rather than out and we can hardeningDisable for the rest.

(In other words I think "adding -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 to the end" is my preferred solution.)

[risicle]

I think one of my concerns is how few things cc-wrapper currently adds to the end of the command line. The general pattern is to let the command-line "win".

(urgh let's bring this back to #224822)

[risicle]

#246244 was merged instead