diff mbox

gcc fstack-protector-explicit

Message ID yddlhkwo62l.fsf@CeBiTec.Uni-Bielefeld.DE
State New
Headers show

Commit Message

Rainer Orth Jan. 21, 2015, 9:07 a.m. UTC
Jeff Law <law@redhat.com> writes:

> On 07/01/14 15:34, Daniel Gutson wrote:
>> On Tue, Jul 1, 2014 at 2:25 PM, Jeff Law <law@redhat.com> wrote:
>>> On 03/19/14 08:06, Marcos Díaz wrote:
>>>>
>>>> Well, finally I have the assignment, could you please review this patch?
>>>
>>> Thanks.
>>>
>>> My first thought was that if we've marked the function with an explicit
>>> static protector attribute, then it ought to be protected regardless of any
>>> flags.  Is there some reason to require the -fstack-protect-explicit?
>>
>> They can work separately, since the logic is:
>>
>> if NOT stack-protect-explicit
>>     a function can be protected by the current logic OR it has the attribute
>>     (a function may be not automatically protected with the current logic)
>> ELSE // stack-protect-explicit
>>     only functions marked with the attribute will be protected.
>>
>> IOW, when no stack-protect-explicit, the functions may not be
>> protected due to current logic, so the attribute acts as an override
>> to request protection.
> Sorry this took so long.  I fixed a variety of whitespace errors, wrote a
> better ChangeLog, re-bootstrapped and regression tested the patch (given
> the long delay, I felt it was the least I could do).  Approved and
> installed.

Unfortunately, the new gcc.dg/stackprotectexplicit1.c FAILs on Solaris
(both SPARC and x86):

FAIL: gcc.dg/stackprotectexplicit1.c (test for excess errors)
WARNING: gcc.dg/stackprotectexplicit1.c compilation failed to produce executable

Excess errors:
Undefined                       first referenced
 symbol                             in file
__stack_chk_guard                   /var/tmp//ccv9aOr1.o
ld: fatal: symbol referencing errors. No output written to .

This doesn't occur on Linux since that defines TARGET_THREAD_SSP_OFFSET,
while Solaris (and doubtlessly other targets) need to link with -lssp to
get a definition of __stack_chk_guard.

The following patch does just that.  Not yet tested because currently
trunk doesn't bootstrap (libstdc++.so link failure).

Ok for mainline once that has been done?

Thanks.
	Rainer


2015-01-20  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	* gcc.c (LINK_SSP_SPEC): Handle -fstack-protector-explicit.

Comments

Jakub Jelinek Jan. 22, 2015, 12:37 p.m. UTC | #1
On Wed, Jan 21, 2015 at 10:07:14AM +0100, Rainer Orth wrote:
> Ok for mainline once that has been done?
> 
> Thanks.
> 	Rainer
> 
> 
> 2015-01-20  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>
> 
> 	* gcc.c (LINK_SSP_SPEC): Handle -fstack-protector-explicit.

Ok.
Though wonder if for the TARGET_LIBC_PROVIDES_SSP case LINK_SSP_SPEC
shouldn't be
#define LINK_SSP_SPEC "{fstack-protector|fstack-protector-strong|fstack-protector-explicit|fstack-protector-all:}"
and
gcc/config/freebsd.h:
#define LINK_SSP_SPEC "%{fstack-protector|fstack-protector-all:-lssp_nonshared}"
should be changed too (adding both -string and -explicit).

> # HG changeset patch
> # Parent 32ee1d2fb4ac6498d6363a1841482f8c9fa521d7
> Handle -fstack-protector-explicit in LINK_SSP_SPEC
> 
> diff --git a/gcc/gcc.c b/gcc/gcc.c
> --- a/gcc/gcc.c
> +++ b/gcc/gcc.c
> @@ -730,7 +730,7 @@ proper position among the other output f
>  #ifdef TARGET_LIBC_PROVIDES_SSP
>  #define LINK_SSP_SPEC "%{fstack-protector:}"
>  #else
> -#define LINK_SSP_SPEC "%{fstack-protector|fstack-protector-strong|fstack-protector-all:-lssp_nonshared -lssp}"
> +#define LINK_SSP_SPEC "%{fstack-protector|fstack-protector-strong|fstack-protector-explicit|fstack-protector-all:-lssp_nonshared -lssp}"
>  #endif
>  #endif
>  

	Jakub
Jeff Law Jan. 22, 2015, 4:35 p.m. UTC | #2
On 01/22/15 05:37, Jakub Jelinek wrote:
> On Wed, Jan 21, 2015 at 10:07:14AM +0100, Rainer Orth wrote:
>> Ok for mainline once that has been done?
>>
>> Thanks.
>> 	Rainer
>>
>>
>> 2015-01-20  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>
>>
>> 	* gcc.c (LINK_SSP_SPEC): Handle -fstack-protector-explicit.
>
> Ok.
> Though wonder if for the TARGET_LIBC_PROVIDES_SSP case LINK_SSP_SPEC
> shouldn't be
> #define LINK_SSP_SPEC "{fstack-protector|fstack-protector-strong|fstack-protector-explicit|fstack-protector-all:}"
> and
> gcc/config/freebsd.h:
> #define LINK_SSP_SPEC "%{fstack-protector|fstack-protector-all:-lssp_nonshared}"
> should be changed too (adding both -string and -explicit).
Ranier, sorry about the breakage on Solaris.

WRT other LINK_SPECs, yea, they all need to check the 4 variants of 
-fstack-protector-whatever and if any are found, link in libssp.

Patch to fix that pre-approved.

jeff
diff mbox

Patch

# HG changeset patch
# Parent 32ee1d2fb4ac6498d6363a1841482f8c9fa521d7
Handle -fstack-protector-explicit in LINK_SSP_SPEC

diff --git a/gcc/gcc.c b/gcc/gcc.c
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -730,7 +730,7 @@  proper position among the other output f
 #ifdef TARGET_LIBC_PROVIDES_SSP
 #define LINK_SSP_SPEC "%{fstack-protector:}"
 #else
-#define LINK_SSP_SPEC "%{fstack-protector|fstack-protector-strong|fstack-protector-all:-lssp_nonshared -lssp}"
+#define LINK_SSP_SPEC "%{fstack-protector|fstack-protector-strong|fstack-protector-explicit|fstack-protector-all:-lssp_nonshared -lssp}"
 #endif
 #endif