[Qemu-trivial] [PATCH] configure: Undefine _FORTIFY_SOURCE prior using it

[Qemu-trivial] [PATCH] configure: Undefine _FORTIFY_SOURCE prior using it

[Michal Privoznik]

Currently, we are enforcing the _FORTIFY_SOURCE=2 without any
previous detection if the macro has been already defined, e.g.
by environment, or is just enabled by compiler by default.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index b7635e4..97070eb 100755
--- a/configure
+++ b/configure
@@ -3159,7 +3159,7 @@ if test "$gcov" = "yes" ; then
   CFLAGS="-fprofile-arcs -ftest-coverage -g $CFLAGS"
   LDFLAGS="-fprofile-arcs -ftest-coverage $LDFLAGS"
 elif test "$debug" = "no" ; then
-  CFLAGS="-O2 -D_FORTIFY_SOURCE=2 $CFLAGS"
+  CFLAGS="-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 $CFLAGS"
 fi
 
 
-- 
1.8.0.2

Re: [Qemu-trivial] [Qemu-devel] [PATCH] configure: Undefine _FORTIFY_SOURCE prior using it

[Andreas Färber]

Am 31.01.2013 11:15, schrieb Michal Privoznik:
> Currently, we are enforcing the _FORTIFY_SOURCE=2 without any
> previous detection if the macro has been already defined, e.g.
> by environment, or is just enabled by compiler by default.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  configure | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index b7635e4..97070eb 100755
> --- a/configure
> +++ b/configure
> @@ -3159,7 +3159,7 @@ if test "$gcov" = "yes" ; then
>    CFLAGS="-fprofile-arcs -ftest-coverage -g $CFLAGS"
>    LDFLAGS="-fprofile-arcs -ftest-coverage $LDFLAGS"
>  elif test "$debug" = "no" ; then
> -  CFLAGS="-O2 -D_FORTIFY_SOURCE=2 $CFLAGS"
> +  CFLAGS="-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 $CFLAGS"
>  fi
>  
>  

Should we maybe instead add a compile-test?

#ifdef _FORTIFY_SOURCE
#if _FORTIFY_SOURCE >= 2
#error Environment already has _FORTIFY_SOURCE
#endif
#endif

I admit I have no clue what the number means and whether there are more
fortified levels.

Cheers,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-trivial] [Qemu-devel] [PATCH] configure: Undefine _FORTIFY_SOURCE prior using it

[Michal Privoznik]

On 01.02.2013 10:54, Andreas Färber wrote:
> Am 31.01.2013 11:15, schrieb Michal Privoznik:
>> Currently, we are enforcing the _FORTIFY_SOURCE=2 without any
>> previous detection if the macro has been already defined, e.g.
>> by environment, or is just enabled by compiler by default.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  configure | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/configure b/configure
>> index b7635e4..97070eb 100755
>> --- a/configure
>> +++ b/configure
>> @@ -3159,7 +3159,7 @@ if test "$gcov" = "yes" ; then
>>    CFLAGS="-fprofile-arcs -ftest-coverage -g $CFLAGS"
>>    LDFLAGS="-fprofile-arcs -ftest-coverage $LDFLAGS"
>>  elif test "$debug" = "no" ; then
>> -  CFLAGS="-O2 -D_FORTIFY_SOURCE=2 $CFLAGS"
>> +  CFLAGS="-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 $CFLAGS"
>>  fi
>>  
>>  
> 
> Should we maybe instead add a compile-test?
> 
> #ifdef _FORTIFY_SOURCE
> #if _FORTIFY_SOURCE >= 2
> #error Environment already has _FORTIFY_SOURCE
> #endif
> #endif
> 
> I admit I have no clue what the number means and whether there are more
> fortified levels.
> 
> Cheers,
> Andreas
> 

I don't think that's necessary. The 2nd level is the highest one [1] or [2].
It seems like in my case it's compiler who's defining the macro:

$ echo "int main() {return 0;}" | gcc -D_FORTIFY_SOURCE=2 -x c -
<command-line>:0:0: warning: "_FORTIFY_SOURCE" redefined [enabled by default]
<stdin>:1:0: note: this is the location of the previous definition

in which case we must undefine it. However, if the _FORTIFY_SOURCE is
defined by environment, I think we should not override it. So maybe need
a different approach. Anyway, with current state I cannot compile. I am using
gcc version 4.7.2 (Gentoo 4.7.2 p1.3, pie-0.5.5)

Michal

1: http://gcc.gnu.org/ml/gcc-patches/2004-09/msg02055.html
2: http://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libssp/ssp/ssp.h.in

Re: [Qemu-trivial] [Qemu-devel] [PATCH] configure: Undefine _FORTIFY_SOURCE prior using it

[Stefan Hajnoczi]

On Fri, Feb 01, 2013 at 04:18:23PM +0100, Michal Privoznik wrote:
> On 01.02.2013 10:54, Andreas Färber wrote:
> > Am 31.01.2013 11:15, schrieb Michal Privoznik:
> >> Currently, we are enforcing the _FORTIFY_SOURCE=2 without any
> >> previous detection if the macro has been already defined, e.g.
> >> by environment, or is just enabled by compiler by default.
> >>
> >> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> >> ---
> >>  configure | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/configure b/configure
> >> index b7635e4..97070eb 100755
> >> --- a/configure
> >> +++ b/configure
> >> @@ -3159,7 +3159,7 @@ if test "$gcov" = "yes" ; then
> >>    CFLAGS="-fprofile-arcs -ftest-coverage -g $CFLAGS"
> >>    LDFLAGS="-fprofile-arcs -ftest-coverage $LDFLAGS"
> >>  elif test "$debug" = "no" ; then
> >> -  CFLAGS="-O2 -D_FORTIFY_SOURCE=2 $CFLAGS"
> >> +  CFLAGS="-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 $CFLAGS"
> >>  fi
> >>  
> >>  
> > 
> > Should we maybe instead add a compile-test?
> > 
> > #ifdef _FORTIFY_SOURCE
> > #if _FORTIFY_SOURCE >= 2
> > #error Environment already has _FORTIFY_SOURCE
> > #endif
> > #endif
> > 
> > I admit I have no clue what the number means and whether there are more
> > fortified levels.
> > 
> > Cheers,
> > Andreas
> > 
> 
> I don't think that's necessary. The 2nd level is the highest one [1] or [2].
> It seems like in my case it's compiler who's defining the macro:
> 
> $ echo "int main() {return 0;}" | gcc -D_FORTIFY_SOURCE=2 -x c -
> <command-line>:0:0: warning: "_FORTIFY_SOURCE" redefined [enabled by default]
> <stdin>:1:0: note: this is the location of the previous definition
> 
> in which case we must undefine it. However, if the _FORTIFY_SOURCE is
> defined by environment, I think we should not override it. So maybe need
> a different approach. Anyway, with current state I cannot compile. I am using
> gcc version 4.7.2 (Gentoo 4.7.2 p1.3, pie-0.5.5)

It would be nice to fix this for QEMU 1.4 but my gcc FORTIFY_SOURCE foo
is not strong enough to know what the best approach is here.

Reviews from anyone else?

Stefan

Re: [Qemu-trivial] [Qemu-devel] [PATCH] configure: Undefine _FORTIFY_SOURCE prior using it

[Paolo Bonzini]

Il 06/02/2013 15:49, Stefan Hajnoczi ha scritto:
>> > I don't think that's necessary. The 2nd level is the highest one [1] or [2].
>> > It seems like in my case it's compiler who's defining the macro:
>> > 
>> > $ echo "int main() {return 0;}" | gcc -D_FORTIFY_SOURCE=2 -x c -
>> > <command-line>:0:0: warning: "_FORTIFY_SOURCE" redefined [enabled by default]
>> > <stdin>:1:0: note: this is the location of the previous definition
>> > 
>> > in which case we must undefine it. However, if the _FORTIFY_SOURCE is
>> > defined by environment, I think we should not override it. So maybe need
>> > a different approach. Anyway, with current state I cannot compile. I am using
>> > gcc version 4.7.2 (Gentoo 4.7.2 p1.3, pie-0.5.5)
> It would be nice to fix this for QEMU 1.4 but my gcc FORTIFY_SOURCE foo
> is not strong enough to know what the best approach is here.
> 
> Reviews from anyone else?

I would prefer to avoid having _FORTIFY_SOURCE completely, and let
distros do it.  Alternatively, tie it to a new --enable switch which
would do -U -D.

But if there is a real problem for 1.4 the patch looks good.

Paolo

Re: [Qemu-trivial] [Qemu-devel] [PATCH] configure: Undefine _FORTIFY_SOURCE prior using it

[Richard Henderson]

On 2013-02-06 06:49, Stefan Hajnoczi wrote:
>> $ echo "int main() {return 0;}" | gcc -D_FORTIFY_SOURCE=2 -x c -
>> <command-line>:0:0: warning: "_FORTIFY_SOURCE" redefined [enabled by default]
>> <stdin>:1:0: note: this is the location of the previous definition
>>
>> in which case we must undefine it. However, if the _FORTIFY_SOURCE is
>> defined by environment, I think we should not override it. So maybe need
>> a different approach. Anyway, with current state I cannot compile. I am using
>> gcc version 4.7.2 (Gentoo 4.7.2 p1.3, pie-0.5.5)
>
> It would be nice to fix this for QEMU 1.4 but my gcc FORTIFY_SOURCE foo
> is not strong enough to know what the best approach is here.

Using -U -D is just fine to override something that the distro turned on 
by default.


r~

Re: [Qemu-trivial] [Qemu-devel] [PATCH] configure: Undefine _FORTIFY_SOURCE prior using it

[Michal Privoznik]

On 06.02.2013 16:09, Paolo Bonzini wrote:
> Il 06/02/2013 15:49, Stefan Hajnoczi ha scritto:
>>>> I don't think that's necessary. The 2nd level is the highest one [1] or [2].
>>>> It seems like in my case it's compiler who's defining the macro:
>>>>
>>>> $ echo "int main() {return 0;}" | gcc -D_FORTIFY_SOURCE=2 -x c -
>>>> <command-line>:0:0: warning: "_FORTIFY_SOURCE" redefined [enabled by default]
>>>> <stdin>:1:0: note: this is the location of the previous definition
>>>>
>>>> in which case we must undefine it. However, if the _FORTIFY_SOURCE is
>>>> defined by environment, I think we should not override it. So maybe need
>>>> a different approach. Anyway, with current state I cannot compile. I am using
>>>> gcc version 4.7.2 (Gentoo 4.7.2 p1.3, pie-0.5.5)
>> It would be nice to fix this for QEMU 1.4 but my gcc FORTIFY_SOURCE foo
>> is not strong enough to know what the best approach is here.
>>
>> Reviews from anyone else?
> 
> I would prefer to avoid having _FORTIFY_SOURCE completely, and let
> distros do it.  Alternatively, tie it to a new --enable switch which
> would do -U -D.
> 
> But if there is a real problem for 1.4 the patch looks good.
> 
> Paolo
> 

Sorry for resurrecting such old thread, but what's the conclusion then?
I keep hitting this problem and I am tired of having one single patch on
the top of HEAD. And I bet others ran into this as well.

Michal

Re: [Qemu-trivial] [Qemu-devel] [PATCH] configure: Undefine _FORTIFY_SOURCE prior using it

[Markus Armbruster]

Michal Privoznik <mprivozn@redhat.com> writes:

> On 06.02.2013 16:09, Paolo Bonzini wrote:
>> Il 06/02/2013 15:49, Stefan Hajnoczi ha scritto:
>>>>> I don't think that's necessary. The 2nd level is the highest one
>>>>> [1] or [2].
>>>>> It seems like in my case it's compiler who's defining the macro:
>>>>>
>>>>> $ echo "int main() {return 0;}" | gcc -D_FORTIFY_SOURCE=2 -x c -
>>>>> <command-line>:0:0: warning: "_FORTIFY_SOURCE" redefined [enabled
>>>>> by default]
>>>>> <stdin>:1:0: note: this is the location of the previous definition
>>>>>
>>>>> in which case we must undefine it. However, if the _FORTIFY_SOURCE is
>>>>> defined by environment, I think we should not override it. So maybe need
>>>>> a different approach. Anyway, with current state I cannot
>>>>> compile. I am using
>>>>> gcc version 4.7.2 (Gentoo 4.7.2 p1.3, pie-0.5.5)
>>> It would be nice to fix this for QEMU 1.4 but my gcc FORTIFY_SOURCE foo
>>> is not strong enough to know what the best approach is here.
>>>
>>> Reviews from anyone else?
>> 
>> I would prefer to avoid having _FORTIFY_SOURCE completely, and let
>> distros do it.  Alternatively, tie it to a new --enable switch which
>> would do -U -D.
>> 
>> But if there is a real problem for 1.4 the patch looks good.
>> 
>> Paolo
>> 
>
> Sorry for resurrecting such old thread, but what's the conclusion then?

Thread petered out without a conclusion?

> I keep hitting this problem and I am tired of having one single patch on
> the top of HEAD. And I bet others ran into this as well.

Few things focus discussions as well as a patch does.  Suggest to post
one to get things going again.