cc-wrapper: use -fwrapv instead of -fno-strict-overflow in clang

[tjni]

Description of changes

Please consider this a proof of concept that still needs discussion and testing. I'd appreciate opinions on whether this is the right thing to do. If it is, I'd also like thoughts on how to implement it in a simple, elegant way.

Background and Motivation

Reported in #39687, builds on macOS can encounter warnings like :

clang-11: error: argument unused during compilation: '-fno-strict-overflow' [-Werror,-Wunused-command-line-argument]

If we want to suppress this warning (such as in Kitty's build), we can add:

hardeningDisable = lib.optionals stdenv.cc.isClang ["strictoverflow"];

I find these warnings distracting when I debug builds, so I'd like to fix them once and for all (if possible).

Why does Clang emit this warning?

I think it is related to code at https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Clang.cpp#L6449:

// -fno-strict-overflow implies -fwrapv if it isn't disabled, but
// -fstrict-overflow won't turn off an explicitly enabled -fwrapv.
if (Arg *A = Args.getLastArg(options::OPT_fwrapv, options::OPT_fno_wrapv)) {
  if (A->getOption().matches(options::OPT_fwrapv))
    CmdArgs.push_back("-fwrapv");
} else if (Arg *A = Args.getLastArg(options::OPT_fstrict_overflow,
                                    options::OPT_fno_strict_overflow)) {
  if (A->getOption().matches(options::OPT_fno_strict_overflow))
    CmdArgs.push_back("-fwrapv");
}

That is, -fno-strict-overflow does nothing more than imply -fwrapv. When upstream adds -fwrapv (or even -fno-wrapv) itself, then -fno-strict-overflow does nothing, and I suspect it gets reported as unused.

What does this PR do?

It simply checks if cc-wrapper is wrapping Clang and uses -fwrapv instead of -fno-strict-overflow if so. However, I don't think the way I've done it is so elegant.

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.

[tjni]

This doesn't even work as-is because @isClang@ is not always being substituted properly. I'll leave it as-is pending initial feedback.

[tjni]

I'm marking this as ready for review now because I think it could be committed. On aarch64-darwin, I built kitty without disabling strictoverflow hardening, and the warning is no longer emitted. I'm still open to feedback on the implementation.

[ghost]

LGTM but I don't use clang much so I'm not really qualified to approve this... it affects quite a lot of builds.

[nixos-discourse]

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

https://discourse.nixos.org/t/prs-ready-for-review/3032/2466