[PATCH] libxl/Makefile: Don't optimize debug builds; add macro debugging information

[PATCH] libxl/Makefile: Don't optimize debug builds; add macro debugging information

[Euan Harris]

libxl debug builds are built with optimization level -O1, inherited from
the CFLAGS definition in StdGNU.mk.   Optimizations confuse the debugger,
and the comment justifying -O1 in StdGNU.mk should not apply for a
userspace library.   Disable optimization by appending -O0 to CFLAGS,
which overrides the -O1 flag specified earlier.

Also specify -g3, to add macro debugging information which allows
gdb to expand macro invocations.   This is useful as libxl uses many
non-trivial macros.

Signed-off-by: Euan Harris <euan.harris@citrix.com>
---
 tools/libxl/Makefile |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index df08c8a..26d8679 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -15,6 +15,12 @@ CFLAGS += -Werror -Wno-format-zero-length -Wmissing-declarations \
 	-Wno-declaration-after-statement -Wformat-nonliteral
 CFLAGS += -I. -fPIC
 
+ifeq ($(debug),y)
+# Disable optimizations and debugging information for macros
+CFLAGS += -O0 -g3
+endif
+
+
 ifeq ($(CONFIG_Linux),y)
 LIBUUID_LIBS += -luuid
 endif
-- 
1.7.1

Re: [PATCH] libxl/Makefile: Don't optimize debug builds; add macro debugging information

[Ian Campbell]

On Mon, 2014-12-01 at 10:39 +0000, Euan Harris wrote:
> libxl debug builds are built with optimization level -O1, inherited from
> the CFLAGS definition in StdGNU.mk.   Optimizations confuse the debugger,
> and the comment justifying -O1 in StdGNU.mk should not apply for a
> userspace library.   Disable optimization by appending -O0 to CFLAGS,
> which overrides the -O1 flag specified earlier.

I think if this argument applies (I see no reason to disagree) then it
should apply to the whole of tools/* or at least to tools/lib* and not
just to libxl. IOW this probably belongs at a higher level somewhere.

> Also specify -g3, to add macro debugging information which allows
> gdb to expand macro invocations.   This is useful as libxl uses many
> non-trivial macros.

Useful, I'd never heard of this. Do you know which version of gcc
introduced it? (AKA do we need to make it part of configure.ac to check
availability or not).

Not sure if you were proposing this change for 4.5 or not.

> 
> Signed-off-by: Euan Harris <euan.harris@citrix.com>
> ---
>  tools/libxl/Makefile |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> index df08c8a..26d8679 100644
> --- a/tools/libxl/Makefile
> +++ b/tools/libxl/Makefile
> @@ -15,6 +15,12 @@ CFLAGS += -Werror -Wno-format-zero-length -Wmissing-declarations \
>  	-Wno-declaration-after-statement -Wformat-nonliteral
>  CFLAGS += -I. -fPIC
>  
> +ifeq ($(debug),y)
> +# Disable optimizations and debugging information for macros
> +CFLAGS += -O0 -g3
> +endif
> +
> +
>  ifeq ($(CONFIG_Linux),y)
>  LIBUUID_LIBS += -luuid
>  endif

Re: [PATCH] libxl/Makefile: Don't optimize debug builds; add macro debugging information

[Euan Harris]

On Mon, Dec 01, 2014 at 11:43:13AM +0000, Ian Campbell wrote:
> On Mon, 2014-12-01 at 10:39 +0000, Euan Harris wrote:
> > libxl debug builds are built with optimization level -O1, inherited from
> > the CFLAGS definition in StdGNU.mk.   Optimizations confuse the debugger,
> > and the comment justifying -O1 in StdGNU.mk should not apply for a
> > userspace library.   Disable optimization by appending -O0 to CFLAGS,
> > which overrides the -O1 flag specified earlier.
> 
> I think if this argument applies (I see no reason to disagree) then it
> should apply to the whole of tools/* or at least to tools/lib* and not
> just to libxl. IOW this probably belongs at a higher level somewhere.

Ok, I'll submit a new patch putting it in tools/Rules.mk

> > Also specify -g3, to add macro debugging information which allows
> > gdb to expand macro invocations.   This is useful as libxl uses many
> > non-trivial macros.
> 
> Useful, I'd never heard of this. Do you know which version of gcc
> introduced it? (AKA do we need to make it part of configure.ac to check
> availability or not).

It's documented in GCC 2.95.3 [1], which is as far back as the online
manuals go.

[1] https://gcc.gnu.org/onlinedocs/gcc-2.95.3/gcc_2.html#SEC9

> Not sure if you were proposing this change for 4.5 or not.

It would be nice to have, but I was assuming that 4.5 was more or less
closed by now.

Thanks,
Euan

Re: [PATCH] libxl/Makefile: Don't optimize debug builds; add macro debugging information

[Ian Campbell]

On Mon, 2014-12-01 at 11:55 +0000, Euan Harris wrote:
> On Mon, Dec 01, 2014 at 11:43:13AM +0000, Ian Campbell wrote:
> > On Mon, 2014-12-01 at 10:39 +0000, Euan Harris wrote:
> > > libxl debug builds are built with optimization level -O1, inherited from
> > > the CFLAGS definition in StdGNU.mk.   Optimizations confuse the debugger,
> > > and the comment justifying -O1 in StdGNU.mk should not apply for a
> > > userspace library.   Disable optimization by appending -O0 to CFLAGS,
> > > which overrides the -O1 flag specified earlier.
> > 
> > I think if this argument applies (I see no reason to disagree) then it
> > should apply to the whole of tools/* or at least to tools/lib* and not
> > just to libxl. IOW this probably belongs at a higher level somewhere.
> 
> Ok, I'll submit a new patch putting it in tools/Rules.mk

Thanks.

> > > Also specify -g3, to add macro debugging information which allows
> > > gdb to expand macro invocations.   This is useful as libxl uses many
> > > non-trivial macros.
> > 
> > Useful, I'd never heard of this. Do you know which version of gcc
> > introduced it? (AKA do we need to make it part of configure.ac to check
> > availability or not).
> 
> It's documented in GCC 2.95.3 [1], which is as far back as the online
> manuals go.
> 
> [1] https://gcc.gnu.org/onlinedocs/gcc-2.95.3/gcc_2.html#SEC9

OK, I'm pretty sure that's (way....) before our cut-off. Thanks.

> > Not sure if you were proposing this change for 4.5 or not.
> 
> It would be nice to have, but I was assuming that 4.5 was more or less
> closed by now.

Exceptions can be asked/argued for. I'd be a bit wary of something like
this since the knock-on effects might be a bit subtle. Completely fine
during a dev window though.

Ian.

[PATCH v2] tools/Rules.mk: Don't optimize debug builds; add macro debugging information

[Euan Harris]

Tools debug builds are built with optimization level -O1, inherited from
the CFLAGS definition in StdGNU.mk.   Optimizations confuse the debugger,
and the comment justifying -O1 in StdGNU.mk should not apply for a
userspace library.   Disable optimization by appending -O0 to CFLAGS,
which overrides the -O1 flag specified earlier.

Also specify -g3, to add macro debugging information which allows
gdb to expand macro invocations.   This is useful as libxl uses many
non-trivial macros.

Signed-off-by: Euan Harris <euan.harris@citrix.com>

Changes since v1:
  * moved flag override to tools/Rules.mk so it affects all tools
---
 tools/Rules.mk |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/tools/Rules.mk b/tools/Rules.mk
index 87a56dc..7ef1ce5 100644
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -54,6 +54,11 @@ CFLAGS_libxenvchan = -I$(XEN_LIBVCHAN)
 LDLIBS_libxenvchan = $(SHLIB_libxenctrl) $(SHLIB_libxenstore) -L$(XEN_LIBVCHAN) -lxenvchan
 SHLIB_libxenvchan  = -Wl,-rpath-link=$(XEN_LIBVCHAN)
 
+ifeq ($(debug),y)
+# Disable optimizations and debugging information for macros
+CFLAGS += -O0 -g3
+endif
+
 LIBXL_BLKTAP ?= $(CONFIG_BLKTAP2)
 
 ifeq ($(LIBXL_BLKTAP),y)
-- 
1.7.1

Re: [PATCH v2] tools/Rules.mk: Don't optimize debug builds; add macro debugging information

[Ian Campbell]

On Mon, 2014-12-01 at 14:21 +0000, Euan Harris wrote:
> Tools debug builds are built with optimization level -O1, inherited from
> the CFLAGS definition in StdGNU.mk.   Optimizations confuse the debugger,
> and the comment justifying -O1 in StdGNU.mk should not apply for a
> userspace library.

(For others, that comment reads: Less than -O1 produces bad code and
large stack frames)

>    Disable optimization by appending -O0 to CFLAGS,
> which overrides the -O1 flag specified earlier.
> 
> Also specify -g3, to add macro debugging information which allows
> gdb to expand macro invocations.   This is useful as libxl uses many
> non-trivial macros.
> 
> Signed-off-by: Euan Harris <euan.harris@citrix.com>
> 
> Changes since v1:
>   * moved flag override to tools/Rules.mk so it affects all tools
> ---
>  tools/Rules.mk |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/tools/Rules.mk b/tools/Rules.mk
> index 87a56dc..7ef1ce5 100644
> --- a/tools/Rules.mk
> +++ b/tools/Rules.mk
> @@ -54,6 +54,11 @@ CFLAGS_libxenvchan = -I$(XEN_LIBVCHAN)
>  LDLIBS_libxenvchan = $(SHLIB_libxenctrl) $(SHLIB_libxenstore) -L$(XEN_LIBVCHAN) -lxenvchan
>  SHLIB_libxenvchan  = -Wl,-rpath-link=$(XEN_LIBVCHAN)
>  
> +ifeq ($(debug),y)
> +# Disable optimizations and debugging information for macros
                               ^enable

... or else it looks like you are saying it should be turning of macro
debug info too. This is something which can be done on commit.

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> +CFLAGS += -O0 -g3
> +endif
> +
>  LIBXL_BLKTAP ?= $(CONFIG_BLKTAP2)
>  
>  ifeq ($(LIBXL_BLKTAP),y)

Re: [PATCH v2] tools/Rules.mk: Don't optimize debug builds; add macro debugging information

[Ian Campbell]

On Thu, 2015-01-08 at 17:16 +0000, Ian Campbell wrote:

> > +ifeq ($(debug),y)
> > +# Disable optimizations and debugging information for macros
>                                ^enable
> 
> ... or else it looks like you are saying it should be turning of macro
> debug info too. This is something which can be done on commit.
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>

Applied, inserting "enable" as described.

Re: [PATCH v2] tools/Rules.mk: Don't optimize debug builds; add macro debugging information

[Wen Congyang]

On 12/01/2014 10:21 PM, Euan Harris wrote:
> Tools debug builds are built with optimization level -O1, inherited from
> the CFLAGS definition in StdGNU.mk.   Optimizations confuse the debugger,
> and the comment justifying -O1 in StdGNU.mk should not apply for a
> userspace library.   Disable optimization by appending -O0 to CFLAGS,
> which overrides the -O1 flag specified earlier.
> 
> Also specify -g3, to add macro debugging information which allows
> gdb to expand macro invocations.   This is useful as libxl uses many
> non-trivial macros.
> 
> Signed-off-by: Euan Harris <euan.harris@citrix.com>
> 
> Changes since v1:
>   * moved flag override to tools/Rules.mk so it affects all tools
> ---
>  tools/Rules.mk |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/tools/Rules.mk b/tools/Rules.mk
> index 87a56dc..7ef1ce5 100644
> --- a/tools/Rules.mk
> +++ b/tools/Rules.mk
> @@ -54,6 +54,11 @@ CFLAGS_libxenvchan = -I$(XEN_LIBVCHAN)
>  LDLIBS_libxenvchan = $(SHLIB_libxenctrl) $(SHLIB_libxenstore) -L$(XEN_LIBVCHAN) -lxenvchan
>  SHLIB_libxenvchan  = -Wl,-rpath-link=$(XEN_LIBVCHAN)
>  
> +ifeq ($(debug),y)
> +# Disable optimizations and debugging information for macros
> +CFLAGS += -O0 -g3
> +endif
> +
>  LIBXL_BLKTAP ?= $(CONFIG_BLKTAP2)
>  
>  ifeq ($(LIBXL_BLKTAP),y)
> 

This patch causes a building error:
gcc -fno-strict-aliasing -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv -DNDEBUG -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv -O1 -fno-omit-frame-pointer -m64 -g -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs -O0 -g3 -D__XEN_TOOLS__ -MMD -MF .install.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -fno-optimize-sibling-calls -fPIC -I../../tools/include -I../../tools/libxc/include -Ixen/lowlevel
 /xc -I/usr/include/python2.7 -c xen/lowlevel/xc/xc.c -o build/temp.linux-x86_64-2.7/xen/lowlevel/xc/xc.o -fno-strict-aliasing -Werror
In file included from /usr/include/limits.h:25:0,
                 from /usr/lib/gcc/x86_64-redhat-linux/4.9.2/include/limits.h:168,
                 from /usr/lib/gcc/x86_64-redhat-linux/4.9.2/include/syslimits.h:7,
                 from /usr/lib/gcc/x86_64-redhat-linux/4.9.2/include/limits.h:34,
                 from /usr/include/python2.7/Python.h:19,
                 from xen/lowlevel/xc/xc.c:7:
/usr/include/features.h:328:4: error: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Werror=cpp]
 #  warning _FORTIFY_SOURCE requires compiling with optimization (-O)
    ^
cc1: all warnings being treated as errors
error: command 'gcc' failed with exit status 1

The following patch can fix this problem:

>From d16961971e14f6e50f9a9905449929d5a7c60860 Mon Sep 17 00:00:00 2001
From: Wen Congyang <wency@cn.fujitsu.com>
Date: Tue, 13 Jan 2015 12:05:30 +0800
Subject: [PATCH] Fix a building error

Commit 1166ecf7 disables optimization. But _FORTIFY_SOURCE
requires compiling with optimization (-O). Disable _FORTIFY_SOURCE
by appending -Wp,-U_FORTIFY_SOURCE to CFLAGS.

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 tools/Rules.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/Rules.mk b/tools/Rules.mk
index 962a743..8ad1b05 100644
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -56,7 +56,7 @@ SHLIB_libxenvchan  = -Wl,-rpath-link=$(XEN_LIBVCHAN)
 
 ifeq ($(debug),y)
 # Disable optimizations and enable debugging information for macros
-CFLAGS += -O0 -g3
+CFLAGS += -O0 -g3 -Wp,-U_FORTIFY_SOURCE
 endif
 
 LIBXL_BLKTAP ?= $(CONFIG_BLKTAP2)
-- 
2.1.0

Re: [PATCH v2] tools/Rules.mk: Don't optimize debug builds; add macro debugging information

[Ian Campbell]

On Tue, 2015-01-13 at 13:52 +0800, Wen Congyang wrote:
> On 12/01/2014 10:21 PM, Euan Harris wrote:
> > Tools debug builds are built with optimization level -O1, inherited from
> > the CFLAGS definition in StdGNU.mk.   Optimizations confuse the debugger,
> > and the comment justifying -O1 in StdGNU.mk should not apply for a
> > userspace library.   Disable optimization by appending -O0 to CFLAGS,
> > which overrides the -O1 flag specified earlier.
> > 
> > Also specify -g3, to add macro debugging information which allows
> > gdb to expand macro invocations.   This is useful as libxl uses many
> > non-trivial macros.
> > 
> > Signed-off-by: Euan Harris <euan.harris@citrix.com>
> > 
> > Changes since v1:
> >   * moved flag override to tools/Rules.mk so it affects all tools
> > ---
> >  tools/Rules.mk |    5 +++++
> >  1 files changed, 5 insertions(+), 0 deletions(-)
> > 
> > diff --git a/tools/Rules.mk b/tools/Rules.mk
> > index 87a56dc..7ef1ce5 100644
> > --- a/tools/Rules.mk
> > +++ b/tools/Rules.mk
> > @@ -54,6 +54,11 @@ CFLAGS_libxenvchan = -I$(XEN_LIBVCHAN)
> >  LDLIBS_libxenvchan = $(SHLIB_libxenctrl) $(SHLIB_libxenstore) -L$(XEN_LIBVCHAN) -lxenvchan
> >  SHLIB_libxenvchan  = -Wl,-rpath-link=$(XEN_LIBVCHAN)
> >  
> > +ifeq ($(debug),y)
> > +# Disable optimizations and debugging information for macros
> > +CFLAGS += -O0 -g3
> > +endif
> > +
> >  LIBXL_BLKTAP ?= $(CONFIG_BLKTAP2)
> >  
> >  ifeq ($(LIBXL_BLKTAP),y)
> > 
> 
> This patch causes a building error:
> gcc -fno-strict-aliasing -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv -DNDEBUG -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv -O1 -fno-omit-frame-pointer -m64 -g -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs -O0 -g3 -D__XEN_TOOLS__ -MMD -MF .install.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -fno-optimize-sibling-calls -fPIC -I../../tools/include -I../../tools/libxc/include -Ixen/lowlev
 el/xc -I/usr/include/python2.7 -c xen/lowlevel/xc/xc.c -o build/temp.linux-x86_64-2.7/xen/lowlevel/xc/xc.o -fno-strict-aliasing -Werror
> In file included from /usr/include/limits.h:25:0,
>                  from /usr/lib/gcc/x86_64-redhat-linux/4.9.2/include/limits.h:168,
>                  from /usr/lib/gcc/x86_64-redhat-linux/4.9.2/include/syslimits.h:7,
>                  from /usr/lib/gcc/x86_64-redhat-linux/4.9.2/include/limits.h:34,
>                  from /usr/include/python2.7/Python.h:19,
>                  from xen/lowlevel/xc/xc.c:7:
> /usr/include/features.h:328:4: error: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Werror=cpp]

Where is _FORTIFY_SOURCE coming from? I don't see it in our tree
anywhere except stubdom/Makefile which is disabling it and the build
worked for me. Perhaps it is coming from your build environment
somewhere? How are you configuring and building Xen?

Maybe what we want to do is only disable optimisations if debug=y AND
-D_FORTIFY_SOURCE is not set? Might involve some autoconf checks to
determine the fortification level in the user provided CFLAGS, which
might be a bit faffsome.

Ian.

>  #  warning _FORTIFY_SOURCE requires compiling with optimization (-O)
>     ^
> cc1: all warnings being treated as errors
> error: command 'gcc' failed with exit status 1
> 
> The following patch can fix this problem:
> 
> From d16961971e14f6e50f9a9905449929d5a7c60860 Mon Sep 17 00:00:00 2001
> From: Wen Congyang <wency@cn.fujitsu.com>
> Date: Tue, 13 Jan 2015 12:05:30 +0800
> Subject: [PATCH] Fix a building error
> 
> Commit 1166ecf7 disables optimization. But _FORTIFY_SOURCE
> requires compiling with optimization (-O). Disable _FORTIFY_SOURCE
> by appending -Wp,-U_FORTIFY_SOURCE to CFLAGS.
> 
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
>  tools/Rules.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/Rules.mk b/tools/Rules.mk
> index 962a743..8ad1b05 100644
> --- a/tools/Rules.mk
> +++ b/tools/Rules.mk
> @@ -56,7 +56,7 @@ SHLIB_libxenvchan  = -Wl,-rpath-link=$(XEN_LIBVCHAN)
>  
>  ifeq ($(debug),y)
>  # Disable optimizations and enable debugging information for macros
> -CFLAGS += -O0 -g3
> +CFLAGS += -O0 -g3 -Wp,-U_FORTIFY_SOURCE
>  endif
>  
>  LIBXL_BLKTAP ?= $(CONFIG_BLKTAP2)

Re: [PATCH v2] tools/Rules.mk: Don't optimize debug builds; add macro debugging information

[Wen Congyang]

On 01/13/2015 06:11 PM, Ian Campbell wrote:
> On Tue, 2015-01-13 at 13:52 +0800, Wen Congyang wrote:
>> On 12/01/2014 10:21 PM, Euan Harris wrote:
>>> Tools debug builds are built with optimization level -O1, inherited from
>>> the CFLAGS definition in StdGNU.mk.   Optimizations confuse the debugger,
>>> and the comment justifying -O1 in StdGNU.mk should not apply for a
>>> userspace library.   Disable optimization by appending -O0 to CFLAGS,
>>> which overrides the -O1 flag specified earlier.
>>>
>>> Also specify -g3, to add macro debugging information which allows
>>> gdb to expand macro invocations.   This is useful as libxl uses many
>>> non-trivial macros.
>>>
>>> Signed-off-by: Euan Harris <euan.harris@citrix.com>
>>>
>>> Changes since v1:
>>>   * moved flag override to tools/Rules.mk so it affects all tools
>>> ---
>>>  tools/Rules.mk |    5 +++++
>>>  1 files changed, 5 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/tools/Rules.mk b/tools/Rules.mk
>>> index 87a56dc..7ef1ce5 100644
>>> --- a/tools/Rules.mk
>>> +++ b/tools/Rules.mk
>>> @@ -54,6 +54,11 @@ CFLAGS_libxenvchan = -I$(XEN_LIBVCHAN)
>>>  LDLIBS_libxenvchan = $(SHLIB_libxenctrl) $(SHLIB_libxenstore) -L$(XEN_LIBVCHAN) -lxenvchan
>>>  SHLIB_libxenvchan  = -Wl,-rpath-link=$(XEN_LIBVCHAN)
>>>  
>>> +ifeq ($(debug),y)
>>> +# Disable optimizations and debugging information for macros
>>> +CFLAGS += -O0 -g3
>>> +endif
>>> +
>>>  LIBXL_BLKTAP ?= $(CONFIG_BLKTAP2)
>>>  
>>>  ifeq ($(LIBXL_BLKTAP),y)
>>>
>>
>> This patch causes a building error:
>> gcc -fno-strict-aliasing -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv -DNDEBUG -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv -O1 -fno-omit-frame-pointer -m64 -g -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs -O0 -g3 -D__XEN_TOOLS__ -MMD -MF .install.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -fno-optimize-sibling-calls -fPIC -I../../tools/include -I../../tools/libxc/include -Ixen/lowle
 vel/xc -I/usr/include/python2.7 -c xen/lowlevel/xc/xc.c -o build/temp.linux-x86_64-2.7/xen/lowlevel/xc/xc.o -fno-strict-aliasing -Werror
>> In file included from /usr/include/limits.h:25:0,
>>                  from /usr/lib/gcc/x86_64-redhat-linux/4.9.2/include/limits.h:168,
>>                  from /usr/lib/gcc/x86_64-redhat-linux/4.9.2/include/syslimits.h:7,
>>                  from /usr/lib/gcc/x86_64-redhat-linux/4.9.2/include/limits.h:34,
>>                  from /usr/include/python2.7/Python.h:19,
>>                  from xen/lowlevel/xc/xc.c:7:
>> /usr/include/features.h:328:4: error: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Werror=cpp]
> 
> Where is _FORTIFY_SOURCE coming from? I don't see it in our tree
> anywhere except stubdom/Makefile which is disabling it and the build
> worked for me. Perhaps it is coming from your build environment
> somewhere? How are you configuring and building Xen?

I don't knwo where is _FORTIFY_SOURCE coming from.

I use fedora 21 to build xen. The command line is:
./autogen.sh
./configure --prefix=/usr --libdir=/usr/lib64
make -j8 tools

Thanks
Wen Congyang

> 
> Maybe what we want to do is only disable optimisations if debug=y AND
> -D_FORTIFY_SOURCE is not set? Might involve some autoconf checks to
> determine the fortification level in the user provided CFLAGS, which
> might be a bit faffsome.
> 
> Ian.
> 
>>  #  warning _FORTIFY_SOURCE requires compiling with optimization (-O)
>>     ^
>> cc1: all warnings being treated as errors
>> error: command 'gcc' failed with exit status 1
>>
>> The following patch can fix this problem:
>>
>> From d16961971e14f6e50f9a9905449929d5a7c60860 Mon Sep 17 00:00:00 2001
>> From: Wen Congyang <wency@cn.fujitsu.com>
>> Date: Tue, 13 Jan 2015 12:05:30 +0800
>> Subject: [PATCH] Fix a building error
>>
>> Commit 1166ecf7 disables optimization. But _FORTIFY_SOURCE
>> requires compiling with optimization (-O). Disable _FORTIFY_SOURCE
>> by appending -Wp,-U_FORTIFY_SOURCE to CFLAGS.
>>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>>  tools/Rules.mk | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/Rules.mk b/tools/Rules.mk
>> index 962a743..8ad1b05 100644
>> --- a/tools/Rules.mk
>> +++ b/tools/Rules.mk
>> @@ -56,7 +56,7 @@ SHLIB_libxenvchan  = -Wl,-rpath-link=$(XEN_LIBVCHAN)
>>  
>>  ifeq ($(debug),y)
>>  # Disable optimizations and enable debugging information for macros
>> -CFLAGS += -O0 -g3
>> +CFLAGS += -O0 -g3 -Wp,-U_FORTIFY_SOURCE
>>  endif
>>  
>>  LIBXL_BLKTAP ?= $(CONFIG_BLKTAP2)
> 
> 
> .
> 

Re: [PATCH v2] tools/Rules.mk: Don't optimize debug builds; add macro debugging information

[Wen Congyang]

On 01/13/2015 06:11 PM, Ian Campbell wrote:
> On Tue, 2015-01-13 at 13:52 +0800, Wen Congyang wrote:
>> On 12/01/2014 10:21 PM, Euan Harris wrote:
>>> Tools debug builds are built with optimization level -O1, inherited from
>>> the CFLAGS definition in StdGNU.mk.   Optimizations confuse the debugger,
>>> and the comment justifying -O1 in StdGNU.mk should not apply for a
>>> userspace library.   Disable optimization by appending -O0 to CFLAGS,
>>> which overrides the -O1 flag specified earlier.
>>>
>>> Also specify -g3, to add macro debugging information which allows
>>> gdb to expand macro invocations.   This is useful as libxl uses many
>>> non-trivial macros.
>>>
>>> Signed-off-by: Euan Harris <euan.harris@citrix.com>
>>>
>>> Changes since v1:
>>>   * moved flag override to tools/Rules.mk so it affects all tools
>>> ---
>>>  tools/Rules.mk |    5 +++++
>>>  1 files changed, 5 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/tools/Rules.mk b/tools/Rules.mk
>>> index 87a56dc..7ef1ce5 100644
>>> --- a/tools/Rules.mk
>>> +++ b/tools/Rules.mk
>>> @@ -54,6 +54,11 @@ CFLAGS_libxenvchan = -I$(XEN_LIBVCHAN)
>>>  LDLIBS_libxenvchan = $(SHLIB_libxenctrl) $(SHLIB_libxenstore) -L$(XEN_LIBVCHAN) -lxenvchan
>>>  SHLIB_libxenvchan  = -Wl,-rpath-link=$(XEN_LIBVCHAN)
>>>  
>>> +ifeq ($(debug),y)
>>> +# Disable optimizations and debugging information for macros
>>> +CFLAGS += -O0 -g3
>>> +endif
>>> +
>>>  LIBXL_BLKTAP ?= $(CONFIG_BLKTAP2)
>>>  
>>>  ifeq ($(LIBXL_BLKTAP),y)
>>>
>>
>> This patch causes a building error:
>> gcc -fno-strict-aliasing -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv -DNDEBUG -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv -O1 -fno-omit-frame-pointer -m64 -g -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs -O0 -g3 -D__XEN_TOOLS__ -MMD -MF .install.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -fno-optimize-sibling-calls -fPIC -I../../tools/include -I../../tools/libxc/include -Ixen/lowle
 vel/xc -I/usr/include/python2.7 -c xen/lowlevel/xc/xc.c -o build/temp.linux-x86_64-2.7/xen/lowlevel/xc/xc.o -fno-strict-aliasing -Werror
>> In file included from /usr/include/limits.h:25:0,
>>                  from /usr/lib/gcc/x86_64-redhat-linux/4.9.2/include/limits.h:168,
>>                  from /usr/lib/gcc/x86_64-redhat-linux/4.9.2/include/syslimits.h:7,
>>                  from /usr/lib/gcc/x86_64-redhat-linux/4.9.2/include/limits.h:34,
>>                  from /usr/include/python2.7/Python.h:19,
>>                  from xen/lowlevel/xc/xc.c:7:
>> /usr/include/features.h:328:4: error: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Werror=cpp]
> 
> Where is _FORTIFY_SOURCE coming from? I don't see it in our tree
> anywhere except stubdom/Makefile which is disabling it and the build
> worked for me. Perhaps it is coming from your build environment
> somewhere? How are you configuring and building Xen?

_FORTIFY_SOURCE is just for python, and it comes from /usr/lib64/python2.7/config/Makefile:

# Compiler options
OPT=            -DNDEBUG -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches  -m64 -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv
BASECFLAGS=      -fno-strict-aliasing
CFLAGS=         $(BASECFLAGS) -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches  -m64 -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv  $(OPT) $(EXTRA_CFLAGS)


> 
> Maybe what we want to do is only disable optimisations if debug=y AND
> -D_FORTIFY_SOURCE is not set? Might involve some autoconf checks to
> determine the fortification level in the user provided CFLAGS, which
> might be a bit faffsome.
> 
> Ian.
> 
>>  #  warning _FORTIFY_SOURCE requires compiling with optimization (-O)
>>     ^
>> cc1: all warnings being treated as errors
>> error: command 'gcc' failed with exit status 1
>>
>> The following patch can fix this problem:
>>
>> From d16961971e14f6e50f9a9905449929d5a7c60860 Mon Sep 17 00:00:00 2001
>> From: Wen Congyang <wency@cn.fujitsu.com>
>> Date: Tue, 13 Jan 2015 12:05:30 +0800
>> Subject: [PATCH] Fix a building error
>>
>> Commit 1166ecf7 disables optimization. But _FORTIFY_SOURCE
>> requires compiling with optimization (-O). Disable _FORTIFY_SOURCE
>> by appending -Wp,-U_FORTIFY_SOURCE to CFLAGS.
>>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>>  tools/Rules.mk | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/Rules.mk b/tools/Rules.mk
>> index 962a743..8ad1b05 100644
>> --- a/tools/Rules.mk
>> +++ b/tools/Rules.mk
>> @@ -56,7 +56,7 @@ SHLIB_libxenvchan  = -Wl,-rpath-link=$(XEN_LIBVCHAN)
>>  
>>  ifeq ($(debug),y)
>>  # Disable optimizations and enable debugging information for macros
>> -CFLAGS += -O0 -g3
>> +CFLAGS += -O0 -g3 -Wp,-U_FORTIFY_SOURCE
>>  endif
>>  
>>  LIBXL_BLKTAP ?= $(CONFIG_BLKTAP2)
> 
> 
> .
> 

Re: [PATCH v2] tools/Rules.mk: Don't optimize debug builds; add macro debugging information

[Ian Campbell]

On Tue, 2015-01-13 at 19:17 +0800, Wen Congyang wrote:
> On 01/13/2015 06:11 PM, Ian Campbell wrote:
> > On Tue, 2015-01-13 at 13:52 +0800, Wen Congyang wrote:
> >> On 12/01/2014 10:21 PM, Euan Harris wrote:
> >>> Tools debug builds are built with optimization level -O1, inherited from
> >>> the CFLAGS definition in StdGNU.mk.   Optimizations confuse the debugger,
> >>> and the comment justifying -O1 in StdGNU.mk should not apply for a
> >>> userspace library.   Disable optimization by appending -O0 to CFLAGS,
> >>> which overrides the -O1 flag specified earlier.
> >>>
> >>> Also specify -g3, to add macro debugging information which allows
> >>> gdb to expand macro invocations.   This is useful as libxl uses many
> >>> non-trivial macros.
> >>>
> >>> Signed-off-by: Euan Harris <euan.harris@citrix.com>
> >>>
> >>> Changes since v1:
> >>>   * moved flag override to tools/Rules.mk so it affects all tools
> >>> ---
> >>>  tools/Rules.mk |    5 +++++
> >>>  1 files changed, 5 insertions(+), 0 deletions(-)
> >>>
> >>> diff --git a/tools/Rules.mk b/tools/Rules.mk
> >>> index 87a56dc..7ef1ce5 100644
> >>> --- a/tools/Rules.mk
> >>> +++ b/tools/Rules.mk
> >>> @@ -54,6 +54,11 @@ CFLAGS_libxenvchan = -I$(XEN_LIBVCHAN)
> >>>  LDLIBS_libxenvchan = $(SHLIB_libxenctrl) $(SHLIB_libxenstore) -L$(XEN_LIBVCHAN) -lxenvchan
> >>>  SHLIB_libxenvchan  = -Wl,-rpath-link=$(XEN_LIBVCHAN)
> >>>  
> >>> +ifeq ($(debug),y)
> >>> +# Disable optimizations and debugging information for macros
> >>> +CFLAGS += -O0 -g3
> >>> +endif
> >>> +
> >>>  LIBXL_BLKTAP ?= $(CONFIG_BLKTAP2)
> >>>  
> >>>  ifeq ($(LIBXL_BLKTAP),y)
> >>>
> >>
> >> This patch causes a building error:
> >> gcc -fno-strict-aliasing -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv -DNDEBUG -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv -O1 -fno-omit-frame-pointer -m64 -g -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs -O0 -g3 -D__XEN_TOOLS__ -MMD -MF .install.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -fno-optimize-sibling-calls -fPIC -I../../tools/include -I../../tools/libxc/include -Ixen/low
 level/xc -I/usr/include/python2.7 -c xen/lowlevel/xc/xc.c -o build/temp.linux-x86_64-2.7/xen/lowlevel/xc/xc.o -fno-strict-aliasing -Werror
> >> In file included from /usr/include/limits.h:25:0,
> >>                  from /usr/lib/gcc/x86_64-redhat-linux/4.9.2/include/limits.h:168,
> >>                  from /usr/lib/gcc/x86_64-redhat-linux/4.9.2/include/syslimits.h:7,
> >>                  from /usr/lib/gcc/x86_64-redhat-linux/4.9.2/include/limits.h:34,
> >>                  from /usr/include/python2.7/Python.h:19,
> >>                  from xen/lowlevel/xc/xc.c:7:
> >> /usr/include/features.h:328:4: error: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Werror=cpp]
> > 
> > Where is _FORTIFY_SOURCE coming from? I don't see it in our tree
> > anywhere except stubdom/Makefile which is disabling it and the build
> > worked for me. Perhaps it is coming from your build environment
> > somewhere? How are you configuring and building Xen?
> 
> _FORTIFY_SOURCE is just for python, and it comes from /usr/lib64/python2.7/config/Makefile:
> 
> # Compiler options
> OPT=            -DNDEBUG -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches  -m64 -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv
> BASECFLAGS=      -fno-strict-aliasing
> CFLAGS=         $(BASECFLAGS) -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches  -m64 -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv  $(OPT) $(EXTRA_CFLAGS)
> 

That's a bit antisocial of it...

I can't reproduce this on Debian with Python 2.7. _FORTIFY_SOURCE=2 is
not passed when building the Python bits (it is passed when linking,
oddly). Debian's /usr/lib/python2.7/config-x86_64-linux-gnu/Makefile
looks rather different to what you quote above though, it avoids bare
CFLAGS for one thing (reserving them for the user).

I have a suspicion that this might be a bug in the Fedora Python
packaging, in that they have leaked inappropriate build-time flags from
the actual Python build into the set of flags to be used to build 3rd
party Python modules.

Please can you check this with the Fedora folks? If they respond that
this is deliberate etc then please let us know and we'll see what we can
do in Xen to work around it.

Ian.

Re: [PATCH v2] tools/Rules.mk: Don't optimize debug builds; add macro debugging information

[Wen Congyang]

On 01/13/2015 07:30 PM, Ian Campbell wrote:
> On Tue, 2015-01-13 at 19:17 +0800, Wen Congyang wrote:
>> On 01/13/2015 06:11 PM, Ian Campbell wrote:
>>> On Tue, 2015-01-13 at 13:52 +0800, Wen Congyang wrote:
>>>> On 12/01/2014 10:21 PM, Euan Harris wrote:
>>>>> Tools debug builds are built with optimization level -O1, inherited from
>>>>> the CFLAGS definition in StdGNU.mk.   Optimizations confuse the debugger,
>>>>> and the comment justifying -O1 in StdGNU.mk should not apply for a
>>>>> userspace library.   Disable optimization by appending -O0 to CFLAGS,
>>>>> which overrides the -O1 flag specified earlier.
>>>>>
>>>>> Also specify -g3, to add macro debugging information which allows
>>>>> gdb to expand macro invocations.   This is useful as libxl uses many
>>>>> non-trivial macros.
>>>>>
>>>>> Signed-off-by: Euan Harris <euan.harris@citrix.com>
>>>>>
>>>>> Changes since v1:
>>>>>   * moved flag override to tools/Rules.mk so it affects all tools
>>>>> ---
>>>>>  tools/Rules.mk |    5 +++++
>>>>>  1 files changed, 5 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/tools/Rules.mk b/tools/Rules.mk
>>>>> index 87a56dc..7ef1ce5 100644
>>>>> --- a/tools/Rules.mk
>>>>> +++ b/tools/Rules.mk
>>>>> @@ -54,6 +54,11 @@ CFLAGS_libxenvchan = -I$(XEN_LIBVCHAN)
>>>>>  LDLIBS_libxenvchan = $(SHLIB_libxenctrl) $(SHLIB_libxenstore) -L$(XEN_LIBVCHAN) -lxenvchan
>>>>>  SHLIB_libxenvchan  = -Wl,-rpath-link=$(XEN_LIBVCHAN)
>>>>>  
>>>>> +ifeq ($(debug),y)
>>>>> +# Disable optimizations and debugging information for macros
>>>>> +CFLAGS += -O0 -g3
>>>>> +endif
>>>>> +
>>>>>  LIBXL_BLKTAP ?= $(CONFIG_BLKTAP2)
>>>>>  
>>>>>  ifeq ($(LIBXL_BLKTAP),y)
>>>>>
>>>>
>>>> This patch causes a building error:
>>>> gcc -fno-strict-aliasing -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv -DNDEBUG -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv -O1 -fno-omit-frame-pointer -m64 -g -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs -O0 -g3 -D__XEN_TOOLS__ -MMD -MF .install.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -fno-optimize-sibling-calls -fPIC -I../../tools/include -I../../tools/libxc/include -Ixen/low
 level/xc -I/usr/include/python2.7 -c xen/lowlevel/xc/xc.c -o build/temp.linux-x86_64-2.7/xen/lowlevel/xc/xc.o -fno-strict-aliasing -Werror
>>>> In file included from /usr/include/limits.h:25:0,
>>>>                  from /usr/lib/gcc/x86_64-redhat-linux/4.9.2/include/limits.h:168,
>>>>                  from /usr/lib/gcc/x86_64-redhat-linux/4.9.2/include/syslimits.h:7,
>>>>                  from /usr/lib/gcc/x86_64-redhat-linux/4.9.2/include/limits.h:34,
>>>>                  from /usr/include/python2.7/Python.h:19,
>>>>                  from xen/lowlevel/xc/xc.c:7:
>>>> /usr/include/features.h:328:4: error: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Werror=cpp]
>>>
>>> Where is _FORTIFY_SOURCE coming from? I don't see it in our tree
>>> anywhere except stubdom/Makefile which is disabling it and the build
>>> worked for me. Perhaps it is coming from your build environment
>>> somewhere? How are you configuring and building Xen?
>>
>> _FORTIFY_SOURCE is just for python, and it comes from /usr/lib64/python2.7/config/Makefile:
>>
>> # Compiler options
>> OPT=            -DNDEBUG -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches  -m64 -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv
>> BASECFLAGS=      -fno-strict-aliasing
>> CFLAGS=         $(BASECFLAGS) -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches  -m64 -mtune=generic -D_GNU_SOURCE -fPIC -fwrapv  $(OPT) $(EXTRA_CFLAGS)
>>
> 
> That's a bit antisocial of it...
> 
> I can't reproduce this on Debian with Python 2.7. _FORTIFY_SOURCE=2 is
> not passed when building the Python bits (it is passed when linking,
> oddly). Debian's /usr/lib/python2.7/config-x86_64-linux-gnu/Makefile
> looks rather different to what you quote above though, it avoids bare
> CFLAGS for one thing (reserving them for the user).
> 
> I have a suspicion that this might be a bug in the Fedora Python
> packaging, in that they have leaked inappropriate build-time flags from
> the actual Python build into the set of flags to be used to build 3rd
> party Python modules.
> 
> Please can you check this with the Fedora folks? If they respond that
> this is deliberate etc then please let us know and we'll see what we can
> do in Xen to work around it.

Here is the reponse from Fedora folks:
-------- Forwarded Message --------
Subject: Re: [Xen-devel] [PATCH v2] tools/Rules.mk: Don't optimize debug builds; add macro debugging information
Date: Wed, 14 Jan 2015 12:22:19 -0500
From: Bohuslav Kabrda <bkabrda@redhat.com>
To: Wen Congyang <wency@cn.fujitsu.com>

----- Original Message -----
> Hi, bkabrda
> 
> When I build some python modules in xen, I find some building error.
> 
> The reason is that: we need to disable optimization by -O0, but
> the CFLAGS in python's Makefile have -Wp,-D_FORTIFY_SOURCE=2.
> 
> My question is that: Is this flag essential?
> 
> Thanks
> Wen Congyang

Hi,
all the flags come from default RPM build configuration that all Fedora packages are supposed to use. They are saved by Python's build process to config/Makefile. Running "setup.py build" then uses them for building extension modules. So to answer your question: yes, this is mandated by Fedora and will not go away.
Now, there are two solutions to your situation I can see:
- either add "undef_macros" to setup.py definition of the extension module, as documented at [1]
- run "setup.py build" in environment where you set "CFLAGS=-U_FORTIFY_SOURCE"
Both will do the same - undefine the _FORTIFY_SOURCE macro in the gcc invocations.

Hope this helps,
Slavek.

[1] https://docs.python.org/2/distutils/setupscript.html#preprocessor-options

> 
> Ian.
> 
> .
> 

[PATCH] Fix building error

[Wen Congyang]

Commit 1166ecf7 disables optimization. But python's build
process may use CFLAGS -Wp,-D_FORTIFY_SOURCE=2, and suppose
the 3rd party python modules to use. The macro _FORTIFY_SOURCE
requires compiling with optimization (-O). Disable _FORTIFY_SOURCE
by appending -Wp,-U_FORTIFY_SOURCE to CFLAGS.

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 tools/pygrub/Makefile | 5 +++++
 tools/python/Makefile | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/tools/pygrub/Makefile b/tools/pygrub/Makefile
index 3dff608..3d86254 100644
--- a/tools/pygrub/Makefile
+++ b/tools/pygrub/Makefile
@@ -2,6 +2,11 @@
 XEN_ROOT = $(CURDIR)/../..
 include $(XEN_ROOT)/tools/Rules.mk
 
+ifeq ($(debug),y)
+# Disable _FORTIFY_SOURCE
+CFLAGS += -Wp,-U_FORTIFY_SOURCE
+endif
+
 .PHONY: all
 all: build
 .PHONY: build
diff --git a/tools/python/Makefile b/tools/python/Makefile
index 533d3de..3997660 100644
--- a/tools/python/Makefile
+++ b/tools/python/Makefile
@@ -4,6 +4,11 @@ include $(XEN_ROOT)/tools/Rules.mk
 .PHONY: all
 all: build
 
+ifeq ($(debug),y)
+# Disable _FORTIFY_SOURCE
+CFLAGS += -Wp,-U_FORTIFY_SOURCE
+endif
+
 .PHONY: build
 build: genwrap.py $(XEN_ROOT)/tools/libxl/libxl_types.idl \
 		$(XEN_ROOT)/tools/libxl/idl.py
-- 
2.1.0

Re: [PATCH] Fix building error

[Olaf Hering]

On Thu, Jan 15, Wen Congyang wrote:

> Commit 1166ecf7 disables optimization. But python's build
> process may use CFLAGS -Wp,-D_FORTIFY_SOURCE=2, and suppose
> the 3rd party python modules to use. The macro _FORTIFY_SOURCE
> requires compiling with optimization (-O). Disable _FORTIFY_SOURCE
> by appending -Wp,-U_FORTIFY_SOURCE to CFLAGS.

Why is that not done globally, in the place where -O is tweaked?
The comment can be removed, it states the obvious.

Olaf

Re: [PATCH] Fix building error

[Wen Congyang]

On 01/15/2015 03:57 PM, Olaf Hering wrote:
> On Thu, Jan 15, Wen Congyang wrote:
> 
>> Commit 1166ecf7 disables optimization. But python's build
>> process may use CFLAGS -Wp,-D_FORTIFY_SOURCE=2, and suppose
>> the 3rd party python modules to use. The macro _FORTIFY_SOURCE
>> requires compiling with optimization (-O). Disable _FORTIFY_SOURCE
>> by appending -Wp,-U_FORTIFY_SOURCE to CFLAGS.
> 
> Why is that not done globally, in the place where -O is tweaked?
> The comment can be removed, it states the obvious.

Xen doesn't use the macro _FORTIFY_SOURCE, so it only affects python
modules now.

Thanks
Wen Congyang

> 
> Olaf
> 

Re: [PATCH] Fix building error

[Olaf Hering]

On Thu, Jan 15, Wen Congyang wrote:

> Xen doesn't use the macro _FORTIFY_SOURCE, so it only affects python
> modules now.

It does if it is passed via global CFLAGS, like the RPM_OPT_FLAGS from a
rpm package build.

Olaf

Re: [PATCH] Fix building error

[Wen Congyang]

On 01/15/2015 05:21 PM, Olaf Hering wrote:
> On Thu, Jan 15, Wen Congyang wrote:
> 
>> Xen doesn't use the macro _FORTIFY_SOURCE, so it only affects python
>> modules now.
> 
> It does if it is passed via global CFLAGS, like the RPM_OPT_FLAGS from a
> rpm package build.

Yes, I don't consider such case. I will send a new patch later.

Thanks
Wen Congyang

> 
> Olaf
> 

[PATCH v2] Fix building error

[Wen Congyang]

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
 tools/Rules.mk | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/Rules.mk b/tools/Rules.mk
index 962a743..0a54bf1 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
+# _FORTIFY_SOURCE requires compiling with optimization
+CFLAGS += -Wp,-U_FORTIFY_SOURCE
 endif
 
 LIBXL_BLKTAP ?= $(CONFIG_BLKTAP2)
-- 
2.1.0

Re: [PATCH v2] Fix building error

[Ian Jackson]

Wen Congyang writes ("[PATCH v2] Fix building error"):
>  ifeq ($(debug),y)
>  # Disable optimizations and enable debugging information for macros
>  CFLAGS += -O0 -g3
> +# _FORTIFY_SOURCE requires compiling with optimization
> +CFLAGS += -Wp,-U_FORTIFY_SOURCE

I'm not entirely convinced about this.  I have two kinds of concern:

One is practical, which is that ATM AIUI a debug build, while not
intended for production use, is not any less secure.  (Leaving aside
the ability of guests to perform a DoS with copious debugging output.)

The other is that we seem to be entering into a battle of escalation
between various Makefiles.  Specifically, this seems to be a
workaround for a bug in some other Makefiles we are using: the bug
being that these other Makefiles can't cope with -O0.  And
unconditionally squashing the other Makefiles' options seems like a
big hammer.

The fact that Fortify doesn't support -O0 is a property of Fortify
which ought to be encoded in Fortify (or the places where it is
enabled).

Assuming that the underlying bug is intractible I think the right
answer is for an affected developer to do one of the following, as a
workaround: either, manually override Fortify when requesting a debug
build (by setting EXTRA_CFLAGS_XEN_TOOLS), or manually override the
-O0 setting.

To make this easier to do without editing tools/Rules.mk I would
support a patch to Rules.mk which has it honour a variable containing
a debug-specific set of CFLAGS.

Thanks,
Ian.

Re: [PATCH v2] Fix building error

[Ian Campbell]

On Thu, 2015-01-15 at 11:26 +0000, Ian Jackson wrote:
> Wen Congyang writes ("[PATCH v2] Fix building error"):
> >  ifeq ($(debug),y)
> >  # Disable optimizations and enable debugging information for macros
> >  CFLAGS += -O0 -g3
> > +# _FORTIFY_SOURCE requires compiling with optimization
> > +CFLAGS += -Wp,-U_FORTIFY_SOURCE
> 
> I'm not entirely convinced about this.  I have two kinds of concern:
> 
> One is practical, which is that ATM AIUI a debug build, while not
> intended for production use, is not any less secure.  (Leaving aside
> the ability of guests to perform a DoS with copious debugging output.)
> 
> The other is that we seem to be entering into a battle of escalation
> between various Makefiles.  Specifically, this seems to be a
> workaround for a bug in some other Makefiles we are using: the bug
> being that these other Makefiles can't cope with -O0.  And
> unconditionally squashing the other Makefiles' options seems like a
> big hammer.
> 
> The fact that Fortify doesn't support -O0 is a property of Fortify
> which ought to be encoded in Fortify (or the places where it is
> enabled).
> 
> Assuming that the underlying bug is intractible

I think so, see <54B73623.9040503@cn.fujitsu.com>. I suppose one could
enter into a dialogue with Fedora about the practice of enabling these
flags for all Python modules built on a Fedora system vs. just those
built via RPM etc.

>  I think the right
> answer is for an affected developer

Which will be all developers using Fedora AFAICT.

>  to do one of the following, as a
> workaround: either, manually override Fortify when requesting a debug
> build (by setting EXTRA_CFLAGS_XEN_TOOLS), or manually override the
> -O0 setting.
> 
> To make this easier to do without editing tools/Rules.mk I would
> support a patch to Rules.mk which has it honour a variable containing
> a debug-specific set of CFLAGS.

This seems reasonable enough to me.

The original patch in <54B73658.6030309@cn.fujitsu.com> (correctly IMHO)
applied the workaround only to the Python parts of the build
(tools/{python,pygrub}) whereas this v2 and your suggestion would affect
all of tools/*. That seems like a reasonable compromise under the
circumstances (the alternative being special overrides for Python or
something, no nice).

Ian.

Re: [PATCH v2] Fix building error

[Wen Congyang]

On 01/19/2015 11:23 PM, Ian Campbell wrote:
> On Thu, 2015-01-15 at 11:26 +0000, Ian Jackson wrote:
>> Wen Congyang writes ("[PATCH v2] Fix building error"):
>>>  ifeq ($(debug),y)
>>>  # Disable optimizations and enable debugging information for macros
>>>  CFLAGS += -O0 -g3
>>> +# _FORTIFY_SOURCE requires compiling with optimization
>>> +CFLAGS += -Wp,-U_FORTIFY_SOURCE
>>
>> I'm not entirely convinced about this.  I have two kinds of concern:
>>
>> One is practical, which is that ATM AIUI a debug build, while not
>> intended for production use, is not any less secure.  (Leaving aside
>> the ability of guests to perform a DoS with copious debugging output.)
>>
>> The other is that we seem to be entering into a battle of escalation
>> between various Makefiles.  Specifically, this seems to be a
>> workaround for a bug in some other Makefiles we are using: the bug
>> being that these other Makefiles can't cope with -O0.  And
>> unconditionally squashing the other Makefiles' options seems like a
>> big hammer.
>>
>> The fact that Fortify doesn't support -O0 is a property of Fortify
>> which ought to be encoded in Fortify (or the places where it is
>> enabled).
>>
>> Assuming that the underlying bug is intractible
> 
> I think so, see <54B73623.9040503@cn.fujitsu.com>. I suppose one could
> enter into a dialogue with Fedora about the practice of enabling these
> flags for all Python modules built on a Fedora system vs. just those
> built via RPM etc.
> 
>>  I think the right
>> answer is for an affected developer
> 
> Which will be all developers using Fedora AFAICT.
> 
>>  to do one of the following, as a
>> workaround: either, manually override Fortify when requesting a debug
>> build (by setting EXTRA_CFLAGS_XEN_TOOLS), or manually override the
>> -O0 setting.
>>
>> To make this easier to do without editing tools/Rules.mk I would
>> support a patch to Rules.mk which has it honour a variable containing
>> a debug-specific set of CFLAGS.

I don't understand your suggestion.

> 
> This seems reasonable enough to me.
> 
> The original patch in <54B73658.6030309@cn.fujitsu.com> (correctly IMHO)
> applied the workaround only to the Python parts of the build
> (tools/{python,pygrub}) whereas this v2 and your suggestion would affect
> all of tools/*. That seems like a reasonable compromise under the
> circumstances (the alternative being special overrides for Python or
> something, no nice).

Building problems only affect developers, so I think we should fix this problem.

If the developers add extra flags to define the macro _FORTIFY_SOURCE, it also
breaks building. We can disable debug for such case, or undefine the macro
in Rules.mk. If the macro _FORTIFY_SOURCE is not defined, undefining it is harmless.

Thanks
Wen Congyang

> 
> Ian.
> 
> .
> 

Re: [PATCH v2] Fix building error

[Ian Campbell]

On Tue, 2015-01-20 at 10:21 +0800, Wen Congyang wrote:
> On 01/19/2015 11:23 PM, Ian Campbell wrote:
> > On Thu, 2015-01-15 at 11:26 +0000, Ian Jackson wrote:
> >> Wen Congyang writes ("[PATCH v2] Fix building error"):
> >>>  ifeq ($(debug),y)
> >>>  # Disable optimizations and enable debugging information for macros
> >>>  CFLAGS += -O0 -g3
> >>> +# _FORTIFY_SOURCE requires compiling with optimization
> >>> +CFLAGS += -Wp,-U_FORTIFY_SOURCE
> >>
> >> I'm not entirely convinced about this.  I have two kinds of concern:
> >>
> >> One is practical, which is that ATM AIUI a debug build, while not
> >> intended for production use, is not any less secure.  (Leaving aside
> >> the ability of guests to perform a DoS with copious debugging output.)
> >>
> >> The other is that we seem to be entering into a battle of escalation
> >> between various Makefiles.  Specifically, this seems to be a
> >> workaround for a bug in some other Makefiles we are using: the bug
> >> being that these other Makefiles can't cope with -O0.  And
> >> unconditionally squashing the other Makefiles' options seems like a
> >> big hammer.
> >>
> >> The fact that Fortify doesn't support -O0 is a property of Fortify
> >> which ought to be encoded in Fortify (or the places where it is
> >> enabled).
> >>
> >> Assuming that the underlying bug is intractible
> > 
> > I think so, see <54B73623.9040503@cn.fujitsu.com>. I suppose one could
> > enter into a dialogue with Fedora about the practice of enabling these
> > flags for all Python modules built on a Fedora system vs. just those
> > built via RPM etc.
> > 
> >>  I think the right
> >> answer is for an affected developer
> > 
> > Which will be all developers using Fedora AFAICT.
> > 
> >>  to do one of the following, as a
> >> workaround: either, manually override Fortify when requesting a debug
> >> build (by setting EXTRA_CFLAGS_XEN_TOOLS), or manually override the
> >> -O0 setting.
> >>
> >> To make this easier to do without editing tools/Rules.mk I would
> >> support a patch to Rules.mk which has it honour a variable containing
> >> a debug-specific set of CFLAGS.
> 
> I don't understand your suggestion.

He means for the build system to honour a new
EXTRA_FLAGS_XEN_TOOLS_DEBUG (or similar) iff debug=y.

> > This seems reasonable enough to me.
> > 
> > The original patch in <54B73658.6030309@cn.fujitsu.com> (correctly IMHO)
> > applied the workaround only to the Python parts of the build
> > (tools/{python,pygrub}) whereas this v2 and your suggestion would affect
> > all of tools/*. That seems like a reasonable compromise under the
> > circumstances (the alternative being special overrides for Python or
> > something, no nice).
> 
> Building problems only affect developers, so I think we should fix this problem.
> 
> If the developers add extra flags to define the macro _FORTIFY_SOURCE, it also
> breaks building.

So can adding all manner of random macros to the build. If a developer
adds such flags then they should be prepared to deal with the
consequences, including dealing with any breakage which results.

The only reason we aren't taking this hard line with the situation you
find yourself in is that a major distro has seen fit to define these
extra macros into the default Python build system on that distro, hence
we are trying to find a reasonable compromise for users/developers of
that distro without making things worse for users/developers of other
distros which don't do this.

>  We can disable debug for such case, or undefine the macro
> in Rules.mk. If the macro _FORTIFY_SOURCE is not defined, undefining it is harmless.

On the other hand a user who has a fixed version of fortify which is
compatible with -O0 then cannot enable it if we do this.

Ian.

Re: [PATCH v2] Fix building error

[Wen Congyang]

On 01/20/2015 06:18 PM, Ian Campbell wrote:
> On Tue, 2015-01-20 at 10:21 +0800, Wen Congyang wrote:
>> On 01/19/2015 11:23 PM, Ian Campbell wrote:
>>> On Thu, 2015-01-15 at 11:26 +0000, Ian Jackson wrote:
>>>> Wen Congyang writes ("[PATCH v2] Fix building error"):
>>>>>  ifeq ($(debug),y)
>>>>>  # Disable optimizations and enable debugging information for macros
>>>>>  CFLAGS += -O0 -g3
>>>>> +# _FORTIFY_SOURCE requires compiling with optimization
>>>>> +CFLAGS += -Wp,-U_FORTIFY_SOURCE
>>>>
>>>> I'm not entirely convinced about this.  I have two kinds of concern:
>>>>
>>>> One is practical, which is that ATM AIUI a debug build, while not
>>>> intended for production use, is not any less secure.  (Leaving aside
>>>> the ability of guests to perform a DoS with copious debugging output.)
>>>>
>>>> The other is that we seem to be entering into a battle of escalation
>>>> between various Makefiles.  Specifically, this seems to be a
>>>> workaround for a bug in some other Makefiles we are using: the bug
>>>> being that these other Makefiles can't cope with -O0.  And
>>>> unconditionally squashing the other Makefiles' options seems like a
>>>> big hammer.
>>>>
>>>> The fact that Fortify doesn't support -O0 is a property of Fortify
>>>> which ought to be encoded in Fortify (or the places where it is
>>>> enabled).
>>>>
>>>> Assuming that the underlying bug is intractible
>>>
>>> I think so, see <54B73623.9040503@cn.fujitsu.com>. I suppose one could
>>> enter into a dialogue with Fedora about the practice of enabling these
>>> flags for all Python modules built on a Fedora system vs. just those
>>> built via RPM etc.
>>>
>>>>  I think the right
>>>> answer is for an affected developer
>>>
>>> Which will be all developers using Fedora AFAICT.
>>>
>>>>  to do one of the following, as a
>>>> workaround: either, manually override Fortify when requesting a debug
>>>> build (by setting EXTRA_CFLAGS_XEN_TOOLS), or manually override the
>>>> -O0 setting.
>>>>
>>>> To make this easier to do without editing tools/Rules.mk I would
>>>> support a patch to Rules.mk which has it honour a variable containing
>>>> a debug-specific set of CFLAGS.
>>
>> I don't understand your suggestion.
> 
> He means for the build system to honour a new
> EXTRA_FLAGS_XEN_TOOLS_DEBUG (or similar) iff debug=y.
> 
>>> This seems reasonable enough to me.
>>>
>>> The original patch in <54B73658.6030309@cn.fujitsu.com> (correctly IMHO)
>>> applied the workaround only to the Python parts of the build
>>> (tools/{python,pygrub}) whereas this v2 and your suggestion would affect
>>> all of tools/*. That seems like a reasonable compromise under the
>>> circumstances (the alternative being special overrides for Python or
>>> something, no nice).
>>
>> Building problems only affect developers, so I think we should fix this problem.
>>
>> If the developers add extra flags to define the macro _FORTIFY_SOURCE, it also
>> breaks building.
> 
> So can adding all manner of random macros to the build. If a developer
> adds such flags then they should be prepared to deal with the
> consequences, including dealing with any breakage which results.

Agree with it.

> 
> The only reason we aren't taking this hard line with the situation you
> find yourself in is that a major distro has seen fit to define these
> extra macros into the default Python build system on that distro, hence
> we are trying to find a reasonable compromise for users/developers of
> that distro without making things worse for users/developers of other
> distros which don't do this.
> 
>>  We can disable debug for such case, or undefine the macro
>> in Rules.mk. If the macro _FORTIFY_SOURCE is not defined, undefining it is harmless.
> 
> On the other hand a user who has a fixed version of fortify which is
> compatible with -O0 then cannot enable it if we do this.

Yes. We can detect it in the configure. And only undefine it when fortify cannot work with -O0

Thanks
Wen Congyang

> 
> Ian.
> 
> .
> 

Re: [PATCH v2] Fix building error

[Olaf Hering]

On Mon, Jan 19, Ian Campbell wrote:

> The original patch in <54B73658.6030309@cn.fujitsu.com> (correctly IMHO)
> applied the workaround only to the Python parts of the build
> (tools/{python,pygrub}) whereas this v2 and your suggestion would affect
> all of tools/*. That seems like a reasonable compromise under the
> circumstances (the alternative being special overrides for Python or
> something, no nice).

After looking again at this the original patch seems to be the correct
workaround.

Olaf

Re: [PATCH v2] Fix building error

[Julien Grall]

Hi,

On 19/01/15 15:23, Ian Campbell wrote:
> On Thu, 2015-01-15 at 11:26 +0000, Ian Jackson wrote:
>> Wen Congyang writes ("[PATCH v2] Fix building error"):
>>>  ifeq ($(debug),y)
>>>  # Disable optimizations and enable debugging information for macros
>>>  CFLAGS += -O0 -g3
>>> +# _FORTIFY_SOURCE requires compiling with optimization
>>> +CFLAGS += -Wp,-U_FORTIFY_SOURCE
>>
>> I'm not entirely convinced about this.  I have two kinds of concern:
>>
>> One is practical, which is that ATM AIUI a debug build, while not
>> intended for production use, is not any less secure.  (Leaving aside
>> the ability of guests to perform a DoS with copious debugging output.)
>>
>> The other is that we seem to be entering into a battle of escalation
>> between various Makefiles.  Specifically, this seems to be a
>> workaround for a bug in some other Makefiles we are using: the bug
>> being that these other Makefiles can't cope with -O0.  And
>> unconditionally squashing the other Makefiles' options seems like a
>> big hammer.
>>
>> The fact that Fortify doesn't support -O0 is a property of Fortify
>> which ought to be encoded in Fortify (or the places where it is
>> enabled).
>>
>> Assuming that the underlying bug is intractible
> 
> I think so, see <54B73623.9040503@cn.fujitsu.com>. I suppose one could
> enter into a dialogue with Fedora about the practice of enabling these
> flags for all Python modules built on a Fedora system vs. just those
> built via RPM etc.

FWIW, I get the same error when building tools on OpenSuse.

I would find annoying to manually override the CFLAGS everytime I have
to build Xen.

Regards,

-- 
Julien Grall

Re: [PATCH v2] Fix building error

[Jan Beulich]

>>> On 15.01.15 at 12:26, <Ian.Jackson@eu.citrix.com> wrote:
> Wen Congyang writes ("[PATCH v2] Fix building error"):
>>  ifeq ($(debug),y)
>>  # Disable optimizations and enable debugging information for macros
>>  CFLAGS += -O0 -g3
>> +# _FORTIFY_SOURCE requires compiling with optimization
>> +CFLAGS += -Wp,-U_FORTIFY_SOURCE
> 
> I'm not entirely convinced about this.  I have two kinds of concern:
> 
> One is practical, which is that ATM AIUI a debug build, while not
> intended for production use, is not any less secure.  (Leaving aside
> the ability of guests to perform a DoS with copious debugging output.)
> 
> The other is that we seem to be entering into a battle of escalation
> between various Makefiles.  Specifically, this seems to be a
> workaround for a bug in some other Makefiles we are using: the bug
> being that these other Makefiles can't cope with -O0.  And
> unconditionally squashing the other Makefiles' options seems like a
> big hammer.
> 
> The fact that Fortify doesn't support -O0 is a property of Fortify
> which ought to be encoded in Fortify (or the places where it is
> enabled).
> 
> Assuming that the underlying bug is intractible I think the right
> answer is for an affected developer to do one of the following, as a
> workaround: either, manually override Fortify when requesting a debug
> build (by setting EXTRA_CFLAGS_XEN_TOOLS), or manually override the
> -O0 setting.
> 
> To make this easier to do without editing tools/Rules.mk I would
> support a patch to Rules.mk which has it honour a variable containing
> a debug-specific set of CFLAGS.

Having run into this just now too, and seeing that the previous
discussion didn't really lead anywhere, I wonder what should be
done about this. I check as far back as I reasonably could, and
glibc apparently never supported _FORTIFY_SOURCE without
optimization. The change in behavior at some point simply was
that rather than disabling this silently, they now warn about it
(which of course is fatal for a build with -Werror). I also checked
Python, and they also seem to have been enabling
_FORTIFY_SOURCE forever. Consequently, with the previously
suggested patches not having found acceptance, how about

--- unstable.orig/tools/pygrub/Makefile
+++ unstable/tools/pygrub/Makefile
@@ -2,15 +2,17 @@
 XEN_ROOT = $(CURDIR)/../..
 include $(XEN_ROOT)/tools/Rules.mk
 
+PY_CFLAGS = $(patsubst -O0,-O1,$(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 \
--- unstable.orig/tools/python/Makefile
+++ unstable/tools/python/Makefile
@@ -4,6 +4,8 @@ include $(XEN_ROOT)/tools/Rules.mk
 .PHONY: all
 all: build
 
+PY_CFLAGS = $(patsubst -O0,-O1,$(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/libx
 		$(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

Jan

Re: [PATCH v2] Fix building error

[Ian Jackson]

Jan Beulich writes ("Re: [Xen-devel] [PATCH v2] Fix building error"):
> On 15.01.15 at 12:26, <Ian.Jackson@eu.citrix.com> wrote:
> > Assuming that the underlying bug is intractible I think the right
> > answer is for an affected developer to do one of the following, as a
> > workaround: either, manually override Fortify when requesting a debug
> > build (by setting EXTRA_CFLAGS_XEN_TOOLS), or manually override the
> > -O0 setting.
...
> Having run into this just now too, and seeing that the previous
> discussion didn't really lead anywhere, I wonder what should be
> done about this. I check as far back as I reasonably could, and
> glibc apparently never supported _FORTIFY_SOURCE without
> optimization. The change in behavior at some point simply was
> that rather than disabling this silently, they now warn about it
> (which of course is fatal for a build with -Werror). I also checked
> Python, and they also seem to have been enabling
> _FORTIFY_SOURCE forever. Consequently, with the previously
> suggested patches not having found acceptance, how about
...
> +PY_CFLAGS = $(patsubst -O0,-O1,$(CFLAGS)) $(APPEND_LDFLAGS)

This is still bad.  What if the user explicitly wants to build without
optimisation ?  That's not unusual.  And it's only the combination of
-O0 and FORTIFY that fails.  (And it's only on broken platforms that
FORTIFY appears by default.)

I guess I would tolerate a patch which spots the combination of
_FORTIFY_SOURCE and -O0 and only in that case changes -O1 to -O0.

Ian.

Re: [PATCH v2] Fix building error

[Jan Beulich]

>>> On 04.02.15 at 16:43, <Ian.Jackson@eu.citrix.com> wrote:
> Jan Beulich writes ("Re: [Xen-devel] [PATCH v2] Fix building error"):
>> On 15.01.15 at 12:26, <Ian.Jackson@eu.citrix.com> wrote:
>> > Assuming that the underlying bug is intractible I think the right
>> > answer is for an affected developer to do one of the following, as a
>> > workaround: either, manually override Fortify when requesting a debug
>> > build (by setting EXTRA_CFLAGS_XEN_TOOLS), or manually override the
>> > -O0 setting.
> ...
>> Having run into this just now too, and seeing that the previous
>> discussion didn't really lead anywhere, I wonder what should be
>> done about this. I check as far back as I reasonably could, and
>> glibc apparently never supported _FORTIFY_SOURCE without
>> optimization. The change in behavior at some point simply was
>> that rather than disabling this silently, they now warn about it
>> (which of course is fatal for a build with -Werror). I also checked
>> Python, and they also seem to have been enabling
>> _FORTIFY_SOURCE forever. Consequently, with the previously
>> suggested patches not having found acceptance, how about
> ...
>> +PY_CFLAGS = $(patsubst -O0,-O1,$(CFLAGS)) $(APPEND_LDFLAGS)
> 
> This is still bad.  What if the user explicitly wants to build without
> optimisation ?  That's not unusual.  And it's only the combination of
> -O0 and FORTIFY that fails.  (And it's only on broken platforms that
> FORTIFY appears by default.)

Broken platforms? (Of course I ask without having any idea where
python-config takes that -D_FORTIFY_SOURCE=2 [together with
everything else it sets] from.)

Also I think overriding -O<n> here is no worse that Python adding
-DNDEBUG regardless of us doing a debug build. (Besides they
actually do pass -O2 too, just that our additions override this.)

> I guess I would tolerate a patch which spots the combination of
> _FORTIFY_SOURCE and -O0 and only in that case changes -O1 to -O0.

But that would have to happen in (I guess) setup.py, unless we want
to invoke "python-config --cflags" from the Makefile to find out. And
I don't see myself touching any Python code...

Jan

Re: [PATCH v2] Fix building error

[Ian Jackson]

Jan Beulich writes ("Re: [Xen-devel] [PATCH v2] Fix building error"):
> On 04.02.15 at 16:43, <Ian.Jackson@eu.citrix.com> wrote:
> > This is still bad.  What if the user explicitly wants to build without
> > optimisation ?  That's not unusual.  And it's only the combination of
> > -O0 and FORTIFY that fails.  (And it's only on broken platforms that
> > FORTIFY appears by default.)
> 
> Broken platforms? (Of course I ask without having any idea where
> python-config takes that -D_FORTIFY_SOURCE=2 [together with
> everything else it sets] from.)

IMO a platform where the standard Python build tools produce compiler
options incompatible with -O0 is broken.

> Also I think overriding -O<n> here is no worse that Python adding
> -DNDEBUG regardless of us doing a debug build.

What???  I'm just not going there.

> > I guess I would tolerate a patch which spots the combination of
> > _FORTIFY_SOURCE and -O0 and only in that case changes -O1 to -O0.
> 
> But that would have to happen in (I guess) setup.py, unless we want
> to invoke "python-config --cflags" from the Makefile to find out. And
> I don't see myself touching any Python code...

I don't mind some kind of configure check for this particular
brokenness.  python-config --cflags seems like it would be a fine way
to detect it.

Ian.

Re: [PATCH v2] Fix building error

[Olaf Hering]

On Wed, Feb 04, Jan Beulich wrote:

> I also checked Python, and they also seem to have been enabling
> _FORTIFY_SOURCE forever. Consequently, with the previously suggested
> patches not having found acceptance, how about

I'm sure python just carries the CFLAGS used for its build, so that
other python related code can use the very same CFLAGS. IMO that is
valid. Since the whole distro is built with those CFLAGS via
RPM_OPT_FLAGS it will end up also in python.

I wonder, is it just SUSE and Fedora that use _FORTIFY_SOURCE in global
CFLAGS?

Olaf