From f38d96dd61c0a56ffb7367311d7e7db5e9b0a681 Mon Sep 17 00:00:00 2001 From: Florian Krohm Date: Wed, 3 Dec 2014 22:53:00 +0000 Subject: [PATCH] Add -Wformat -Wformat-security to the list of compile flags. This was not as straight forward as expected. Specifically, adding the new flag to CFLAGS in configure.ac did not work and was causing compiler warnings. For instance, compiling memcheck/tests/execve2.c will generate a -Wnonnull warning even though the testcase is explicitly compiled with -Wno-nonnull. The reason is that (a) -Wformat is implied by -Wnonnull and (b) the list of compiler flags gets assembled in the wrong order. The culprit appears to be that we modify CFLAGS in configure.ac and that really is not the right place. Conceptually, configure should determine tool-chain capabilities and not assemble compiler flags. That should be done in Makefiles. This patch entangles all this. So, whatever was added to CFLAGS in configure.ac has now been moved to Makefile.all.am and Makefile.tool-tests.am. Those are: -Wno-long-long -Wwrite-strings -Wcast-qual -fno-stack-protector Note, that this change allows us to simplify Makefile.tool-tests.am which in the past was disabling some of those flags (e.g. by adding -Wno-cast-qual again). In case of the clang compiler, extra command line options are needed. I've moved those into a separate 'if COMPILER_IS_CLANG' section and not merge them into baseline flags. Related to BZ 334727. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@14798 --- Makefile.all.am | 12 ++++++- Makefile.tool-tests.am | 26 +++++++------- configure.ac | 52 +++++++++++++--------------- memcheck/tests/vbit-test/Makefile.am | 6 ++-- 4 files changed, 51 insertions(+), 45 deletions(-) diff --git a/Makefile.all.am b/Makefile.all.am index 6257a20b70..44b024af51 100644 --- a/Makefile.all.am +++ b/Makefile.all.am @@ -105,10 +105,20 @@ AM_CFLAGS_BASE = \ -Wpointer-arith \ -Wstrict-prototypes \ -Wmissing-declarations \ - @FLAG_W_NO_TAUTOLOGICAL_COMPARE@ \ + -Wno-long-long \ + @FLAG_W_CAST_QUAL@ \ + @FLAG_W_WRITE_STRINGS@ \ + @FLAG_W_FORMAT@ \ + @FLAG_W_FORMAT_SECURITY@ \ + @FLAG_FNO_STACK_PROTECTOR@ \ -fno-strict-aliasing \ -fno-builtin +if COMPILER_IS_CLANG +AM_CFLAGS_BASE += -Wno-cast-align -Wno-self-assign \ + -Wno-tautological-compare +endif + # These flags are used for building the preload shared objects (PSOs). # The aim is to give reasonable performance but also to have good # stack traces, since users often see stack traces extending diff --git a/Makefile.tool-tests.am b/Makefile.tool-tests.am index 4ca6f7cc53..ca9d9a6603 100644 --- a/Makefile.tool-tests.am +++ b/Makefile.tool-tests.am @@ -18,8 +18,10 @@ endif # Nb: Tools need to augment these flags with an arch-selection option, such # as $(AM_FLAG_M3264_PRI). -AM_CFLAGS = -Winline -Wall -Wshadow -g -AM_CXXFLAGS = -Winline -Wall -Wshadow -g +AM_CFLAGS = -Winline -Wall -Wshadow -Wno-long-long -g \ + @FLAG_FNO_STACK_PROTECTOR@ +AM_CXXFLAGS = -Winline -Wall -Wshadow -Wno-long-long -g \ + @FLAG_FNO_STACK_PROTECTOR@ # Include AM_CPPFLAGS in AM_CCASFLAGS to allow for older versions of # automake; see comments in Makefile.all.am for more detail. AM_CCASFLAGS = $(AM_CPPFLAGS) @@ -28,21 +30,17 @@ if VGCONF_OS_IS_DARWIN noinst_DSYMS = $(check_PROGRAMS) endif -if HAS_WRITE_STRINGS_WARNING -CFLAGS += -Wno-write-strings -endif - if COMPILER_IS_CLANG -CFLAGS += -Wno-format-extra-args # perf/tinycc.c -CFLAGS += -Wno-literal-range # none/tests/amd64/fxtract.c -CFLAGS += -Wno-string-plus-int # drd/tests/annotate_ignore_rw.c -CXXFLAGS += -Wno-unused-private-field # drd/tests/tsan_unittest.cpp +AM_CFLAGS += -Wno-format-extra-args # perf/tinycc.c +AM_CFLAGS += -Wno-literal-range # none/tests/amd64/fxtract.c +AM_CFLAGS += -Wno-tautological-constant-out-of-range-compare # ...../aes.c +AM_CFLAGS += -Wno-self-assign # memcheck/tests/unit_libcbase.c +AM_CFLAGS += -Wno-string-plus-int # drd/tests/annotate_ignore_rw.c +AM_CFLAGS += -Wno-uninitialized # clang 3.4.2 and earlier +AM_CFLAGS += -Wno-unused-value # clang 3.0.0 +AM_CXXFLAGS += -Wno-unused-private-field # drd/tests/tsan_unittest.cpp endif -# Compile testcases without -Wcast-qual -CFLAGS += -Wno-cast-qual -CXXFLAGS += -Wno-cast-qual - check-local: build-noinst_DSYMS clean-local: clean-noinst_DSYMS diff --git a/configure.ac b/configure.ac index 69c28d5681..11986775dc 100644 --- a/configure.ac +++ b/configure.ac @@ -15,11 +15,16 @@ AM_INIT_AUTOMAKE([foreign subdir-objects]) AM_MAINTAINER_MODE +#---------------------------------------------------------------------------- +# Do NOT modify these flags here. Except in feature tests in which case +# the original values must be properly restored. +#---------------------------------------------------------------------------- +CFLAGS="$CFLAGS" +CXXFLAGS="$CXXFLAGS" + #---------------------------------------------------------------------------- # Checks for various programs. #---------------------------------------------------------------------------- -CFLAGS="-Wno-long-long $CFLAGS" -CXXFLAGS="-Wno-long-long $CXXFLAGS" AC_PROG_LN_S AC_PROG_CC @@ -145,11 +150,6 @@ fi AM_CONDITIONAL(COMPILER_IS_CLANG, test $is_clang = clang -o $is_clang = applellvm) AM_CONDITIONAL(COMPILER_IS_ICC, test $is_clang = icc) -if test $is_clang = clang -o $is_clang = applellvm ; then - CFLAGS="$CFLAGS -Wno-tautological-compare -Wno-cast-align -Wno-self-assign" - CXXFLAGS="$CXXFLAGS -Wno-tautological-compare -Wno-cast-align -Wno-self-assign" -fi - # Note: m4 arguments are quoted with [ and ] so square brackets in shell # statements have to be quoted. case "${is_clang}-${gcc_version}" in @@ -1686,11 +1686,6 @@ AC_DEFUN([AC_GCC_WARNING_COND],[ ]) AC_GCC_WARNING_COND([pointer-sign], [HAS_POINTER_SIGN_WARNING]) -AC_GCC_WARNING_COND([write-strings], [HAS_WRITE_STRINGS_WARNING]) -if test $has_warning_flag = yes; then - CFLAGS="$CFLAGS -Wwrite-strings" - CXXFLAGS="$CXXFLAGS -Wwrite-strings" -fi # Convenience function to check whether GCC supports a particular # warning option. Similar to AC_GCC_WARNING_COND, but does a @@ -1712,14 +1707,31 @@ AC_DEFUN([AC_GCC_WARNING_SUBST_NO],[ CFLAGS=$safe_CFLAGS ]) +# Convenience function. Like AC_GCC_WARNING_SUBST_NO, except it substitutes +# -W$1 (instead of -Wno-$1). +AC_DEFUN([AC_GCC_WARNING_SUBST],[ + AC_MSG_CHECKING([if gcc accepts -W$1]) + safe_CFLAGS=$CFLAGS + CFLAGS="-W$1" + AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]], [[;]])], [ + AC_SUBST([$2], [-W$1]) + AC_MSG_RESULT([yes])], [ + AC_SUBST([$2], []) + AC_MSG_RESULT([no])]) + CFLAGS=$safe_CFLAGS +]) + AC_GCC_WARNING_SUBST_NO([empty-body], [FLAG_W_NO_EMPTY_BODY]) AC_GCC_WARNING_SUBST_NO([format-zero-length], [FLAG_W_NO_FORMAT_ZERO_LENGTH]) -AC_GCC_WARNING_SUBST_NO([tautological-compare], [FLAG_W_NO_TAUTOLOGICAL_COMPARE]) AC_GCC_WARNING_SUBST_NO([nonnull], [FLAG_W_NO_NONNULL]) AC_GCC_WARNING_SUBST_NO([overflow], [FLAG_W_NO_OVERFLOW]) AC_GCC_WARNING_SUBST_NO([uninitialized], [FLAG_W_NO_UNINITIALIZED]) AC_GCC_WARNING_SUBST_NO([unused-function], [FLAG_W_NO_UNUSED_FUNCTION]) AC_GCC_WARNING_SUBST_NO([static-local-in-inline], [FLAG_W_NO_STATIC_LOCAL_IN_INLINE]) +AC_GCC_WARNING_SUBST([write-strings], [FLAG_W_WRITE_STRINGS]) +AC_GCC_WARNING_SUBST([format], [FLAG_W_FORMAT]) +AC_GCC_WARNING_SUBST([format-security], [FLAG_W_FORMAT_SECURITY]) +AC_GCC_WARNING_SUBST([cast-qual], [FLAG_W_CAST_QUAL]) # does this compiler support -Wextra or the older -W ? @@ -1748,14 +1760,6 @@ AC_MSG_RESULT([-Wextra]) ]) CFLAGS=$safe_CFLAGS -# does this compiler support -Wcast-qual ? -AC_GCC_WARNING_COND([cast-qual], [HAS_CAST_QUAL_WARNING]) -if test $has_warning_flag = yes; then - CFLAGS="$CFLAGS -Wcast-qual" - CXXFLAGS="$CXXFLAGS -Wcast-qual" -fi - - # does this compiler support -fno-stack-protector ? AC_MSG_CHECKING([if gcc accepts -fno-stack-protector]) @@ -1777,12 +1781,6 @@ CFLAGS=$safe_CFLAGS AC_SUBST(FLAG_FNO_STACK_PROTECTOR) -if test x$no_stack_protector = xyes; then - CFLAGS="$CFLAGS -fno-stack-protector" - CXXFLAGS="$CXXFLAGS -fno-stack-protector" -fi - - # does this compiler support --param inline-unit-growth=... ? AC_MSG_CHECKING([if gcc accepts --param inline-unit-growth]) diff --git a/memcheck/tests/vbit-test/Makefile.am b/memcheck/tests/vbit-test/Makefile.am index 6ae57b05c7..7dc14a4f9e 100644 --- a/memcheck/tests/vbit-test/Makefile.am +++ b/memcheck/tests/vbit-test/Makefile.am @@ -33,13 +33,13 @@ SOURCES = \ valgrind.c vbit_test_SOURCES = $(SOURCES) -vbit_test_CPPFLAGS = $(AM_CPPFLAGS) \ +vbit_test_CPPFLAGS = $(AM_CPPFLAGS_PRI) \ -I$(top_srcdir)/include \ -I$(top_srcdir)/memcheck \ -I$(top_srcdir)/VEX/pub -vbit_test_CFLAGS = $(AM_CFLAGS) -std=c99 +vbit_test_CFLAGS = $(AM_CFLAGS_PRI) -std=c99 vbit_test_DEPENDENCIES = vbit_test_LDADD = -vbit_test_LDFLAGS = +vbit_test_LDFLAGS = $(AM_CFLAGS_PRI) -std=c99 vbit_test_LINK = $(LINK) $(LINK) $(vbit_test_CFLAGS) $(vbit_test_LDFLAGS) -- 2.47.2