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

Re: [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-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-devel] [Qemu-trivial] [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-devel] [Qemu-trivial] [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.