Skip to content

Commit

Permalink
build: Use own variable SECP_CFLAGS instead of touching user CFLAGS
Browse files Browse the repository at this point in the history 
Fixes one of the items in bitcoin#923, namely the warnings of the form
    '_putenv' redeclared without dllimport attribute:
    previous dllimport ignored [-Wattributes]

This also cleans up the way we add CFLAGS, in particular flags enabling
warnings. Now we perform some more fine-grained checking for flag
support, which is not strictly necessary but the changes also help to
document autoconf.ac.
  • Loading branch information
@real-or-random
real-or-random committed Jul 1, 2021
1 parent 1758a92 commit 0725626
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 43 deletions.
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)
])
83 changes: 42 additions & 41 deletions configure.ac
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,35 +70,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 still touch the CFLAGS for --coverage and -O0/-O2.
# 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([-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 @@ -360,8 +366,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"
SECP_CFLAGS_FOR_BUILD="$SECP_CFLAGS"
CPPFLAGS_FOR_BUILD="$CPPFLAGS"
CFLAGS_FOR_BUILD="$CFLAGS"
LDFLAGS_FOR_BUILD="$LDFLAGS"
else
AX_PROG_CC_FOR_BUILD
Expand All @@ -378,24 +385,14 @@ if test x"$use_ecmult_static_precomputation" != x"no"; then
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"
Expand All @@ -419,6 +416,7 @@ if test x"$use_ecmult_static_precomputation" != x"no"; then
fi

AC_SUBST(CC_FOR_BUILD)
AC_SUBST(SECP_CFLAGS_FOR_BUILD)
AC_SUBST(CFLAGS_FOR_BUILD)
AC_SUBST(CPPFLAGS_FOR_BUILD)
AC_SUBST(LDFLAGS_FOR_BUILD)
Expand Down Expand Up @@ -490,6 +488,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,12 +531,14 @@ fi
echo
echo " valgrind = $enable_valgrind"
echo " CC = $CC"
echo " SECP_CFLAGS = $SECP_CFLAGS"
echo " CFLAGS = $CFLAGS"
echo " CPPFLAGS = $CPPFLAGS"
echo " LDFLAGS = $LDFLAGS"
echo
if test x"$set_precomp" = x"yes"; then
echo " CC_FOR_BUILD = $CC_FOR_BUILD"
echo " SECP_CFLAGS_FOR_BUILD = $SECP_CFLAGS_FOR_BUILD"
echo " CFLAGS_FOR_BUILD = $CFLAGS_FOR_BUILD"
echo " CPPFLAGS_FOR_BUILD = $CPPFLAGS_FOR_BUILD"
echo " LDFLAGS_FOR_BUILD = $LDFLAGS_FOR_BUILD"
Expand Down

0 comments on commit 0725626

Please sign in to comment.