cmake: incorrect assumption that `Debug` build type will not use optimisations

fanquake commented at 3:40 pm on September 3, 2024: member

CMakeLists.txt currently states: https://github.com/bitcoin/bitcoin/blob/9cb9651d92ddb5d92724f6a52440601c7a0bbcf8/CMakeLists.txt#L487-L491

However that isn’t always true. If, for example, depends is built with DEBUG=1, and CMake is configured with -DCMAKE_BUILD_TYPE=Debug, -O1 will actually be used. This is confusing, not just because the documentation is incorrect, but when making a change to switch the build type to Debug in oss-fuzz, it’s caused unexpected results: https://github.com/google/oss-fuzz/pull/12443#issuecomment-2326564546.

Rather than assuming that builds with the Debug build type don’t use optimisations, the check should be fixed to actually check what optimisation level is being used.

i.e:

0make -C depends/ NO_QT=1 NO_WALLET=1 NO_ZMQ=1 NO_UPNP=1 -j17 DEBUG=1
1cmake -B build --toolchain=/root/ci_scratch/depends/x86_64-pc-linux-gnu/toolchain.cmake -DCMAKE_BUILD_TYPE=Debug
2<snip>
3C++ compiler flags .................... -pipe -std=c++20 -O0 -ftrapv -O1 -g3

(-O1 is last, so is what will be used).

fanquake added the label Build system on Sep 3, 2024

ryanofsky commented at 3:48 pm on September 3, 2024: contributor

Not sure if this makes sense, but if the problem is that specifying _FORTIFY_SOURCE without optimizations triggers a compiler warning, maybe there should just be a check that drops _FORTIFY_SOURCE when it triggers the warning. That might avoid problems if there is a discrepancy between the way our cmake code detects optimizations and the way _FORTIFY_SOURCE implementation does.

hebasto commented at 4:05 pm on September 3, 2024: member

However that isn’t always true. If, for example, depends is built with DEBUG=1, and CMake is configured with -DCMAKE_BUILD_TYPE=Debug, -O1 will actually be used.

I think we should first prioritize coordinating the optimization and debugging flags between the depends subsystem and the main build system:

Should the optimization and debugging flags coincide, or are they allowed to differ?
If the optimization and debugging can differ between the depends subsystem and in the main build system, which should take precedence?

UPD. #29796 is related.

fanquake commented at 3:56 pm on September 4, 2024: member

I think we should first prioritize coordinating the optimization and debugging flags between the depends subsystem and the main build system:

I’m not sure? This hardening feature should be turned on if optimisations are being used, and hardening isn’t disabled. Whether or not depends flags override CMake defaults, or if depends defaults match CMake defaults seems tangential.

fanquake referenced this in commit 9a8d337785 on Sep 5, 2024

fanquake referenced this in commit 30803a35d5 on Sep 5, 2024

fanquake commented at 3:36 pm on September 5, 2024: member

Not sure if this makes sense, but if the problem is that specifying _FORTIFY_SOURCE without optimizations triggers a compiler warning, maybe there should just be a check that drops _FORTIFY_SOURCE when it triggers the warning. That might avoid problems if there is a discrepancy between the way our cmake code detects optimizations and the way _FORTIFY_SOURCE implementation does.

That could be a good approach, but there’s also the possiblity that FORTIFY will warn if the selected level will be treated as a lower level (i.e 3 selected, but will be treated as 2), depending on the libc, platform etc, which would cause it’s usage to be inadvertantly disabled. I’ve opted to implement what is (hopefully) a straightforward enough check for optimisations in #30824, which is the same test used by glibc.

fanquake referenced this in commit af482fcc6d on Sep 6, 2024

fanquake closed this on Sep 9, 2024

fanquake referenced this in commit 94bc3c4cc0 on Sep 9, 2024

cmake: incorrect assumption that Debug build type will not use optimisations #30800

cmake: incorrect assumption that `Debug` build type will not use optimisations #30800