cc-wrapper: Pass shellcheck and other cleanups

[Ericson2314]

Motivation for this change

In many cases, this involved taking @orivej's and @edolstra's recent ld-wrapper improvements, and applying then elsewhere.

The only semantics changes I know of are

• Fix bad word-splitting---already done with ld-wrapper with no issues

• Remove unused and probably wrong LD=---didn't take into account cross and in cc-wrapper pointed to unwrapped ld while in ld-wrapper pointed to wrapped ld.

Things done

Please check what applies. Note that these are not hard requirements but merely serve as information for reviewers.

• Tested using sandboxing
  (nix.useSandbox on NixOS,
  or option build-use-sandbox in nix.conf
  on non-NixOS)
• Built stdenv on platform(s)
  • NixOS
  • macOS
  • Linux
• 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 nox --run "nox-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Fits CONTRIBUTING.md.

---

CC @orivej

[mention-bot]

@Ericson2314, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @vcunat and @abbradar to be potential reviewers.

[Ericson2314]

Here's some things I'd like feedback on (along with me discovering my own mistake :)).

[Ericson2314]

I think not including extraBefore here was an oversight of whoever added it originally. Note that the debugging code below originally also skipped it.

[Ericson2314]

This was the same one as above---eager copy paste---and unneeded.

[Ericson2314]

Somebody should weigh in on this.

[grahamc]

thsi is probably not intentional

[copumpkin]

I weigh in on "thsi" most likely being a typo. Does that count?

[Ericson2314]

Wait, we already check for file existence, so this is definitely isn't needed.

[Ericson2314]

Semantics change, but all the flags are sans-whitespace so it seems fine.

[Ericson2314]

I think this is clearer than the regex stuff that was there before---and shellcheck said the regex stuff was done wrong anyways such that only string literals would match.

[Ericson2314]

N.B. bugs like this weren't being caught because extra word splitting on arrays.

[Ericson2314]

This was unused.

[Ericson2314]

Semantics change, but the old version was a mistake. I fixed a hardening flag bug uncovered by this; hope there aren't others.

@orivej did something the same fix for ld-wrapper, and I guess it was fine there?

[Ericson2314]

So the "skip" thing does say "bad path", but not knowing what flag it came from is strictly less useful. I'm conflicted.

[Ericson2314]

This is a dead variable, but used in commented code, so I guess I'll leave it for now.

[Ericson2314]

In #27672 I made cc-wrapper robust with undefined variables with set -u. That makes me extra confident in this commit, as it is included in that.

[grahamc]

I'm worried about the n+=1:

[grahamc@Morbo:~]$ declare -i n=0

[grahamc@Morbo:~]$ i+=1

[grahamc@Morbo:~]$ echo $i
1

[grahamc@Morbo:~]$ i+=1

[grahamc@Morbo:~]$ echo $i
11

otherwise this looks great

[grahamc]

thsi is probably not intentional

[Ericson2314]

OK I think I dealt with all comments.

[Ericson2314]

This would be the unwrapped ld, which seems wrong.

[Ericson2314]

This however would be the wrapped ld. Since cc will fork ld-wrapper, the old LD=..., assuming LD was already an export so it affected forked processes' environments (!), would be clobbered anyways.

[Ericson2314]

Added back better comment without typo.