build: Do not set _FORTIFY_SOURCE if it causes compiler warnings #20824

pull hebasto wants to merge 1 commits into bitcoin:master from hebasto:210101-fortify changing 1 files +7 −4
  1. hebasto commented at 4:31 pm on January 1, 2021: member

    On master (e75f91eae3936269b40b4bfdfe540d5526270936) passing CXXFLAGS variable to the configure script causes the default -O2 optimization flag is not set in the AC_PROG_CXX macro:

    If output variable CXXFLAGS was not already set, set it to -g -O2 for the GNU C++ compiler (-O2 on systems where G++ does not accept -g), or -g for other compilers.

    Such behavior leads to multiple warnings when compiling with clang 11.0 (on Fedora 33):

    0/usr/include/features.h:397:4: warning: _FORTIFY_SOURCE requires compiling with optimization (-O) [-W#warnings]
    1#  warning _FORTIFY_SOURCE requires compiling with optimization (-O)
    2   ^
    31 warning generated.
    

    This PR ensures that -D_FORTIFY_SOURCE flag won’t cause such warnings.

  2. build: Do not set _FORTIFY_SOURCE if it causes compiler warnings e71dc01cf3
  3. DrahtBot added the label Build system on Jan 1, 2021
  4. luke-jr commented at 5:08 pm on January 1, 2021: member

    Not sure this is a good idea. Better the builder knows he isn’t getting the fortification?

    Also, pretty sure the PR as-is breaks when the compiler has _FORTIFY_SOURCE enabled by default (with just -D, it causes a warning, hence the check for -U)

  5. fanquake commented at 7:56 am on January 4, 2021: member

    Not sure this is a good idea. Better the builder knows he isn’t getting the fortification?

    I agree with Luke. This change is (essentially silently) disabling a security feature in lieu of emitting compiler warnings in situations where you’d actually want them.

    If someone has overridden CXXFLAGS without understanding what they are doing, or any potential consequences, the warnings will alert them to that fact.

    If someone knows what they are doing, and is providing their own CXXFLAGS, they know they can “fix” the warnings by adding an optimize option (-Ox), or, if they don’t want any optimization / don’t want to be using FORTIFY_SOURCE at all, they can add -U_FORTIFY_SOURCE to their CPPFLAGS etc.

  6. hebasto commented at 9:11 am on January 4, 2021: member

    @luke-jr @fanquake Thanks for your comments. Agree with both.

    Closing.

  7. hebasto closed this on Jan 4, 2021

  8. DrahtBot locked this on Aug 16, 2022

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-07-08 19:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me