Re: [PATCH v4] tools: work around collision of -O0 and -D_FORTIFY_SOURCE

[Don Slutz <dslutz@verizon.com>]

On 02/06/15 06:04, Ian Jackson wrote:
> Some systems have python-config include -D_FORTIFY_SOURCE in the
> CFLAGS.  But -D_FORTIFY_SOURCE does not (currently) work with -O0, and
> -O0 is enabled in debug builds (since 1166ecf781).  As a result, on
> those systems, debug builds fail.
> 
> Work around this problem as follows:
>  * In configure, detect -D_FORTIFY_SOURCE in $(python-config --cflags)
>  * If detected, set the new autoconf substitution and make variable
>    PY_NOOPT_CFLAGS to -O1.
>  * In tools/Rules.mk, where we add -O0, also add PY_NOOPT_CFLAGS
>    (which will override the -O0 with -O1 if required).
> 
> Overriding the -O0 is better than disabling Fortify because the
> latter might have an adverse security impact.  A user who wants to
> disable optimisation completely even for Python and also disable
> Fortify can set the environment variable
>     EXTRA_CFLAGS_XEN_TOOLS='-U_FORTIFY_SOURCE -O0'
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> Reported-by: Jan Beulich <JBeulich@suse.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Euan Harris <euan.harris@citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> CC: Don Slutz <dslutz@verizon.com>
> 
> ---

Works for me so:

Tested-by: Don Slutz <dslutz@verizon.com>

   -Don Slutz


> v4: [Ian Jackson] Spot -Wp,-D_FORTIFY_SOURCE= too
> v3: [Jan Beulich] Limit no-optimization override to Python interface code.
> v2: [Ian Jackson] Use autoconf
> v1: [Jan Beulich] Initial proposal
> ---
>  config/Tools.mk.in         |    1 +
>  m4/python_fortify_noopt.m4 |   31 +++++++++++++++++++++++++++++++
>  tools/Rules.mk             |    2 ++
>  tools/configure            |   39 +++++++++++++++++++++++++++++++++++++++
>  tools/configure.ac         |    2 ++
>  tools/pygrub/Makefile      |    6 ++++--
>  tools/python/Makefile      |    6 ++++--
>  7 files changed, 83 insertions(+), 4 deletions(-)
>  create mode 100644 m4/python_fortify_noopt.m4
> 
> diff --git a/config/Tools.mk.in b/config/Tools.mk.in
> index 30267fa..e7da99d 100644
> --- a/config/Tools.mk.in
> +++ b/config/Tools.mk.in
> @@ -13,6 +13,7 @@ BISON               := @BISON@
>  FLEX                := @FLEX@
>  PYTHON              := @PYTHON@
>  PYTHON_PATH         := @PYTHONPATH@
> +PY_NOOPT_CFLAGS     := @PY_NOOPT_CFLAGS@
>  PERL                := @PERL@
>  CURL_CONFIG         := @CURL@
>  XML2_CONFIG         := @XML@
> diff --git a/m4/python_fortify_noopt.m4 b/m4/python_fortify_noopt.m4
> new file mode 100644
> index 0000000..f9cb52b
> --- /dev/null
> +++ b/m4/python_fortify_noopt.m4
> @@ -0,0 +1,31 @@
> +dnl Defines PY_NOOPT_CFLAGS to either '' or -O1
> +dnl
> +
> +dnl This is necessary because on some systems setup.py includes
> +dnl -D_FORTIFY_SOURCE but have a -D_FORTIFY_SOURCE which breaks
> +dnl with -O0.  On those systems we arrange to use -O1 for debug
> +dnl builds instead.
> +
> +AC_DEFUN([AX_CHECK_PYTHON_FORTIFY_NOOPT], [
> +    AC_CACHE_CHECK([whether Python setup.py brokenly enables -D_FORTIFY_SOURCE],
> +                   [ax_cv_python_fortify],[
> +        ax_cv_python_fortify=no
> +        for arg in $($PYTHON-config --cflags); do
> +            case "$arg" in
> +            -D_FORTIFY_SOURCE=0) ax_cv_python_fortify=no ;;
> +            -D_FORTIFY_SOURCE=*) ax_cv_python_fortify=yes ;;
> +            -Wp,-D_FORTIFY_SOURCE=0) ax_cv_python_fortify=no ;;
> +            -Wp,-D_FORTIFY_SOURCE=*) ax_cv_python_fortify=yes ;;
> +            *) ;;
> +            esac
> +        done
> +    ])
> +
> +    AS_IF([test x$ax_cv_python_fortify = xyes],[
> +        PY_NOOPT_CFLAGS=-O1
> +    ], [
> +        PY_NOOPT_CFLAGS=''
> +    ])
> +
> +    AC_SUBST(PY_NOOPT_CFLAGS)
> +])
> diff --git a/tools/Rules.mk b/tools/Rules.mk
> index 74cf37e..3c29d07 100644
> --- a/tools/Rules.mk
> +++ b/tools/Rules.mk
> @@ -57,6 +57,8 @@ SHLIB_libxenvchan  = -Wl,-rpath-link=$(XEN_LIBVCHAN)
>  ifeq ($(debug),y)
>  # Disable optimizations and enable debugging information for macros
>  CFLAGS += -O0 -g3
> +# But allow an override to -O0 in case Python enforces -D_FORTIFY_SOURCE=<n>.
> +PY_CFLAGS += $(PY_NOOPT_CFLAGS)
>  endif
>  
>  LIBXL_BLKTAP ?= $(CONFIG_BLKTAP2)
> diff --git a/tools/configure b/tools/configure
> index ab04e8c..e7dac75 100755
> --- a/tools/configure
> +++ b/tools/configure
> @@ -652,6 +652,7 @@ PKG_CONFIG_LIBDIR
>  PKG_CONFIG_PATH
>  PKG_CONFIG
>  CURSES_LIBS
> +PY_NOOPT_CFLAGS
>  EGREP
>  GREP
>  CPP
> @@ -3453,6 +3454,10 @@ esac
>  
>  
>  
> +
> +
> +
> +
>  # pkg.m4 - Macros to locate and utilise pkg-config.            -*- Autoconf -*-
>  # serial 1 (pkg-config-0.24)
>  #
> @@ -7043,6 +7048,40 @@ CPPFLAGS=$ac_previous_cppflags
>  LDLFAGS=$ac_previous_ldflags
>  
>  
> +    { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether Python setup.py brokenly enables -D_FORTIFY_SOURCE" >&5
> +$as_echo_n "checking whether Python setup.py brokenly enables -D_FORTIFY_SOURCE... " >&6; }
> +if ${ax_cv_python_fortify+:} false; then :
> +  $as_echo_n "(cached) " >&6
> +else
> +
> +        ax_cv_python_fortify=no
> +        for arg in $($PYTHON-config --cflags); do
> +            case "$arg" in
> +            -D_FORTIFY_SOURCE=0) ax_cv_python_fortify=no ;;
> +            -D_FORTIFY_SOURCE=*) ax_cv_python_fortify=yes ;;
> +            -Wp,-D_FORTIFY_SOURCE=0) ax_cv_python_fortify=no ;;
> +            -Wp,-D_FORTIFY_SOURCE=*) ax_cv_python_fortify=yes ;;
> +            *) ;;
> +            esac
> +        done
> +
> +fi
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ax_cv_python_fortify" >&5
> +$as_echo "$ax_cv_python_fortify" >&6; }
> +
> +    if test x$ax_cv_python_fortify = xyes; then :
> +
> +        PY_NOOPT_CFLAGS=-O1
> +
> +else
> +
> +        PY_NOOPT_CFLAGS=''
> +
> +fi
> +
> +
> +
> +
>  fi
>  
>  if ! $rump; then
> diff --git a/tools/configure.ac b/tools/configure.ac
> index d9cbf1f..03dadd7 100644
> --- a/tools/configure.ac
> +++ b/tools/configure.ac
> @@ -58,6 +58,7 @@ m4_include([../m4/checkpolicy.m4])
>  m4_include([../m4/set_cflags_ldflags.m4])
>  m4_include([../m4/python_version.m4])
>  m4_include([../m4/python_devel.m4])
> +m4_include([../m4/python_fortify_noopt.m4])
>  m4_include([../m4/ocaml.m4])
>  m4_include([../m4/uuid.m4])
>  m4_include([../m4/pkg.m4])
> @@ -295,6 +296,7 @@ AX_CHECK_PYTHON_VERSION([2], [3])
>  
>  AS_IF([test "$cross_compiling" != yes], [
>      AX_CHECK_PYTHON_DEVEL()
> +    AX_CHECK_PYTHON_FORTIFY_NOOPT()
>  ])
>  
>  if ! $rump; then
> diff --git a/tools/pygrub/Makefile b/tools/pygrub/Makefile
> index 6fd194c..00e654a 100644
> --- a/tools/pygrub/Makefile
> +++ b/tools/pygrub/Makefile
> @@ -2,15 +2,17 @@
>  XEN_ROOT = $(CURDIR)/../..
>  include $(XEN_ROOT)/tools/Rules.mk
>  
> +PY_CFLAGS = $(CFLAGS) $(PY_NOOPT_CFLAGS) $(APPEND_LDFLAGS)
> +
>  .PHONY: all
>  all: build
>  .PHONY: build
>  build:
> -	CC="$(CC)" CFLAGS="$(CFLAGS) $(APPEND_LDFLAGS)" $(PYTHON) setup.py build
> +	CC="$(CC)" CFLAGS="$(PY_CFLAGS)" $(PYTHON) setup.py build
>  
>  .PHONY: install
>  install: all
> -	CC="$(CC)" CFLAGS="$(CFLAGS) $(APPEND_LDFLAGS)" $(PYTHON) setup.py install \
> +	CC="$(CC)" CFLAGS="$(PY_CFLAGS)" $(PYTHON) setup.py install \
>  		$(PYTHON_PREFIX_ARG) --root="$(DESTDIR)" \
>  		--install-scripts=$(LIBEXEC_BIN) --force
>  	set -e; if [ $(BINDIR) != $(LIBEXEC_BIN) -a \
> diff --git a/tools/python/Makefile b/tools/python/Makefile
> index af95119..e933be8 100644
> --- a/tools/python/Makefile
> +++ b/tools/python/Makefile
> @@ -4,6 +4,8 @@ include $(XEN_ROOT)/tools/Rules.mk
>  .PHONY: all
>  all: build
>  
> +PY_CFLAGS = $(CFLAGS) $(PY_NOOPT_CFLAGS) $(LDFLAGS) $(APPEND_LDFLAGS)
> +
>  .PHONY: build
>  build: genwrap.py $(XEN_ROOT)/tools/libxl/libxl_types.idl \
>  		$(XEN_ROOT)/tools/libxl/idl.py
> @@ -11,11 +13,11 @@ build: genwrap.py $(XEN_ROOT)/tools/libxl/libxl_types.idl \
>  		$(XEN_ROOT)/tools/libxl/libxl_types.idl \
>  		xen/lowlevel/xl/_pyxl_types.h \
>  		xen/lowlevel/xl/_pyxl_types.c
> -	CC="$(CC)" CFLAGS="$(CFLAGS) $(LDFLAGS) $(APPEND_LDFLAGS)" $(PYTHON) setup.py build
> +	CC="$(CC)" CFLAGS="$(PY_CFLAGS)" $(PYTHON) setup.py build
>  
>  .PHONY: install
>  install:
> -	CC="$(CC)" CFLAGS="$(CFLAGS) $(LDFLAGS) $(APPEND_LDFLAGS)" $(PYTHON) setup.py install \
> +	CC="$(CC)" CFLAGS="$(PY_CFLAGS)" $(PYTHON) setup.py install \
>  		$(PYTHON_PREFIX_ARG) --root="$(DESTDIR)" --force
>  
>  .PHONY: test
>