build: Silence [-Wunused-command-line-argument] warnings by hebasto · Pull Request #21788 · bitcoin/bitcoin

hebasto · 2021-04-27T18:42:21Z

Apple clang version 12.0.5 (clang-1205.0.22.9) that is a part of Xcode 12.5, and is based on LLVM clang 11.1.0, fires spammy warnings:

clang: warning: argument unused during compilation: '-fstack-clash-protection' [-Wunused-command-line-argument]

From the https://github.com/apple/llvm-project:

$ git log --oneline | grep 'stack-clash-protection'
00065d5cbd02 Revert "-fstack-clash-protection: Return an actual error when used on unsupported OS"
4d59c8fdb955 -fstack-clash-protection: Return an actual error when used on unsupported OS
df3bfaa39071 [Driver] Change -fnostack-clash-protection to  -fno-stack-clash-protection
68e07da3e5d5 [clang][PowerPC] Enable -fstack-clash-protection option for ppc64
515bfc66eace [SystemZ] Implement -fstack-clash-protection
e67cbac81211 Support -fstack-clash-protection for x86
454621160066 Revert "Support -fstack-clash-protection for x86"
0fd51a4554f5 Support -fstack-clash-protection for x86
658495e6ecd4 Revert "Support -fstack-clash-protection for x86"
e229017732bc Support -fstack-clash-protection for x86
b03c3d8c6209 Revert "Support -fstack-clash-protection for x86"
4a1a0690ad68 Support -fstack-clash-protection for x86
f6d98429fcdb Revert "Support -fstack-clash-protection for x86"
39f50da2a357 Support -fstack-clash-protection for x86

I suppose, that Apple clang-1205.0.22.9 ends with on of the "Revert..." commits.

This PR prevents using of the -fstack-clash-protection flag if it causes warnings.

System: macOS Big Sur 11.3 (20E232).

jarolrod

ACK b9fe3af

tested on macOS 11.3

Master

make -j"$(($(sysctl -n hw.logicalcpu)+1))" 2>&1 > /dev/null | grep -o "fstack-clash-protection" | wc -l

615

PR

make -j"$(($(sysctl -n hw.logicalcpu)+1))" 2>&1 > /dev/null | grep -o "fstack-clash-protection" | wc -l

0

Guix Hashes

find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum

96c151bcd1f1663264f5f630aff70b7c8cdf010ee00048858678a932db8e5ff2  guix-build-b9fe3afce6fb/output/aarch64-linux-gnu/bitcoin-b9fe3afce6fb-aarch64-linux-gnu-debug.tar.gz
4fa8458a775d8d89be675a4c6e02fefb1835b5ae4598012b85b8b43a722a136d  guix-build-b9fe3afce6fb/output/aarch64-linux-gnu/bitcoin-b9fe3afce6fb-aarch64-linux-gnu.tar.gz
aa1b9ce1d73e09e7afd54dde338eefa3b1ab19bcff730ad3e893444bf7660c1a  guix-build-b9fe3afce6fb/output/arm-linux-gnueabihf/bitcoin-b9fe3afce6fb-arm-linux-gnueabihf-debug.tar.gz
660e82039a41c72ce35b39ce0321e28fd863999dc979f303753b7ba71048fac4  guix-build-b9fe3afce6fb/output/arm-linux-gnueabihf/bitcoin-b9fe3afce6fb-arm-linux-gnueabihf.tar.gz
bbb21a4bc22273dc45019db09f6038f9a4006880e9af6318b60f03e02360a55b  guix-build-b9fe3afce6fb/output/dist-archive/bitcoin-b9fe3afce6fb.tar.gz
7424146d4f66cce624844c192b1619430e9722c6d8da32c27a819518213db323  guix-build-b9fe3afce6fb/output/powerpc64-linux-gnu/bitcoin-b9fe3afce6fb-powerpc64-linux-gnu-debug.tar.gz
d959de8ee2f5360473fb3b67e4b235bad9709c9b1167cae04cb7c5e43de60146  guix-build-b9fe3afce6fb/output/powerpc64-linux-gnu/bitcoin-b9fe3afce6fb-powerpc64-linux-gnu.tar.gz
211dc230e3bb50ce492389eab7015f5a305a1bc2bfaa2cecbb7bea294aa13273  guix-build-b9fe3afce6fb/output/powerpc64le-linux-gnu/bitcoin-b9fe3afce6fb-powerpc64le-linux-gnu-debug.tar.gz
3036df123d9aead52f34c8c595759a4ee32598a877ce4b1e3eeec109343c668d  guix-build-b9fe3afce6fb/output/powerpc64le-linux-gnu/bitcoin-b9fe3afce6fb-powerpc64le-linux-gnu.tar.gz
66f90ff528c59cb9a2f8d92d1e8f4a0800bd7f75567878f20f02d4a6192f6070  guix-build-b9fe3afce6fb/output/riscv64-linux-gnu/bitcoin-b9fe3afce6fb-riscv64-linux-gnu-debug.tar.gz
e361b6d7a1809b27d331cd1df02192f785de93b82b99b23421ca77c8a26d4682  guix-build-b9fe3afce6fb/output/riscv64-linux-gnu/bitcoin-b9fe3afce6fb-riscv64-linux-gnu.tar.gz
a2980ec97fc0a64403d8adb82c3cd5014e9b0d2e911e511df8dc1fe09b67cc86  guix-build-b9fe3afce6fb/output/x86_64-apple-darwin18/bitcoin-b9fe3afce6fb-osx-unsigned.dmg
3a56ad51c6824488684a2021b5d952aa1fc38a6d165add1f72d4b3643fc5947b  guix-build-b9fe3afce6fb/output/x86_64-apple-darwin18/bitcoin-b9fe3afce6fb-osx-unsigned.tar.gz
f1feb7fef798ef1c5a3304408df3a1590a62b843d996e7bc5659d45683d677b4  guix-build-b9fe3afce6fb/output/x86_64-apple-darwin18/bitcoin-b9fe3afce6fb-osx64.tar.gz
b0c2a26050869dc4b83fb05765856ab48b4a683026f51b1b3152d265d594bf0e  guix-build-b9fe3afce6fb/output/x86_64-linux-gnu/bitcoin-b9fe3afce6fb-x86_64-linux-gnu-debug.tar.gz
b0cd5c21673d8c97768de7dcd1de2e9bc7b6910ce6e034bccb8223d0430a5722  guix-build-b9fe3afce6fb/output/x86_64-linux-gnu/bitcoin-b9fe3afce6fb-x86_64-linux-gnu.tar.gz
33c5c7fd18897dd242594c2b441a03d167f67ebcdc7e2a392195c6182f6c1812  guix-build-b9fe3afce6fb/output/x86_64-w64-mingw32/bitcoin-b9fe3afce6fb-win-unsigned.tar.gz
011bd9c0f807fbd7bc3bdc267ecf5f5fe432b16d315c4421293b45a34faa38df  guix-build-b9fe3afce6fb/output/x86_64-w64-mingw32/bitcoin-b9fe3afce6fb-win64-debug.zip
153dea4cc0bc59e939b7aeaae042450efd784b2225ab81872c1473ac84c70f51  guix-build-b9fe3afce6fb/output/x86_64-w64-mingw32/bitcoin-b9fe3afce6fb-win64-setup-unsigned.exe
598968164f4ba9759b6bcb64609b01c776efb72abb29badd0298a71255fdc167  guix-build-b9fe3afce6fb/output/x86_64-w64-mingw32/bitcoin-b9fe3afce6fb-win64.zip

fanquake · 2021-04-28T04:12:15Z

This change is not macOS specific, so it shouldn't be labelled that way or made to seem that way in the commit message. You're changing how we test for support for a compile flag, which, in this case, is relevant for any OS other than Windows, across both Clang and GCC.

Regardless of the above. What's the underlying cause of the new warnings? What changed that they've only started happening now?

willcl-ark · 2021-04-28T11:36:54Z

ACK b9fe3af

$ MacOS 11.4 Beta (20F5046g)
$ xcode-select version 2384
$ g++ --version
Configured with: --prefix=/Library/Developer/CommandLineTools/usr --with-gxx-include-dir=/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/4.2.1
Apple clang version 12.0.5 (clang-1205.0.22.9)
Target: x86_64-apple-darwin20.5.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

I was in fact hitting compile errors before applying this patch on top of master. Now it both compiles and warnings are removed.

Of note, and I don't want to derail this PR, but I also get about 8000 lines of -Wsuggest-override warnings from various boost packages with the new command line tools 🙄:
compile.txt. I will investigate a little further and open a new issue for this though.

hebasto · 2021-04-28T14:16:38Z

@willcl-ark

Thanks for testing!

Of note, and I don't want to derail this PR, but I also get about 8000 lines of -Wsuggest-override warnings from various boost packages with the new command line tools roll_eyes:
compile.txt. I will investigate a little further and open a new issue for this though.

Probably, you'd want to pass --enable-suppress-external-warnings to your configure script`.

willcl-ark · 2021-04-28T14:22:29Z

Thanks @hebasto, that does indeed suppress those warnings.

hebasto · 2021-04-28T23:19:38Z

@fanquake

This change is not macOS specific, so it shouldn't be labelled that way or made to seem that way in the commit message. You're changing how we test for support for a compile flag, which, in this case, is relevant for any OS other than Windows, across both Clang and GCC.

Thanks. Understand.

Regardless of the above. What's the underlying cause of the new warnings? What changed that they've only started happening now?

I've updated the PR description.

Sjors

I can confirm b9fe3af makes the (maddening) warnings go away on macOS 11.3.1

Sjors · 2021-05-05T17:41:59Z

      ;;
    *)
-      AX_CHECK_COMPILE_FLAG([-fstack-clash-protection],[HARDENED_CXXFLAGS="$HARDENED_CXXFLAGS -fstack-clash-protection"])
+      AX_CHECK_COMPILE_FLAG([-fstack-clash-protection], [HARDENED_CXXFLAGS="$HARDENED_CXXFLAGS -fstack-clash-protection"], [], [$CXXFLAG_WERROR])


This could use a comment.

I'll add it if any other changes will be requested by reviewers.

Is a comment still required after the update to the commit message on e9f948c?

Most of the AX_CHECK_COMPILE_FLAG macros in the configure.ac use the same pattern.

Could you elaborate which comment will be useful?

practicalswift · 2021-05-07T15:31:42Z

~~cr ACK b9fe3af: patch looks correct~~

@hebasto What do you think about making this change conditional on macOS + Clang to make sure non-macOS platforms and non-Clang users are unaffected?

fjahr · 2021-05-09T09:36:34Z

Tested ACK b9fe3af (macOS 11.3.1, clang 12.0.0)

hebasto · 2021-05-09T18:36:40Z

@practicalswift

@hebasto What do you think about making this change conditional on macOS + Clang to make sure non-macOS platforms and non-Clang users are unaffected?

What if other host/build_os/compiler set will not have the -fstack-clash-protection flag available? Isn't the reason to use the AX_CHECK_COMPILE_FLAG macro to check the availability of a flag?

practicalswift · 2021-05-09T20:41:12Z

@hebasto What do you think about making this change conditional on macOS + Clang to make sure non-macOS platforms and non-Clang users are unaffected?

What if other host/build_os/compiler set will not have the -fstack-clash-protection flag available? Isn't the reason to use the AX_CHECK_COMPILE_FLAG macro to check the availability of a flag?

Sure, the AX_CHECK_COMPILE_FLAG must be there for all platforms :)

My thinking was that you could keep the AX_CHECK_COMPILE_FLAG check as-is for platforms unaffected by the problem being addressed, and only change it for platforms where we have reason to believe this change might be needed.

Perhaps doing so is deemed too hacky even for a configure script: IDK TBH :)

The context being this feedback from fanquake (#21788 (comment)): "This change is not macOS specific, so it shouldn't be labelled that way or made to seem that way in the commit message. You're changing how we test for support for a compile flag, which, in this case, is relevant for any OS other than Windows, across both Clang and GCC."

jarolrod · 2021-05-09T22:57:45Z

@fjahr

(macOS 11.3.1, clang 12.0.0)

Did these warnings come up on clang 12.0.0 for you? They shouldn't.

fjahr · 2021-05-10T21:32:35Z

@fjahr

(macOS 11.3.1, clang 12.0.0)

Did these warnings come up on clang 12.0.0 for you? They shouldn't.

It appears they do. Why shouldn't they?

$ clang -v
clang version 12.0.0

Side-note: my error message is also not exactly the same hebasto's at the moment, it specifies clang-12: instead of just clang:

clang-12: warning: argument unused during compilation: '-fstack-clash-protection' [-Wunused-command-line-argument]

hebasto · 2021-05-10T21:40:57Z

@fjahr

Which way did you get your clang installation in?

fjahr · 2021-05-10T23:03:37Z

@fjahr

Which way did you get your clang installation in?

Homebrew

hebasto · 2021-05-22T21:25:30Z

@fanquake @practicalswift

If you have a chance, let me know how this PR could be improved, please.

fanquake · 2021-05-24T04:33:16Z

If you have a chance, let me know how this PR could be improved, please.

Can you change the commit message, and include an explanation of the change in the commit body. As I mentioned above, the change being made here isn't macOS specific, and the macOS version, 11.3 (currently included in the commit message), is irrelevant. The version of Apple Clang you use is governed by the version of Xcode that is installed, and I could install Xcode 12.5 on any version of macOS 11.0+. So this is not a problem specific to macOS 11.3+, or even to Apple Clang, as compiling using LLVM Clang on macOS produces the same warning output.

Regardless of that, if i remember correctly I've seen the same warnings when compiling with Clang on *BSDs. Although it looks like support may soon be enabled there, see https://reviews.freebsd.org/D27366. Related discussion in https://reviews.llvm.org/D92100 as well.

So ideally we have a commit message more like this:

build: convert warnings into errors when testing for -fstack-clash-protection

When building with Clang, if `-fstack-clash-protection` is used with an unsupported target, it may result in hundreds of `-Wunused-command-line-argument` warnings at compile time. This is currently the case when building for at least Darwin using Apple or LLVM Clang.

Unsupported targets may also include *BSD, however that is changing; see further discussion in https://reviews.llvm.org/D92245 and https://reviews.freebsd.org/D27366. 

Note that this option is already skipped for Windows.

…otection When building with Clang, if `-fstack-clash-protection` is used with an unsupported target, it may result in hundreds of `-Wunused-command-line-argument` warnings at compile time. This is currently the case when building for at least Darwin using Apple or LLVM Clang. Unsupported targets may also include *BSD, however that is changing; see further discussion in https://reviews.llvm.org/D92245 and https://reviews.freebsd.org/D27366. Note that this option is already skipped for Windows.

hebasto · 2021-05-24T05:58:51Z

@fanquake

Thanks! Updated.

jarolrod · 2021-05-24T14:56:00Z

re-ACK e9f948c

Don't see anything to review between b9fe3af and e9f948c. The only difference is the commit message.

Sjors · 2021-05-24T15:41:34Z

tACK e9f948c on macOS 11.3.1

…warnings e9f948c build: Convert warnings into errors when testing for -fstack-clash-protection (Hennadii Stepanov) Pull request description: Apple clang version 12.0.5 (clang-1205.0.22.9) that is a part of Xcode 12.5, and is based on LLVM clang 11.1.0, fires spammy warnings: ``` clang: warning: argument unused during compilation: '-fstack-clash-protection' [-Wunused-command-line-argument] ``` From the https://github.com/apple/llvm-project: ``` $ git log --oneline | grep 'stack-clash-protection' 00065d5cbd02 Revert "-fstack-clash-protection: Return an actual error when used on unsupported OS" 4d59c8fdb955 -fstack-clash-protection: Return an actual error when used on unsupported OS df3bfaa39071 [Driver] Change -fnostack-clash-protection to -fno-stack-clash-protection 68e07da3e5d5 [clang][PowerPC] Enable -fstack-clash-protection option for ppc64 515bfc66eace [SystemZ] Implement -fstack-clash-protection e67cbac81211 Support -fstack-clash-protection for x86 454621160066 Revert "Support -fstack-clash-protection for x86" 0fd51a4554f5 Support -fstack-clash-protection for x86 658495e6ecd4 Revert "Support -fstack-clash-protection for x86" e229017732bc Support -fstack-clash-protection for x86 b03c3d8c6209 Revert "Support -fstack-clash-protection for x86" 4a1a0690ad68 Support -fstack-clash-protection for x86 f6d98429fcdb Revert "Support -fstack-clash-protection for x86" 39f50da2a357 Support -fstack-clash-protection for x86 ``` I suppose, that Apple clang-1205.0.22.9 ends with on of the "Revert..." commits. This PR prevents using of the `-fstack-clash-protection` flag if it causes warnings. --- System: macOS Big Sur 11.3 (20E232). ACKs for top commit: jarolrod: re-ACK e9f948c Sjors: tACK e9f948c on macOS 11.3.1 Tree-SHA512: 30186da67f9b0f34418014860c766c2e7f622405520f1cbbc1095d4aa4038b0a86014d76076f318a4b1b09170a96d8167c21d7f53a760e26017f486e1a7d39d4

hebasto added Build system macOS labels Apr 27, 2021

maflcko added Needs gitian build DrahtBot Guix build requested labels Apr 27, 2021

jarolrod reviewed Apr 27, 2021

View reviewed changes

fanquake removed the macOS label Apr 28, 2021

hebasto changed the title ~~build: Silent unused -fstack-clash-protection warnings on macOS 11.3~~ build: Silent [-Wunused-command-line-argument] warnings with Apple clang 12.0.5 Apr 28, 2021

DrahtBot removed the Needs gitian build label Apr 29, 2021

Sjors reviewed May 5, 2021

View reviewed changes

DrahtBot removed the DrahtBot Guix build requested label May 13, 2021

laanwj changed the title ~~build: Silent [-Wunused-command-line-argument] warnings with Apple clang 12.0.5~~ build: Silence [-Wunused-command-line-argument] warnings with Apple clang 12.0.5 May 19, 2021

hebasto force-pushed the 210427-clash branch from b9fe3af to e9f948c Compare May 24, 2021 05:58

hebasto changed the title ~~build: Silence [-Wunused-command-line-argument] warnings with Apple clang 12.0.5~~ build: Silence [-Wunused-command-line-argument] warnings May 24, 2021

bitcoin deleted a comment from DrahtBot May 24, 2021

maflcko added the DrahtBot Guix build requested label May 24, 2021

fanquake merged commit 7041d25 into bitcoin:master May 25, 2021

hebasto deleted the 210427-clash branch May 25, 2021 08:32

maflcko removed the DrahtBot Guix build requested label May 31, 2021

barton2526 mentioned this pull request Sep 19, 2021

build: fix -fstack-clash-protection spam for Clang gridcoin-community/Gridcoin-Research#2340

Merged

gwillen pushed a commit to ElementsProject/elements that referenced this pull request Jun 1, 2022

Merge 7041d25 into merged_master (Bitcoin PR bitcoin/bitcoin#21788)

ec1cd32

bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022

Conversation

hebasto commented Apr 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jarolrod left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fanquake commented Apr 28, 2021

Uh oh!

willcl-ark commented Apr 28, 2021

Uh oh!

hebasto commented Apr 28, 2021

Uh oh!

willcl-ark commented Apr 28, 2021

Uh oh!

hebasto commented Apr 28, 2021

Uh oh!

Sjors left a comment

Choose a reason for hiding this comment

Uh oh!

Sjors May 5, 2021

Choose a reason for hiding this comment

Uh oh!

hebasto May 7, 2021

Choose a reason for hiding this comment

Uh oh!

jarolrod May 24, 2021

Choose a reason for hiding this comment

Uh oh!

hebasto May 24, 2021

Choose a reason for hiding this comment

Uh oh!

practicalswift commented May 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fjahr commented May 9, 2021

Uh oh!

hebasto commented May 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

practicalswift commented May 9, 2021

Uh oh!

jarolrod commented May 9, 2021

Uh oh!

fjahr commented May 10, 2021

Uh oh!

hebasto commented May 10, 2021

Uh oh!

fjahr commented May 10, 2021

Uh oh!

hebasto commented May 22, 2021

Uh oh!

fanquake commented May 24, 2021

Uh oh!

hebasto commented May 24, 2021

Uh oh!

jarolrod commented May 24, 2021

Uh oh!

Sjors commented May 24, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

hebasto commented Apr 27, 2021 •

edited

Loading

jarolrod left a comment •

edited

Loading

practicalswift commented May 7, 2021 •

edited

Loading

hebasto commented May 9, 2021 •

edited

Loading