Skip to content

Commit

Permalink
Merge bitcoin-core/secp256k1#944: Various improvements related to CFLAGS
Browse files Browse the repository at this point in the history 
0302138 ci: Make compiler warning into errors on CI (Tim Ruffing)
b924e1e build: Ensure that configure's compile checks default to -O2 (Tim Ruffing)
7939cd5 build: List *CPPFLAGS before *CFLAGS like on the compiler command line (Tim Ruffing)
595e8a3 build: Enable -Wcast-align=strict warning (Tim Ruffing)
0725626 build: Use own variable SECP_CFLAGS instead of touching user CFLAGS (Tim Ruffing)

Pull request description:

ACKs for top commit:
  jonasnick:
    ACK 0302138

Tree-SHA512: 619eb6b512ae0eb8c51134f5bb1b7bc7a397321dc51073ae3117f9433505ec19b407518b47a181163e1a841216b20487c7a50c6f5045faffa5cfa7fad0b8c906
  • Loading branch information
@jonasnick
jonasnick committed Jul 1, 2021
2 parents 4866178 + 0302138 commit 446d28d
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 63 deletions.
21 changes: 14 additions & 7 deletions .cirrus.yml
View file
Original file line number Diff line number Diff line change
@@ -1,21 +1,28 @@
env:
WIDEMUL: auto
### compiler options
HOST:
# Specific warnings can be disabled with -Wno-error=foo.
# -pedantic-errors is not equivalent to -Werror=pedantic and thus not implied by -Werror according to the GCC manual.
WERROR_CFLAGS: -Werror -pedantic-errors
MAKEFLAGS: -j2
BUILD: check
### secp256k1 config
STATICPRECOMPUTATION: yes
ECMULTGENPRECISION: auto
ASM: no
BUILD: check
WIDEMUL: auto
WITH_VALGRIND: yes
EXTRAFLAGS:
HOST:
### secp256k1 modules
EXPERIMENTAL: no
ECDH: no
RECOVERY: no
SCHNORRSIG: no
EXPERIMENTAL: no
CTIMETEST: yes
BENCH: yes
### test options
TEST_ITERS:
BENCH: yes
BENCH_ITERS: 2
MAKEFLAGS: -j2
CTIMETEST: yes

cat_logs_snippet: &CAT_LOGS
always:
Expand Down
1 change: 1 addition & 0 deletions .gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ aclocal.m4
autom4te.cache/
config.log
config.status
conftest*
*.tar.gz
*.la
libtool
Expand Down
8 changes: 6 additions & 2 deletions Makefile.am
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
ACLOCAL_AMFLAGS = -I build-aux/m4

# AM_CFLAGS will be automatically prepended to CFLAGS by Automake when compiling some foo
# which does not have an explicit foo_CFLAGS variable set.
AM_CFLAGS = $(SECP_CFLAGS)

lib_LTLIBRARIES = libsecp256k1.la
include_HEADERS = include/secp256k1.h
include_HEADERS += include/secp256k1_preallocated.h
Expand Down Expand Up @@ -129,10 +133,10 @@ CPPFLAGS_FOR_BUILD +=-I$(top_srcdir) -I$(builddir)/src
gen_context_OBJECTS = gen_context.o
gen_context_BIN = gen_context$(BUILD_EXEEXT)
gen_%.o: src/gen_%.c src/libsecp256k1-config.h
$(CC_FOR_BUILD) $(DEFS) $(CPPFLAGS_FOR_BUILD) $(CFLAGS_FOR_BUILD) -c $< -o $@
$(CC_FOR_BUILD) $(DEFS) $(CPPFLAGS_FOR_BUILD) $(SECP_CFLAGS_FOR_BUILD) $(CFLAGS_FOR_BUILD) -c $< -o $@

$(gen_context_BIN): $(gen_context_OBJECTS)
$(CC_FOR_BUILD) $(CFLAGS_FOR_BUILD) $(LDFLAGS_FOR_BUILD) $^ -o $@
$(CC_FOR_BUILD) $(SECP_CFLAGS_FOR_BUILD) $(CFLAGS_FOR_BUILD) $(LDFLAGS_FOR_BUILD) $^ -o $@

$(libsecp256k1_la_OBJECTS): src/ecmult_static_context.h
$(tests_OBJECTS): src/ecmult_static_context.h
Expand Down
16 changes: 16 additions & 0 deletions build-aux/m4/bitcoin_secp.m4
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,19 @@ if test x"$has_valgrind" != x"yes"; then
AC_CHECK_HEADER([valgrind/memcheck.h], [has_valgrind=yes; AC_DEFINE(HAVE_VALGRIND,1,[Define this symbol if valgrind is installed])])
fi
])

dnl SECP_TRY_APPEND_CFLAGS(flags, VAR)
dnl Append flags to VAR if CC accepts them.
AC_DEFUN([SECP_TRY_APPEND_CFLAGS], [
AC_MSG_CHECKING([if ${CC} supports $1])
SECP_TRY_APPEND_CFLAGS_saved_CFLAGS="$CFLAGS"
CFLAGS="$1 $CFLAGS"
AC_COMPILE_IFELSE([AC_LANG_SOURCE([[char foo;]])], [flag_works=yes], [flag_works=no])
AC_MSG_RESULT($flag_works)
CFLAGS="$SECP_TRY_APPEND_CFLAGS_saved_CFLAGS"
if test x"$flag_works" = x"yes"; then
$2="$$2 $1"
fi
unset flag_works
AC_SUBST($2)
])
112 changes: 58 additions & 54 deletions configure.ac
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@ 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

# Make the compilation flags quiet unless V=1 is used.
Expand Down Expand Up @@ -70,35 +66,41 @@ case $host_os in
;;
esac

CFLAGS="-W $CFLAGS"

warn_CFLAGS="-std=c89 -pedantic -Wall -Wextra -Wcast-align -Wnested-externs -Wshadow -Wstrict-prototypes -Wundef -Wno-unused-function -Wno-long-long -Wno-overlength-strings"
saved_CFLAGS="$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]) ],
[ AC_MSG_RESULT([no])
CFLAGS="$saved_CFLAGS"
])

saved_CFLAGS="$CFLAGS"
CFLAGS="-Wconditional-uninitialized $CFLAGS"
AC_MSG_CHECKING([if ${CC} supports -Wconditional-uninitialized])
AC_COMPILE_IFELSE([AC_LANG_SOURCE([[char foo;]])],
[ AC_MSG_RESULT([yes]) ],
[ AC_MSG_RESULT([no])
CFLAGS="$saved_CFLAGS"
])

saved_CFLAGS="$CFLAGS"
CFLAGS="-fvisibility=hidden $CFLAGS"
AC_MSG_CHECKING([if ${CC} supports -fvisibility=hidden])
AC_COMPILE_IFELSE([AC_LANG_SOURCE([[char foo;]])],
[ AC_MSG_RESULT([yes]) ],
[ AC_MSG_RESULT([no])
CFLAGS="$saved_CFLAGS"
])
# Try if some desirable compiler flags are supported and append them to SECP_CFLAGS.
#
# These are our own flags, so we append them to our own SECP_CFLAGS variable (instead of CFLAGS) as
# recommended in the automake manual (Section "Flag Variables Ordering"). CFLAGS belongs to the user
# and we are not supposed to touch it. In the Makefile, we will need to ensure that SECP_CFLAGS
# is prepended to CFLAGS when invoking the compiler so that the user always has the last word (flag).
#
# Another advantage of not touching CFLAGS is that the contents of CFLAGS will be picked up by
# libtool for compiling helper executables. For example, when compiling for Windows, libtool will
# generate entire wrapper executables (instead of simple wrapper scripts as on Unix) to ensure
# proper operation of uninstalled programs linked by libtool against the uninstalled shared library.
# These executables are compiled from C source file for which our flags may not be appropriate,
# e.g., -std=c89 flag has lead to undesirable warnings in the past.
#
# TODO We should analogously not touch CPPFLAGS and LDFLAGS but currently there are no issues.
AC_DEFUN([SECP_TRY_APPEND_DEFAULT_CFLAGS], [
# Try to append -Werror=unknown-warning-option to CFLAGS temporarily. Otherwise clang will
# not error out if it gets unknown warning flags and the checks here will always succeed
# no matter if clang knows the flag or not.
SECP_TRY_APPEND_DEFAULT_CFLAGS_saved_CFLAGS="$CFLAGS"
SECP_TRY_APPEND_CFLAGS([-Werror=unknown-warning-option], CFLAGS)
SECP_TRY_APPEND_CFLAGS([-std=c89 -pedantic -Wno-long-long -Wnested-externs -Wshadow -Wstrict-prototypes -Wundef], $1) # GCC >= 3.0, -Wlong-long is implied by -pedantic.
SECP_TRY_APPEND_CFLAGS([-Wno-overlength-strings], $1) # GCC >= 4.2, -Woverlength-strings is implied by -pedantic.
SECP_TRY_APPEND_CFLAGS([-Wall], $1) # GCC >= 2.95 and probably many other compilers
SECP_TRY_APPEND_CFLAGS([-Wno-unused-function], $1) # GCC >= 3.0, -Wunused-function is implied by -Wall.
SECP_TRY_APPEND_CFLAGS([-Wextra], $1) # GCC >= 3.4, this is the newer name of -W, which we don't use because older GCCs will warn about unused functions.
SECP_TRY_APPEND_CFLAGS([-Wcast-align], $1) # GCC >= 2.95
SECP_TRY_APPEND_CFLAGS([-Wcast-align=strict], $1) # GCC >= 8.0
SECP_TRY_APPEND_CFLAGS([-Wconditional-uninitialized], $1) # Clang >= 3.0 only
SECP_TRY_APPEND_CFLAGS([-fvisibility=hidden], $1) # GCC >= 4.0
CFLAGS="$SECP_TRY_APPEND_DEFAULT_CFLAGS_saved_CFLAGS"
])
SECP_TRY_APPEND_DEFAULT_CFLAGS(SECP_CFLAGS)

###
### Define config arguments
Expand Down Expand Up @@ -213,10 +215,14 @@ 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="-O0 --coverage $CFLAGS"
SECP_CFLAGS="-O0 --coverage $SECP_CFLAGS"
LDFLAGS="--coverage $LDFLAGS"
else
CFLAGS="-O2 $CFLAGS"
# Most likely the CFLAGS already contain -O2 because that is autoconf's default.
# We still add it here because passing it twice is not an issue, and handling
# this case would just add unnecessary complexity (see #896).
SECP_CFLAGS="-O2 $SECP_CFLAGS"
SECP_CFLAGS_FOR_BUILD="-O2 $SECP_CFLAGS_FOR_BUILD"
fi

if test x"$req_asm" = x"auto"; then
Expand Down Expand Up @@ -351,6 +357,9 @@ if test x"$enable_valgrind" = x"yes"; then
SECP_INCLUDES="$SECP_INCLUDES $VALGRIND_CPPFLAGS"
fi

# Add -Werror and similar flags passed from the outside (for testing, e.g., in CI)
SECP_CFLAGS="$SECP_CFLAGS $WERROR_CFLAGS"

# Handle static precomputation (after everything which modifies CFLAGS and friends)
if test x"$use_ecmult_static_precomputation" != x"no"; then
if test x"$cross_compiling" = x"no"; then
Expand All @@ -360,8 +369,9 @@ if test x"$use_ecmult_static_precomputation" != x"no"; then
fi
# If we're not cross-compiling, simply use the same compiler for building the static precompation code.
CC_FOR_BUILD="$CC"
CFLAGS_FOR_BUILD="$CFLAGS"
CPPFLAGS_FOR_BUILD="$CPPFLAGS"
SECP_CFLAGS_FOR_BUILD="$SECP_CFLAGS"
CFLAGS_FOR_BUILD="$CFLAGS"
LDFLAGS_FOR_BUILD="$LDFLAGS"
else
AX_PROG_CC_FOR_BUILD
Expand All @@ -371,42 +381,32 @@ if test x"$use_ecmult_static_precomputation" != x"no"; then
cross_compiling=no
SAVE_CC="$CC"
CC="$CC_FOR_BUILD"
SAVE_CFLAGS="$CFLAGS"
CFLAGS="$CFLAGS_FOR_BUILD"
SAVE_CPPFLAGS="$CPPFLAGS"
CPPFLAGS="$CPPFLAGS_FOR_BUILD"
SAVE_CFLAGS="$CFLAGS"
CFLAGS="$CFLAGS_FOR_BUILD"
SAVE_LDFLAGS="$LDFLAGS"
LDFLAGS="$LDFLAGS_FOR_BUILD"

warn_CFLAGS_FOR_BUILD="-Wall -Wextra -Wno-unused-function"
saved_CFLAGS="$CFLAGS"
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]) ],
[ AC_MSG_RESULT([no])
CFLAGS="$saved_CFLAGS"
])
SECP_TRY_APPEND_DEFAULT_CFLAGS(SECP_CFLAGS_FOR_BUILD)

AC_MSG_CHECKING([for working native compiler: ${CC_FOR_BUILD}])
AC_RUN_IFELSE(
[AC_LANG_PROGRAM([], [])],
[working_native_cc=yes],
[working_native_cc=no],[:])

CFLAGS_FOR_BUILD="$CFLAGS"

# Restore the environment
cross_compiling=$save_cross_compiling
CC="$SAVE_CC"
CFLAGS="$SAVE_CFLAGS"
CPPFLAGS="$SAVE_CPPFLAGS"
CFLAGS="$SAVE_CFLAGS"
LDFLAGS="$SAVE_LDFLAGS"

if test x"$working_native_cc" = x"no"; then
AC_MSG_RESULT([no])
set_precomp=no
m4_define([please_set_for_build], [Please set CC_FOR_BUILD, CFLAGS_FOR_BUILD, CPPFLAGS_FOR_BUILD, and/or LDFLAGS_FOR_BUILD.])
m4_define([please_set_for_build], [Please set CC_FOR_BUILD, CPPFLAGS_FOR_BUILD, CFLAGS_FOR_BUILD, and/or LDFLAGS_FOR_BUILD.])
if test x"$use_ecmult_static_precomputation" = x"yes"; then
AC_MSG_ERROR([native compiler ${CC_FOR_BUILD} does not produce working binaries. please_set_for_build])
else
Expand All @@ -419,8 +419,9 @@ if test x"$use_ecmult_static_precomputation" != x"no"; then
fi

AC_SUBST(CC_FOR_BUILD)
AC_SUBST(CFLAGS_FOR_BUILD)
AC_SUBST(CPPFLAGS_FOR_BUILD)
AC_SUBST(SECP_CFLAGS_FOR_BUILD)
AC_SUBST(CFLAGS_FOR_BUILD)
AC_SUBST(LDFLAGS_FOR_BUILD)
else
set_precomp=no
Expand Down Expand Up @@ -490,6 +491,7 @@ AC_SUBST(SECP_INCLUDES)
AC_SUBST(SECP_LIBS)
AC_SUBST(SECP_TEST_LIBS)
AC_SUBST(SECP_TEST_INCLUDES)
AC_SUBST(SECP_CFLAGS)
AM_CONDITIONAL([ENABLE_COVERAGE], [test x"$enable_coverage" = x"yes"])
AM_CONDITIONAL([USE_TESTS], [test x"$use_tests" != x"no"])
AM_CONDITIONAL([USE_EXHAUSTIVE_TESTS], [test x"$use_exhaustive_tests" != x"no"])
Expand Down Expand Up @@ -532,13 +534,15 @@ fi
echo
echo " valgrind = $enable_valgrind"
echo " CC = $CC"
echo " CFLAGS = $CFLAGS"
echo " CPPFLAGS = $CPPFLAGS"
echo " SECP_CFLAGS = $SECP_CFLAGS"
echo " CFLAGS = $CFLAGS"
echo " LDFLAGS = $LDFLAGS"
echo
if test x"$set_precomp" = x"yes"; then
echo " CC_FOR_BUILD = $CC_FOR_BUILD"
echo " CFLAGS_FOR_BUILD = $CFLAGS_FOR_BUILD"
echo " CPPFLAGS_FOR_BUILD = $CPPFLAGS_FOR_BUILD"
echo " SECP_CFLAGS_FOR_BUILD = $SECP_CFLAGS_FOR_BUILD"
echo " CFLAGS_FOR_BUILD = $CFLAGS_FOR_BUILD"
echo " LDFLAGS_FOR_BUILD = $LDFLAGS_FOR_BUILD"
fi

0 comments on commit 446d28d

Please sign in to comment.