Merge bitcoin/bitcoin#29205: build: always set -g -O2 in CORE_CXXFLAGS

00c1e2aa44 build: fix optimisation flags used for --coverage (fanquake)
1dc2c9b385 ci: cleanup C*FLAG usage in Valgrind jobs (fanquake)
6cc2a38c13 build: add sanitizer flags to configure output (fanquake)
08cd5aca18 build: always set -g -O2 in CORE_CXXFLAGS (fanquake)

Pull request description:

  Rather than trying to sporadically rely on / override Autoconf default behaviour. Just always override (if unset), and always set the flags we want (which are the same as the Autoconf defaults).

  Removes the need for duplicate code to clear (if not overridden) `CXXFLAGS`.

  Fixes cases of "missing" `-O2`. i.e this PR when running a Valgrind CI job with changes here:
  ```bash
  CXXFLAGS        =  -g -O2  -fdebug-prefix-map=$(abs_top_srcdir)=.  -Wstack-protector -fstack-protector-all -mbranch-protection=bti   -Werror  -fsanitize=fuzzer  -gdwarf-4
  ```

  Fixes configure output to reflect actual compilation flag ordering, so it's useful.

  Note that if we do still end up with a duplicate "-g -O2" when compiling, that has no effect, and I don't really thinks it's something worth trying to optimize.

ACKs for top commit:
  TheCharlatan:
    lgtm ACK 00c1e2aa44
  hebasto:
    ACK 00c1e2aa44, I have reviewed the code and it looks OK. Also tested `ci/test/00_setup_env_native_valgrind.sh`.
  theuni:
    ACK 00c1e2aa44

Tree-SHA512: cf6c7acf813ba10b198561e83eb72e9b2532a39cb1767c452d031e82921dcd42a47b129735b24c4e36131fd0c8fe7457f7cae870c1e011cdfdd430bdc4d4912b

ci/test/00_setup_env_native_fuzz_with_valgrind.sh

@@ -16,5 +16,5 @@ export RUN_FUZZ_TESTS=true
 export FUZZ_TESTS_CONFIG="--valgrind"
 export GOAL="install"
 # Temporarily pin dwarf 4, until using Valgrind 3.20 or later
-export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer CC='clang -gdwarf-4' CXX='clang++ -gdwarf-4'"
+export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer CC=clang CXX=clang++ CFLAGS=-gdwarf-4 CXXFLAGS=-gdwarf-4"
 export CCACHE_MAXSIZE=200M

ci/test/00_setup_env_native_valgrind.sh

@@ -14,4 +14,4 @@ export NO_DEPENDS=1
 export TEST_RUNNER_EXTRA="--exclude feature_init,rpc_bind,feature_bind_extra"  # Excluded for now, see https://github.com/bitcoin/bitcoin/issues/17765#issuecomment-602068547
 export GOAL="install"
 # Temporarily pin dwarf 4, until using Valgrind 3.20 or later
-export BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=no CC='clang -gdwarf-4' CXX='clang++ -gdwarf-4'"  # TODO enable GUI
+export BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=no CC=clang CXX=clang++ CFLAGS=-gdwarf-4 CXXFLAGS=-gdwarf-4"  # TODO enable GUI

configure.ac

@@ -323,6 +323,15 @@ AC_ARG_ENABLE([external-signer],
 
 AC_LANG_PUSH([C++])
 
+dnl Always set -g -O2 in our CXXFLAGS. Autoconf will try and set CXXFLAGS to "-g -O2" by default,
+dnl so we suppress that (if CXXFLAGS hasn't been overridden by the user), given we are adding it
+dnl ourselves.
+CORE_CXXFLAGS="$CORE_CXXFLAGS -g -O2"
+
+if test "$CXXFLAGS_overridden" = "no"; then
+  CXXFLAGS=""
+fi
+
 dnl Check for a flag to turn compiler warnings into errors. This is helpful for checks which may
 dnl appear to succeed because by default they merely emit warnings when they fail.
 dnl
@@ -347,12 +356,6 @@ case $host in
 esac
 
 if test "$enable_debug" = "yes"; then
-  dnl If debugging is enabled, and the user hasn't overridden CXXFLAGS, clear
-  dnl them, to prevent autoconfs "-g -O2" being added. Otherwise we'd end up
-  dnl with "-O0 -g3 -g -O2".
-  if test "$CXXFLAGS_overridden" = "no"; then
-  CXXFLAGS=""
-  fi
 
   dnl Disable all optimizations
   AX_CHECK_COMPILE_FLAG([-O0], [DEBUG_CXXFLAGS="$DEBUG_CXXFLAGS -O0"], [], [$CXXFLAG_WERROR])
@@ -859,13 +862,7 @@ if test "$use_lcov" = "yes"; then
     [AC_MSG_ERROR([lcov testing requested but --coverage linker flag does not work])])
   AX_CHECK_COMPILE_FLAG([--coverage],[CORE_CXXFLAGS="$CORE_CXXFLAGS --coverage"],
     [AC_MSG_ERROR([lcov testing requested but --coverage flag does not work])])
-  dnl If coverage is enabled, and the user hasn't overridden CXXFLAGS, clear
-  dnl them, to prevent autoconfs "-g -O2" being added. Otherwise we'd end up
-  dnl with "--coverage -Og -O0 -g -O2".
-  if test "$CXXFLAGS_overridden" = "no"; then
-  CXXFLAGS=""
-  fi
-  CORE_CXXFLAGS="$CORE_CXXFLAGS -Og -O0"
+  CORE_CXXFLAGS="$CORE_CXXFLAGS -Og"
 fi
 
 if test "$use_lcov_branch" != "no"; then
@@ -1996,8 +1993,8 @@ echo "  CC              = $CC"
 echo "  CFLAGS          = $PTHREAD_CFLAGS $CFLAGS"
 echo "  CPPFLAGS        = $DEBUG_CPPFLAGS $HARDENED_CPPFLAGS $CORE_CPPFLAGS $CPPFLAGS"
 echo "  CXX             = $CXX"
-echo "  CXXFLAGS        = $DEBUG_CXXFLAGS $HARDENED_CXXFLAGS $WARN_CXXFLAGS $NOWARN_CXXFLAGS $ERROR_CXXFLAGS $GPROF_CXXFLAGS $CORE_CXXFLAGS $CXXFLAGS"
-echo "  LDFLAGS         = $PTHREAD_LIBS $HARDENED_LDFLAGS $GPROF_LDFLAGS $CORE_LDFLAGS $LDFLAGS"
+echo "  CXXFLAGS        = $CORE_CXXFLAGS $DEBUG_CXXFLAGS $HARDENED_CXXFLAGS $WARN_CXXFLAGS $NOWARN_CXXFLAGS $ERROR_CXXFLAGS $GPROF_CXXFLAGS $SANITIZER_CXXFLAGS $CXXFLAGS"
+echo "  LDFLAGS         = $PTHREAD_LIBS $HARDENED_LDFLAGS $GPROF_LDFLAGS $SANITIZER_LDFLAGS $CORE_LDFLAGS $LDFLAGS"
 echo "  AR              = $AR"
 echo "  ARFLAGS         = $ARFLAGS"
 echo

src/Makefile.am

@@ -9,7 +9,7 @@ print-%: FORCE
 DIST_SUBDIRS = secp256k1
 
 AM_LDFLAGS = $(LIBTOOL_LDFLAGS) $(HARDENED_LDFLAGS) $(GPROF_LDFLAGS) $(SANITIZER_LDFLAGS) $(CORE_LDFLAGS)
-AM_CXXFLAGS = $(DEBUG_CXXFLAGS) $(HARDENED_CXXFLAGS) $(WARN_CXXFLAGS) $(NOWARN_CXXFLAGS) $(ERROR_CXXFLAGS) $(GPROF_CXXFLAGS) $(SANITIZER_CXXFLAGS) $(CORE_CXXFLAGS)
+AM_CXXFLAGS = $(CORE_CXXFLAGS) $(DEBUG_CXXFLAGS) $(HARDENED_CXXFLAGS) $(WARN_CXXFLAGS) $(NOWARN_CXXFLAGS) $(ERROR_CXXFLAGS) $(GPROF_CXXFLAGS) $(SANITIZER_CXXFLAGS)
 AM_CPPFLAGS = $(DEBUG_CPPFLAGS) $(HARDENED_CPPFLAGS) $(CORE_CPPFLAGS)
 AM_LIBTOOLFLAGS = --preserve-dup-deps
 PTHREAD_FLAGS = $(PTHREAD_CFLAGS) $(PTHREAD_LIBS)