[libvirt] [PATCH] Remove ssp buffer size setting

[Ján Tomko]

On 06/06/2014 01:03 PM, Daniel P. Berrange wrote:
> On Fri, Jun 06, 2014 at 01:00:20PM +0200, Martin Kletzander wrote:
>> On Fri, Jun 06, 2014 at 11:40:24AM +0200, Ján Tomko wrote:
>>> This option only makes sense with -fstack-protector.
>>> With -fstack-protector-all, even functions with buffers
>>> smaller than this are protected.
>>>
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1105456
>>> ---
>>> m4/virt-compile-warnings.m4 | 8 --------
>>> 1 file changed, 8 deletions(-)
>>>
>>> diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4
>>> index 574fbc4..ebc931d 100644
>>> --- a/m4/virt-compile-warnings.m4
>>> +++ b/m4/virt-compile-warnings.m4
>>> @@ -171,14 +171,6 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[
>>>       dnl be great overhead in adding -fstack-protector-all instead
>>>       dnl wantwarn="$wantwarn -fstack-protector"
>>>       wantwarn="$wantwarn -fstack-protector-all"
>>> -       wantwarn="$wantwarn --param=ssp-buffer-size=4"
>>
>> It would be nice to keep that line in here with the explanation that
>> -fstack-protector-all does not make use of that param.

On second thought, the buffer size makes sense for -fstack-protector,
so I guess it should stay unless we remove '-fstack-protector' as well.

>>
>>> -       dnl Even though it supports it, clang complains about
>>> -       dnl use of --param=ssp-buffer-size=4 unless used with
>>> -       dnl the -c arg. It doesn't like it when used with args
>>> -       dnl that just link together .o files. Unfortunately
>>> -       dnl we can't avoid that with automake, so we must turn
>>> -       dnl off the following clang specific warning
>>> -       wantwarn="$wantwarn -Wno-unused-command-line-argument"
>>
>> Why do you also remove this line?

This warning supression was only added because of the --param flag,
which I was removing (see the comment above it).

>>
>>>       ;;
>>>       *-*-freebsd*)
>>>       dnl FreeBSD ships old gcc 4.2.1 which doesn't handle
>>
>> Also, out of the context of this patch, doesn't that param need to be
>> added to the freebsd version since it uses -fstack-protector only?

I can't do any proper testing on FreeBSD, maybe it would work better than
stack-protector-all with 4.2.1:
http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=cc7cd623

> 
> 
> Ideally we should actually use  -fstack-protector-strong if we find
> it supported, in preference to -fstack-protector-all.

That could work, if the GCC version shipped on FreeBSD doesn't have it broken.
I can send a patch enabling it for Linux after I upgrade my compiler.

> The strong variant would still require us to set ssp-buffer-size.

Not really, gcc only uses it to tell small and large arrays apart:
https://gcc.gnu.org/viewcvs/gcc/trunk/gcc/cfgexpand.c?revision=211306&view=markup#l1391

and then treats them the same for stack-protector-all and -strong:
https://gcc.gnu.org/viewcvs/gcc/trunk/gcc/cfgexpand.c?revision=211306&view=markup#l1439

Jan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140606/e3576ea3/attachment-0001.sig>