Re: [PATCH] Verify warning flags for CC_FOR_BUILD compiler

[Vlad Zakharov <Vladislav dot Zakharov at synopsys dot com>]

Hi Nick, all, 

Please treat this message as a polite reminder to review the patch.

It fixes a common issue discussed earlier in binuilts mailing list:
https://sourceware.org/ml/binutils/2016-08/msg00117.html
and prevents build failures of binutils.

Thanks.

On Wed, 2016-09-07 at 18:18 +0300, Vlad Zakharov wrote:
> Current commit introduces ac_cpp_for_build,
> GCC_WARN_CFLAGS_FOR_BUILD, WARN_CFLAGS_FOR_BUILD and
> AM_CFLAGS_FOR_BUILD variables and defines
> AC_EGREP_CPP_FOR_BUILD macro.
> 
> This macro is used to verify compiler for build (CC_FOR_BUILD)
> to be compatible with different flags and options, i. e.
> verify if CC_FOR_BUILD compiler is suitable to use
> WARN_CFLAGS_FOR_BUILD flags.
> 
> When cross-compiling we have different compiler CC and
> CC_FOR_BUILD. But earlier AM_CFLAGS were used with both CC and
> CC_FOR_BUILD compiler.
> 
> AM_CFLAGS are set up as WARN_CFLAGS + ZLIBINC.
> In it's turn WARN_CFLAGS come up from GCC_WARN_CFLAGS.
> 
> Configure script verified GCC_WARN_CFLAGS to be compatible only
> with CC compiler even though in fact these flags were also
> passed to CC_FOR_BUILD. Such logic leads to some errors, i. e.
> when we have old CC_FOR_BUILD compiler that is not suitable to
> use some of GCC_WARN_CFLAGS.
> 
> To fix it corresponding variables and macro were added.
> AM_CFLAGS_FOR_BUILD are passed now to CC_FOR_BUILD compiler
> instead of AM_CFLAGS.
> 
> Signed-off-by: Vlad Zakharov <vzakhar@synopsys.com>
> ---
>  bfd/ChangeLog        |  7 +++++++
>  bfd/warning.m4       | 49 ++++++++++++++++++++++++++++++++++++++++++++-----
>  binutils/ChangeLog   |  5 +++++
>  binutils/Makefile.am | 10 ++++++----
>  4 files changed, 62 insertions(+), 9 deletions(-)
> 
> diff --git a/bfd/ChangeLog b/bfd/ChangeLog
> index 6863e3a..74fa60a 100644
> --- a/bfd/ChangeLog
> +++ b/bfd/ChangeLog
> @@ -1,3 +1,10 @@
> +2016-09-07  Vlad Zakharov  <vzakhar@synopsys.com>
> +
> +	* configure.ac (AC_EGREP_CPP_FOR_BUILD): Introduce macro 
> +	to verify CC_FOR_BUILD compiler.
> +	(AM_BINUTILS_WARNINGS):	Introduce ac_cpp_for_build variable
> +	and add CC_FOR_BUILD compiler checks. 
> +
>  2016-09-06  H.J. Lu  <hongjiu.lu@intel.com>
>  
>  	PR ld/20550
> diff --git a/bfd/warning.m4 b/bfd/warning.m4
> index 4c5b55d..3d7fdbb 100644
> --- a/bfd/warning.m4
> +++ b/bfd/warning.m4
> @@ -17,15 +17,37 @@ dnl along with this program; see the file COPYING3.  If not see
>  dnl <http://www.gnu.org/licenses/>.
>  dnl
>  
> +# AC_EGREP_CPP_FOR_BUILD(PATTERN, PROGRAM,
> +#              [ACTION-IF-FOUND], [ACTION-IF-NOT-FOUND])
> +# ------------------------------------------------------
> +AC_DEFUN([AC_EGREP_CPP_FOR_BUILD],
> +[AC_LANG_PREPROC_REQUIRE()dnl
> +AC_REQUIRE([AC_PROG_EGREP])dnl
> +AC_LANG_CONFTEST([AC_LANG_SOURCE([[$2]])])
> +AS_IF([dnl eval is necessary to expand ac_cpp.
> +dnl Ultrix and Pyramid sh refuse to redirect output of eval, so use subshell.
> +(eval "$ac_cpp_for_build conftest.$ac_ext") 2>&AS_MESSAGE_LOG_FD |
> +dnl Quote $1 to prevent m4 from eating character classes
> +  $EGREP "[$1]" >/dev/null 2>&1],
> +  [$3],
> +  [$4])
> +rm -f conftest*
> +])# AC_EGREP_CPP_FOR_BUILD
> +
> +
>  AC_DEFUN([AM_BINUTILS_WARNINGS],[
>  # Set the 'development' global.
>  . $srcdir/../bfd/development.sh
>  
> +# Set acp_cpp_for_build variable
> +ac_cpp_for_build="$CC_FOR_BUILD -E $CPPFLAGS_FOR_BUILD"
> +
>  # Default set of GCC warnings to enable.
>  GCC_WARN_CFLAGS="-W -Wall -Wstrict-prototypes -Wmissing-prototypes"
> +GCC_WARN_CFLAGS_FOR_BUILD="-W -Wall -Wstrict-prototypes -Wmissing-prototypes"
>  
>  # Add -Wshadow if the compiler is a sufficiently recent version of GCC.
> -AC_EGREP_CPP([^[0-3]$],[__GNUC__],,GCC_WARN_CFLAGS="$GCC_WARN_CFLAGS -Wshadow")
> +AC_EGREP_CPP([^[0-3]$],[__GNUC__],,GCC_asdfasdfWARN_CFLAGS="$GCC_WARN_CFLAGS -Wshadow")
>  
>  # Add -Wstack-usage if the compiler is a sufficiently recent version of GCC.
>  AC_EGREP_CPP([^[0-4]$],[__GNUC__],,GCC_WARN_CFLAGS="$GCC_WARN_CFLAGS -Wstack-usage=262144")
> @@ -34,6 +56,14 @@ AC_EGREP_CPP([^[0-4]$],[__GNUC__],,GCC_WARN_CFLAGS="$GCC_WARN_CFLAGS -Wstack-usa
>  WARN_WRITE_STRINGS=""
>  AC_EGREP_CPP([^[0-3]$],[__GNUC__],,WARN_WRITE_STRINGS="-Wwrite-strings")
>  
> +# Verify CC_FOR_BUILD to be compatible with waring flags
> +
> +# Add -Wshadow if the compiler is a sufficiently recent version of GCC.
> +AC_EGREP_CPP_FOR_BUILD([^[0-3]$],[__GNUC__],,GCC_WARN_CFLAGS_FOR_BUILD="$GCC_WARN_CFLAGS_FOR_BUILD -Wshadow")
> +
> +# Add -Wstack-usage if the compiler is a sufficiently recent version of GCC.
> +AC_EGREP_CPP_FOR_BUILD([^[0-4]$],[__GNUC__],,GCC_WARN_CFLAGS_FOR_BUILD="$GCC_WARN_CFLAGS_FOR_BUILD -Wstack-
> usage=262144")
> +
>  AC_ARG_ENABLE(werror,
>    [  --enable-werror         treat compile warnings as errors],
>    [case "${enableval}" in
> @@ -47,6 +77,7 @@ case "${host}" in
>    *-*-mingw32*)
>      if test "${GCC}" = yes -a -z "${ERROR_ON_WARNING}" ; then
>        GCC_WARN_CFLAGS="$GCC_WARN_CFLAGS -Wno-format"
> +      GCC_WARN_CFLAGS_FOR_BUILD="$GCC_WARN_CFLAGS_FOR_BUILD -Wno-format"
>      fi
>      ;;
>    *) ;;
> @@ -60,25 +91,32 @@ fi
>  NO_WERROR=
>  if test "${ERROR_ON_WARNING}" = yes ; then
>      GCC_WARN_CFLAGS="$GCC_WARN_CFLAGS -Werror"
> +    GCC_WARN_CFLAGS_FOR_BUILD="$GCC_WARN_CFLAGS_FOR_BUILD -Werror"
>      NO_WERROR="-Wno-error"
>  fi
>  
>  if test "${GCC}" = yes ; then
>    WARN_CFLAGS="${GCC_WARN_CFLAGS}"
> +  WARN_CFLAGS_FOR_BUILD="${GCC_WARN_CFLAGS_FOR_BUILD}"
>  fi
>  
>  AC_ARG_ENABLE(build-warnings,
>  [  --enable-build-warnings enable build-time compiler warnings],
>  [case "${enableval}" in
> -  yes)	WARN_CFLAGS="${GCC_WARN_CFLAGS}";;
> +  yes)	WARN_CFLAGS="${GCC_WARN_CFLAGS}" 
> +        WARN_CFLAGS_FOR_BUILD="${GCC_WARN_CFLAGS_FOR_BUILD}";;
>    no)	if test "${GCC}" = yes ; then
>  	  WARN_CFLAGS="-w"
> +      WARN_CFLAGS_FOR_BUILD="-w" 
>  	fi;;
>    ,*)   t=`echo "${enableval}" | sed -e "s/,/ /g"`
> -        WARN_CFLAGS="${GCC_WARN_CFLAGS} ${t}";;
> +        WARN_CFLAGS="${GCC_WARN_CFLAGS} ${t}"
> +        WARN_CFLAGS_FOR_BUILD="${GCC_WARN_CFLAGS_FOR_BUILD} ${t}";;
>    *,)   t=`echo "${enableval}" | sed -e "s/,/ /g"`
> -        WARN_CFLAGS="${t} ${GCC_WARN_CFLAGS}";;
> -  *)    WARN_CFLAGS=`echo "${enableval}" | sed -e "s/,/ /g"`;;
> +        WARN_CFLAGS="${t} ${GCC_WARN_CFLAGS}"
> +        WARN_CFLAGS_FOR_BUILD="${t} ${GCC_WARN_CFLAGS_FOR_BUILD}";;
> +  *)    WARN_CFLAGS=`echo "${enableval}" | sed -e "s/,/ /g"`
> +        WARN_CFLAGS_FOR_BUILD=`echo "${enableval}" | sed -e "s/,/ /g"`;;
>  esac])
>  
>  if test x"$silent" != x"yes" && test x"$WARN_CFLAGS" != x""; then
> @@ -86,6 +124,7 @@ if test x"$silent" != x"yes" && test x"$WARN_CFLAGS" != x""; then
>  fi
>  
>  AC_SUBST(WARN_CFLAGS)
> +AC_SUBST(WARN_CFLAGS_FOR_BUILD)
>  AC_SUBST(NO_WERROR)
>       AC_SUBST(WARN_WRITE_STRINGS)
>  ])
> diff --git a/binutils/ChangeLog b/binutils/ChangeLog
> index 4b3a746..2884f18 100644
> --- a/binutils/ChangeLog
> +++ b/binutils/ChangeLog
> @@ -1,3 +1,8 @@
> +2016-09-07  Vlad Zakharov  <vzakhar@synopsys.com>
> +
> +	* Makefile.am: Replace AM_CLFAGS with AM_CFLAGS_FOR_BUILD
> +	when building with CC_FOR_BUILD compiler.
> +	
>  2016-09-06  Nick Clifton  <nickc@redhat.com>
>  
>  	* readelf.c (request_dump_bynumber): Only call memcpy if
> diff --git a/binutils/Makefile.am b/binutils/Makefile.am
> index 18af2c8..ef17914 100644
> --- a/binutils/Makefile.am
> +++ b/binutils/Makefile.am
> @@ -47,8 +47,10 @@ ZLIB = @zlibdir@ -lz
>  ZLIBINC = @zlibinc@
>  
>  WARN_CFLAGS = @WARN_CFLAGS@
> +WARN_CFLAGS_FOR_BUILD = @WARN_CFLAGS_FOR_BUILD@
>  NO_WERROR = @NO_WERROR@
>  AM_CFLAGS = $(WARN_CFLAGS) $(ZLIBINC)
> +AM_CFLAGS_FOR_BUILD = $(WARN_CFLAGS_FOR_BUILD) $(ZLIBINC)
>  LIBICONV = @LIBICONV@
>  
>  # these two are almost the same program
> @@ -305,17 +307,17 @@ sysinfo$(EXEEXT_FOR_BUILD): sysinfo.@OBJEXT@ syslex_wrap.@OBJEXT@
>  	$(CC_FOR_BUILD) $(CFLAGS_FOR_BUILD) $(LDFLAGS_FOR_BUILD) -o $@ sysinfo.@OBJEXT@ syslex_wrap.@OBJEXT@
>  
>  syslex_wrap.@OBJEXT@: syslex_wrap.c syslex.c sysinfo.h config.h
> -	$(CC_FOR_BUILD) -c -I. -I$(srcdir) $(AM_CFLAGS) $(CFLAGS_FOR_BUILD) $(NO_WERROR) $(srcdir)/syslex_wrap.c
> +	$(CC_FOR_BUILD) -c -I. -I$(srcdir) $(AM_CFLAGS_FOR_BUILD) $(CFLAGS_FOR_BUILD) $(NO_WERROR)
> $(srcdir)/syslex_wrap.c
>  
>  sysinfo.@OBJEXT@: sysinfo.c
>  	if [ -r sysinfo.c ]; then \
> -	  $(CC_FOR_BUILD) -c -I. $(AM_CFLAGS) $(CFLAGS_FOR_BUILD) $(NO_WERROR) sysinfo.c ; \
> +	  $(CC_FOR_BUILD) -c -I. $(AM_CFLAGS_FOR_BUILD) $(CFLAGS_FOR_BUILD) $(NO_WERROR) sysinfo.c ; \
>  	else \
> -	  $(CC_FOR_BUILD) -c -I. $(AM_CFLAGS) $(CFLAGS_FOR_BUILD) $(NO_WERROR) $(srcdir)/sysinfo.c ; \
> +	  $(CC_FOR_BUILD) -c -I. $(AM_CFLAGS_FOR_BUILD) $(CFLAGS_FOR_BUILD) $(NO_WERROR) $(srcdir)/sysinfo.c ; \
>  	fi
>  
>  bin2c$(EXEEXT_FOR_BUILD): bin2c.c
> -	$(CC_FOR_BUILD) -o $@ $(AM_CPPFLAGS) $(AM_CFLAGS) $(CFLAGS_FOR_BUILD) $(LDFLAGS_FOR_BUILD) $(srcdir)/bin2c.c
> +	$(CC_FOR_BUILD) -o $@ $(AM_CPPFLAGS) $(AM_CFLAGS_FOR_BUILD) $(CFLAGS_FOR_BUILD) $(LDFLAGS_FOR_BUILD)
> $(srcdir)/bin2c.c
>  
>  embedspu: embedspu.sh Makefile
>  	awk '/^program_transform_name=/ {print "program_transform_name=\"$(program_transform_name)\""; next} {print}'
> < $< > $@
-- 
Best regards,
Vlad Zakharov <vzakhar@synopsys.com>