Skip to content

Commit

Permalink
Allow overriding default flags
Browse files Browse the repository at this point in the history
Summary:
```
Right now, it's not easy to reduce the optimization level with CFLAGS
because configure overwrites any optimization flag with -O3. The
automake documentation states that:

    The reason ‘$(CPPFLAGS)’ appears after ‘$(AM_CPPFLAGS)’ or
‘$(mumble_CPPFLAGS)’ in the compile command is that users should always
have the last say.

and also that it's incorrect to redefine CFLAGS in the first place

    You should never redefine a user variable such as CPPFLAGS in
Makefile.am. [...] You should not add options to these user variables
within configure either, for the same reason

With this PR CFLAGS is still redefined, but user-provided flags appear
after the default CFLAGS which means that they override the default
flags (at least in clang and gcc). Otherwise, the default configuration
is not changed. This also means that if CFLAGS are defined by the user,
then -g is not added (which does not seem to make much sense). In order
to keep the -O3 despite the reordering we need to explicitly tell
autoconf to not append -O2 by setting the default to -g with :
${CFLAGS="-g"} as per the manual
```

Backport of secp256k1 [[bitcoin-core/secp256k1#700 | PR700]].

Test Plan:
  make check
  ninja check-secp256k1

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D6309
  • Loading branch information
jonasnick authored and Fabcien committed Jun 2, 2020
1 parent b2531eb commit e1a3a76
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 13 deletions.
2 changes: 0 additions & 2 deletions src/secp256k1/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ if(NOT CMAKE_BUILD_TYPE)
set(__NO_USER_CMAKE_BUILD_TYPE ON CACHE BOOL "True if the user didn't set a build type on the command line")
endif()

set(CMAKE_C_FLAGS_RELWITHDEBINFO "-g -O3")

option(SECP256K1_ENABLE_COVERAGE "Enable coverage" OFF)
option(SECP256K1_ENABLE_BRANCH_COVERAGE "Enable branch coverage" OFF)

Expand Down
23 changes: 12 additions & 11 deletions src/secp256k1/configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ AH_TOP([#ifndef LIBSECP256K1_CONFIG_H])
AH_TOP([#define LIBSECP256K1_CONFIG_H])
AH_BOTTOM([#endif /*LIBSECP256K1_CONFIG_H*/])
AM_INIT_AUTOMAKE([foreign subdir-objects])

# Set -g if CFLAGS are not already set, which matches the default autoconf
# behavior (see PROG_CC in the Autoconf manual) with the exception that we don't
# set -O2 here because we set it in any case (see further down).
: ${CFLAGS="-g"}
LT_INIT

dnl make the compilation flags quiet unless V=1 is used
Expand All @@ -19,10 +24,6 @@ AC_PATH_TOOL(RANLIB, ranlib)
AC_PATH_TOOL(STRIP, strip)
AX_PROG_CC_FOR_BUILD

if test "x$CFLAGS" = "x"; then
CFLAGS="-g"
fi

AM_PROG_CC_C_O

AC_PROG_CC_C89
Expand Down Expand Up @@ -63,11 +64,11 @@ case $host_os in
;;
esac

CFLAGS="$CFLAGS -W"
CFLAGS="-W $CFLAGS"

warn_CFLAGS="-std=c89 -pedantic -Wall -Wextra -Wcast-align -Wnested-externs -Wshadow -Wstrict-prototypes -Wno-unused-function -Wno-long-long -Wno-overlength-strings"
saved_CFLAGS="$CFLAGS"
CFLAGS="$CFLAGS $warn_CFLAGS"
CFLAGS="$warn_CFLAGS $CFLAGS"
AC_MSG_CHECKING([if ${CC} supports ${warn_CFLAGS}])
AC_COMPILE_IFELSE([AC_LANG_SOURCE([[char foo;]])],
[ AC_MSG_RESULT([yes]) ],
Expand All @@ -76,7 +77,7 @@ AC_COMPILE_IFELSE([AC_LANG_SOURCE([[char foo;]])],
])

saved_CFLAGS="$CFLAGS"
CFLAGS="$CFLAGS -fvisibility=hidden"
CFLAGS="-fvisibility=hidden $CFLAGS"
AC_MSG_CHECKING([if ${CC} supports -fvisibility=hidden])
AC_COMPILE_IFELSE([AC_LANG_SOURCE([[char foo;]])],
[ AC_MSG_RESULT([yes]) ],
Expand Down Expand Up @@ -190,10 +191,10 @@ AM_CONDITIONAL([VALGRIND_ENABLED],[test "$enable_valgrind" = "yes"])

if test x"$enable_coverage" = x"yes"; then
AC_DEFINE(COVERAGE, 1, [Define this symbol to compile out all VERIFY code])
CFLAGS="$CFLAGS -O0 --coverage"
LDFLAGS="$LDFLAGS --coverage"
CFLAGS="-O0 --coverage $CFLAGS"
LDFLAGS="--coverage $LDFLAGS"
else
CFLAGS="$CFLAGS -O3"
CFLAGS="-O2 $CFLAGS"
fi

if test x"$use_ecmult_static_precomputation" != x"no"; then
Expand All @@ -211,7 +212,7 @@ if test x"$use_ecmult_static_precomputation" != x"no"; then

warn_CFLAGS_FOR_BUILD="-Wall -Wextra -Wno-unused-function"
saved_CFLAGS="$CFLAGS"
CFLAGS="$CFLAGS $warn_CFLAGS_FOR_BUILD"
CFLAGS="$warn_CFLAGS_FOR_BUILD $CFLAGS"
AC_MSG_CHECKING([if native ${CC_FOR_BUILD} supports ${warn_CFLAGS_FOR_BUILD}])
AC_COMPILE_IFELSE([AC_LANG_SOURCE([[char foo;]])],
[ AC_MSG_RESULT([yes]) ],
Expand Down

0 comments on commit e1a3a76

Please sign in to comment.