[patch] gcc fstack-protector-explicit

[patch] gcc fstack-protector-explicit

[Marcos Díaz]

[-- Attachment #1: Type: text/plain, Size: 2514 bytes --]

Hi,
   the attached patch adds a new attribute and option flag to control
when to do stack protection.
The new attribute (stack_protect)  affects the behavior of gcc by
forcing the stack protection of the function marked with the attribute
if any of the options -fstack-protector, -fstack-protector-strong or
-fstack-protector-explicit(new) are set.

If the new option (-fstack-protector-explicit) is set only those
functions with the attribute stack_protect will have stack protection.

The stack re-ordering of the variables depends on the option set,
currently if flag -fstack-protector is set only char arrays are
reordered in the stack, whereas if flag -fstack-protector-strong or
-fstack-protector-explicit is set then char arrays and other arrays
are ordered first in the stack.
About this reordering of the non char arrays, shouldn't all to-be
protected functions have the full re-ordering? If not, for
completeness, I should make that new flag -fstack-protector-explicit
not to order the non-char arrays, and create a new -strong
counterpart, namely -fstack-protector-explicit-strong which does.

Additionally, I think that the behavior of the flag
-fstack-protector-strong lacked the re-ordering of non char arrays
(named phase 2) so I added the reordering also for such flag.
Current tests pass after applying this patch, plus the tests specially added.
Please commit it for me if OK since I don't have write access.

Changelog:
2013-11-18 Marcos Diaz <marcos.diaz@tallertechnologies.com>

   gcc/
   * c-family/c-cppbuiltin.c (c_cpp_builtins): New cpp define
__SSP_EXPLICIT__ for the new option fstack-protector_explicit
   * c-family/c-common.c:  Added new attribute stack-protect and
respective handler
   * common.opt: Added new option -fstack-protector-explicit
   * cfgexpand.c: Added new enum SPCT_FLAG_EXPLICIT
         (stack_protect_decl_phase):    Added new condition in the
look of conflictive variables for the new attribute and option.
         (expand_used_vars): Added new condition in order to put stack
protection in functions with the stack protect attribute.

   gcc/doc/
   * cpp.texi: New description for the new cpp_define __SSP_EXPLICIT__
   * extend.texi: Added description for the new stack_protect function
attribute.
   * invoke.texi:  description for the new -fstack-protector-strong option

   gcc/testsuite/
   * gcc.dg/stackprotectexplicit1.c: New test file for the flag and
the attribute
   * g++.dg/stackprotectexplicit2.C: New test file for the flag and
the attribute

[-- Attachment #2: fspexplicit.patch --]
[-- Type: text/x-patch, Size: 10424 bytes --]

Index: gcc/testsuite/gcc.dg/stackprotectexplicit1.c
===================================================================
--- gcc/testsuite/gcc.dg/stackprotectexplicit1.c	(revision 0)
+++ gcc/testsuite/gcc.dg/stackprotectexplicit1.c	(revision 0)
@@ -0,0 +1,23 @@
+/* { dg-do run { target native } } */
+/* { dg-options "-fstack-protector-explicit" } */
+/* { dg-require-effective-target fstack_protector } */
+
+#include <stdlib.h>
+
+void
+__stack_chk_fail (void)
+{
+  exit (0); /* pass */
+}
+
+int __attribute__((stack_protect)) main ()
+{
+  int i;
+  char foo[255];
+
+  // smash stack
+  for (i = 0; i <= 400; i++)
+    foo[i] = 42;
+
+  return 1; /* fail */
+}
Index: gcc/testsuite/g++.dg/stackprotectexplicit2.C
===================================================================
--- gcc/testsuite/g++.dg/stackprotectexplicit2.C	(revision 0)
+++ gcc/testsuite/g++.dg/stackprotectexplicit2.C	(revision 0)
@@ -0,0 +1,27 @@
+/* Test that stack protection is done on chosen functions. */
+
+/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
+/* { dg-options "-O2 -fstack-protector-explicit" } */
+
+int A()
+{
+	int A[23];
+	char b[22];
+}
+
+int __attribute__((stack_protect)) B()
+{
+	int a;
+	int b;
+	return a+b;
+}
+
+int __attribute__((stack_protect)) c()
+{
+	int a;
+	char b[34];
+	return 0;
+}
+
+
+/* { dg-final { scan-assembler-times "stack_chk_fail" 2 } } */
\ No newline at end of file
Index: gcc/common.opt
===================================================================
--- gcc/common.opt	(revision 204949)
+++ gcc/common.opt	(working copy)
@@ -1958,6 +1958,10 @@ fstack-protector-strong
 Common Report RejectNegative Var(flag_stack_protect, 3)
 Use a smart stack protection method for certain functions
 
+fstack-protector-explicit
+Common Report RejectNegative Var(flag_stack_protect, 4)
+Use stack protection method only for functions with the stack_protect attribute
+
 fstack-usage
 Common RejectNegative Var(flag_stack_usage)
 Output stack usage information on a per-function basis
Index: gcc/c-family/c-cppbuiltin.c
===================================================================
--- gcc/c-family/c-cppbuiltin.c	(revision 204949)
+++ gcc/c-family/c-cppbuiltin.c	(working copy)
@@ -984,6 +984,8 @@ c_cpp_builtins (cpp_reader *pfile)
   /* Make the choice of the stack protector runtime visible to source code.
      The macro names and values here were chosen for compatibility with an
      earlier implementation, i.e. ProPolice.  */
+  if (flag_stack_protect == 4)
+    cpp_define (pfile, "__SSP_EXPLICIT__=4");
   if (flag_stack_protect == 3)
     cpp_define (pfile, "__SSP_STRONG__=3");
   if (flag_stack_protect == 2)
Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c	(revision 204949)
+++ gcc/c-family/c-common.c	(working copy)
@@ -314,6 +314,7 @@ static tree handle_no_address_safety_ana
 							 int, bool *);
 static tree handle_no_sanitize_undefined_attribute (tree *, tree, tree, int,
 						    bool *);
+static tree handle_stack_protect_attribute (tree *, tree, tree, int, bool *);
 static tree handle_noinline_attribute (tree *, tree, tree, int, bool *);
 static tree handle_noclone_attribute (tree *, tree, tree, int, bool *);
 static tree handle_leaf_attribute (tree *, tree, tree, int, bool *);
@@ -629,6 +630,8 @@ const struct attribute_spec c_common_att
 			      handle_noreturn_attribute, false },
   { "volatile",               0, 0, true,  false, false,
 			      handle_noreturn_attribute, false },
+  { "stack_protect",          0, 0, true,  false, false,
+			      handle_stack_protect_attribute, false },
   { "noinline",               0, 0, true,  false, false,
 			      handle_noinline_attribute, false },
   { "noclone",                0, 0, true,  false, false,
@@ -6624,6 +6627,25 @@ handle_no_sanitize_undefined_attribute (
 
   return NULL_TREE;
 }
+
+/* Handle a "stack_protect" attribute; arguments as in
+   struct attribute_spec.handler.  */
+static tree
+handle_stack_protect_attribute (tree *node, tree name, tree, int,
+				       bool *no_add_attrs)
+{
+  if (TREE_CODE (*node) != FUNCTION_DECL)
+    {
+      warning (OPT_Wattributes, "%qE attribute ignored", name);
+      *no_add_attrs = true;
+    }
+  else
+    DECL_ATTRIBUTES (*node) 
+      = tree_cons (get_identifier ("stack_protect"),
+      NULL_TREE, DECL_ATTRIBUTES (*node));
+
+  return NULL_TREE;
+}
 
 /* Handle a "noinline" attribute; arguments as in
    struct attribute_spec.handler.  */
Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c	(revision 204949)
+++ gcc/cfgexpand.c	(working copy)
@@ -1330,7 +1330,8 @@ clear_tree_used (tree block)
 enum {
   SPCT_FLAG_DEFAULT = 1,
   SPCT_FLAG_ALL = 2,
-  SPCT_FLAG_STRONG = 3
+  SPCT_FLAG_STRONG = 3,
+  SPCT_FLAG_EXPLICIT = 4
 };
 
 /* Examine TYPE and determine a bit mask of the following features.  */
@@ -1403,7 +1404,9 @@ stack_protect_decl_phase (tree decl)
     has_short_buffer = true;
 
   if (flag_stack_protect == SPCT_FLAG_ALL
-      || flag_stack_protect == SPCT_FLAG_STRONG)
+      || flag_stack_protect == SPCT_FLAG_STRONG
+      || (flag_stack_protect == SPCT_FLAG_EXPLICIT
+        && lookup_attribute("stack_protect",DECL_ATTRIBUTES(current_function_decl))))
     {
       if ((bits & (SPCT_HAS_SMALL_CHAR_ARRAY | SPCT_HAS_LARGE_CHAR_ARRAY))
 	  && !(bits & SPCT_HAS_AGGREGATE))
@@ -1743,7 +1746,10 @@ expand_used_vars (void)
 
       /* If stack protection is enabled, we don't share space between
 	 vulnerable data and non-vulnerable data.  */
-      if (flag_stack_protect)
+      if (flag_stack_protect != 0 
+      && (flag_stack_protect != SPCT_FLAG_EXPLICIT 
+      || (flag_stack_protect == SPCT_FLAG_EXPLICIT 
+      && lookup_attribute("stack_protect",DECL_ATTRIBUTES(current_function_decl)))))
 	add_stack_protection_conflicts ();
 
       /* Now that we have collected all stack variables, and have computed a
@@ -1761,15 +1767,22 @@ expand_used_vars (void)
 
     case SPCT_FLAG_STRONG:
       if (gen_stack_protect_signal
-	  || cfun->calls_alloca || has_protected_decls)
+	  || cfun->calls_alloca || has_protected_decls
+	  || lookup_attribute("stack_protect",DECL_ATTRIBUTES(current_function_decl)))
 	create_stack_guard ();
       break;
 
     case SPCT_FLAG_DEFAULT:
-      if (cfun->calls_alloca || has_protected_decls)
+      if (cfun->calls_alloca || has_protected_decls
+	  || lookup_attribute("stack_protect",DECL_ATTRIBUTES(current_function_decl)))
 	create_stack_guard ();
       break;
 
+    case SPCT_FLAG_EXPLICIT:
+      if (lookup_attribute("stack_protect",
+	  DECL_ATTRIBUTES(current_function_decl)))
+	create_stack_guard();
+      break;
     default:
       ;
     }
@@ -1793,8 +1806,11 @@ expand_used_vars (void)
 	  expand_stack_vars (stack_protect_decl_phase_1, &data);
 
 	  /* Phase 2 contains other kinds of arrays.  */
-	  if (flag_stack_protect == 2)
-	    expand_stack_vars (stack_protect_decl_phase_2, &data);
+    if (flag_stack_protect == SPCT_FLAG_ALL
+    || flag_stack_protect == SPCT_FLAG_STRONG
+    || (flag_stack_protect == SPCT_FLAG_EXPLICIT  
+    && lookup_attribute("stack_protect",DECL_ATTRIBUTES(current_function_decl))))
+	  expand_stack_vars (stack_protect_decl_phase_2, &data);
 	}
 
       if (flag_sanitize & SANITIZE_ADDRESS)
Index: gcc/doc/cpp.texi
===================================================================
--- gcc/doc/cpp.texi	(revision 204949)
+++ gcc/doc/cpp.texi	(working copy)
@@ -2353,6 +2353,10 @@ in use.
 This macro is defined, with value 3, when @option{-fstack-protector-strong} is
 in use.
 
+@item __SSP_EXPLICIT__
+This macro is defined, with value 4, when @option{-fstack-protector-explicit} is
+in use.
+
 @item __SANITIZE_ADDRESS__
 This macro is defined, with value 1, when @option{-fsanitize=address} is
 in use.
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 204949)
+++ gcc/doc/invoke.texi	(working copy)
@@ -408,7 +408,8 @@ Objective-C and Objective-C++ Dialects}.
 -fsel-sched-pipelining -fsel-sched-pipelining-outer-loops @gol
 -fshrink-wrap -fsignaling-nans -fsingle-precision-constant @gol
 -fsplit-ivs-in-unroller -fsplit-wide-types -fstack-protector @gol
--fstack-protector-all -fstack-protector-strong -fstrict-aliasing @gol
+-fstack-protector-all -fstack-protector-strong -fstack-protector-explicit @gol
+-fstrict-aliasing @gol
 -fstrict-overflow -fthread-jumps -ftracer -ftree-bit-ccp @gol
 -ftree-builtin-call-dce -ftree-ccp -ftree-ch @gol
 -ftree-coalesce-inline-vars -ftree-coalesce-vars -ftree-copy-prop @gol
@@ -9057,6 +9058,11 @@ Like @option{-fstack-protector} but incl
 be protected --- those that have local array definitions, or have
 references to local frame addresses.
 
+@item -fstack-protector-explicit
+@opindex fstack-protector-explicit
+Like @option{-fstack-protector} but only protects those functions which
+have the @code{stack_protect} attribute
+
 @item -fsection-anchors
 @opindex fsection-anchors
 Try to reduce the number of symbolic address calculations by using
Index: gcc/doc/extend.texi
===================================================================
--- gcc/doc/extend.texi	(revision 204949)
+++ gcc/doc/extend.texi	(working copy)
@@ -2167,7 +2167,7 @@ attributes are currently defined for fun
 @code{returns_nonnull}, @code{gnu_inline},
 @code{externally_visible}, @code{hot}, @code{cold}, @code{artificial},
 @code{no_sanitize_address}, @code{no_address_safety_analysis},
-@code{no_sanitize_undefined}, @code{bnd_legacy},
+@code{no_sanitize_undefined}, @code{bnd_legacy}, @code{stack_protect}
 @code{error} and @code{warning}.
 Several other attributes are defined for functions on particular
 target systems.  Other attributes, including @code{section} are
@@ -3319,6 +3319,12 @@ prologue which decides whether to split
 @code{no_split_stack} attribute do not have that prologue, and thus
 may run with only a small amount of stack space available.
 
+@item stack_protect
+@cindex @code{stack_protect} function attribute
+This function attribute make a stack protection of the function if 
+flags @option{fstack-protector} or @option{fstack-protector-strong}
+or @option{fstack-protector-explicit} are set.
+
 @item noinline
 @cindex @code{noinline} function attribute
 This function attribute prevents a function from being considered for

Re: [patch] gcc fstack-protector-explicit

[Jeff Law]

On 11/18/13 13:05, Marcos Díaz wrote:
> Hi,
>     the attached patch adds a new attribute and option flag to control
> when to do stack protection.
> The new attribute (stack_protect)  affects the behavior of gcc by
> forcing the stack protection of the function marked with the attribute
> if any of the options -fstack-protector, -fstack-protector-strong or
> -fstack-protector-explicit(new) are set.
>
> If the new option (-fstack-protector-explicit) is set only those
> functions with the attribute stack_protect will have stack protection.
>
> The stack re-ordering of the variables depends on the option set,
> currently if flag -fstack-protector is set only char arrays are
> reordered in the stack, whereas if flag -fstack-protector-strong or
> -fstack-protector-explicit is set then char arrays and other arrays
> are ordered first in the stack.
> About this reordering of the non char arrays, shouldn't all to-be
> protected functions have the full re-ordering? If not, for
> completeness, I should make that new flag -fstack-protector-explicit
> not to order the non-char arrays, and create a new -strong
> counterpart, namely -fstack-protector-explicit-strong which does.
>
> Additionally, I think that the behavior of the flag
> -fstack-protector-strong lacked the re-ordering of non char arrays
> (named phase 2) so I added the reordering also for such flag.
> Current tests pass after applying this patch, plus the tests specially added.
> Please commit it for me if OK since I don't have write access.
>
> Changelog:
> 2013-11-18 Marcos Diaz <marcos.diaz@tallertechnologies.com>
[ ... ]
Before doing any significant review on this work I have to ask, do you 
have a copyright assignment on file with the FSF and any necessary 
paperwork from your employer?

Jeff

Re: [patch] gcc fstack-protector-explicit

[Marcos Díaz]

My employer is working on the signature of the papers. Could someone
please do the review meanwhile?

On Tue, Nov 19, 2013 at 3:00 AM, Jeff Law <law@redhat.com> wrote:
> On 11/18/13 13:05, Marcos Díaz wrote:
>>
>> Hi,
>>     the attached patch adds a new attribute and option flag to control
>> when to do stack protection.
>> The new attribute (stack_protect)  affects the behavior of gcc by
>> forcing the stack protection of the function marked with the attribute
>> if any of the options -fstack-protector, -fstack-protector-strong or
>> -fstack-protector-explicit(new) are set.
>>
>> If the new option (-fstack-protector-explicit) is set only those
>> functions with the attribute stack_protect will have stack protection.
>>
>> The stack re-ordering of the variables depends on the option set,
>> currently if flag -fstack-protector is set only char arrays are
>> reordered in the stack, whereas if flag -fstack-protector-strong or
>> -fstack-protector-explicit is set then char arrays and other arrays
>> are ordered first in the stack.
>> About this reordering of the non char arrays, shouldn't all to-be
>> protected functions have the full re-ordering? If not, for
>> completeness, I should make that new flag -fstack-protector-explicit
>> not to order the non-char arrays, and create a new -strong
>> counterpart, namely -fstack-protector-explicit-strong which does.
>>
>> Additionally, I think that the behavior of the flag
>> -fstack-protector-strong lacked the re-ordering of non char arrays
>> (named phase 2) so I added the reordering also for such flag.
>> Current tests pass after applying this patch, plus the tests specially
>> added.
>> Please commit it for me if OK since I don't have write access.
>>
>> Changelog:
>> 2013-11-18 Marcos Diaz <marcos.diaz@tallertechnologies.com>
>
> [ ... ]
> Before doing any significant review on this work I have to ask, do you have
> a copyright assignment on file with the FSF and any necessary paperwork from
> your employer?
>
> Jeff
>



-- 
______________________________


Marcos Díaz

Software Engineer


San Lorenzo 47, 3rd Floor, Office 5

Córdoba, Argentina


Phone: +54 351 4217888 / +54 351 4218211/ +54 351 7617452

Skype: markdiaz22

Re: [patch] gcc fstack-protector-explicit

[Jeff Law]

On 11/19/13 07:04, Marcos Díaz wrote:
> My employer is working on the signature of the papers. Could someone
> please do the review meanwhile?
I'd prefer to wait until the assignment process is complete.  If 
something were to happen and we can't use your code the review time 
would have been wasted (and such things have certainly happened in the 
past).

Once the assignment is recorded, please ping this patch.

Jeff

Re: [patch] gcc fstack-protector-explicit

[Marcos Díaz]

Well, finally I have the assignment, could you please review this patch?

On Wed, Nov 20, 2013 at 4:13 PM, Jeff Law <law@redhat.com> wrote:
> On 11/19/13 07:04, Marcos Díaz wrote:
>>
>> My employer is working on the signature of the papers. Could someone
>> please do the review meanwhile?
>
> I'd prefer to wait until the assignment process is complete.  If something
> were to happen and we can't use your code the review time would have been
> wasted (and such things have certainly happened in the past).
>
> Once the assignment is recorded, please ping this patch.
>
> Jeff
>



-- 
______________________________


Marcos Díaz

Software Engineer


San Lorenzo 47, 3rd Floor, Office 5

Córdoba, Argentina


Phone: +54 351 4217888 / +54 351 4218211/ +54 351 7617452

Skype: markdiaz22

Re: [patch] gcc fstack-protector-explicit

[Jeff Law]

On 03/19/14 08:06, Marcos Díaz wrote:
> Well, finally I have the assignment, could you please review this patch?
Thanks.  I'll take a look once we open up stage1 development again 
(should be soon as 4.9 is getting close to being ready).

jeff

Re: [patch] gcc fstack-protector-explicit

[Marcos Díaz]

On Wed, Mar 19, 2014 at 11:09 AM, 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.  I'll take a look once we open up stage1 development again (should
> be soon as 4.9 is getting close to being ready).
>
> jeff
>

Ping!

Re: [patch] gcc fstack-protector-explicit

[Jeff Law]

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?

The patch itself is relatively simple and I don't see anything that 
looks terribly wrong at first glance.  I think we just need to make sure 
we're on the same page WRT needing the -fstack-protect-explicit flag.

jeff

Re: [patch] gcc fstack-protector-explicit

[Daniel Gutson]

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.

>
> The patch itself is relatively simple and I don't see anything that looks
> terribly wrong at first glance.  I think we just need to make sure we're on
> the same page WRT needing the -fstack-protect-explicit flag.
>
> jeff
>
>



-- 

Daniel F. Gutson
Chief Engineering Officer, SPD


San Lorenzo 47, 3rd Floor, Office 5

Córdoba, Argentina


Phone: +54 351 4217888 / +54 351 4218211

Skype: dgutson

Re: [patch] gcc fstack-protector-explicit

[Marcos Díaz]

On Tue, Jul 1, 2014 at 6:34 PM, Daniel Gutson
<daniel.gutson@tallertechnologies.com> 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.
>
If there isn't any stack-protect flag (strong, common or explicit) the
attribute has no effect
> 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.
>
>>
>> The patch itself is relatively simple and I don't see anything that looks
>> terribly wrong at first glance.  I think we just need to make sure we're on
>> the same page WRT needing the -fstack-protect-explicit flag.
>>
>> jeff
>>
>>
>
>
>
> --
>
> Daniel F. Gutson
> Chief Engineering Officer, SPD
>
>
> San Lorenzo 47, 3rd Floor, Office 5
>
> Córdoba, Argentina
>
>
> Phone: +54 351 4217888 / +54 351 4218211
>
> Skype: dgutson



-- 
______________________________


Marcos Díaz

Software Engineer


San Lorenzo 47, 3rd Floor, Office 5

Córdoba, Argentina


Phone: +54 351 4217888 / +54 351 4218211/ +54 351 7617452

Skype: markdiaz22

Re: [patch] gcc fstack-protector-explicit

[Marcos Díaz]

On Wed, Jul 2, 2014 at 2:58 PM, Marcos Díaz
<marcos.diaz@tallertechnologies.com> wrote:
> On Tue, Jul 1, 2014 at 6:34 PM, Daniel Gutson
> <daniel.gutson@tallertechnologies.com> 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.
>>
> If there isn't any stack-protect flag (strong, common or explicit) the
> attribute has no effect
>> 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.
>>
>>>
>>> The patch itself is relatively simple and I don't see anything that looks
>>> terribly wrong at first glance.  I think we just need to make sure we're on
>>> the same page WRT needing the -fstack-protect-explicit flag.
>>>
>>> jeff
>>>
>>>
>>
>>
>>
>> --
>>
>> Daniel F. Gutson
>> Chief Engineering Officer, SPD
>>
>>
>> San Lorenzo 47, 3rd Floor, Office 5
>>
>> Córdoba, Argentina
>>
>>
>> Phone: +54 351 4217888 / +54 351 4218211
>>
>> Skype: dgutson
>
>
>
> --
> ______________________________
>
>
> Marcos Díaz
>
> Software Engineer
>
>
> San Lorenzo 47, 3rd Floor, Office 5
>
> Córdoba, Argentina
>
>
> Phone: +54 351 4217888 / +54 351 4218211/ +54 351 7617452
>
> Skype: markdiaz22

ping

-- 
______________________________


Marcos Díaz

Software Engineer


San Lorenzo 47, 3rd Floor, Office 5

Córdoba, Argentina


Phone: +54 351 4217888 / +54 351 4218211/ +54 351 7617452

Skype: markdiaz22

Re: [patch] gcc fstack-protector-explicit

[Jeff Law]

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.

Sorry for the terribly long delay.

jeff

Re: [patch] gcc fstack-protector-explicit

[Rainer Orth]

[-- Attachment #1: Type: text/plain, Size: 2279 bytes --]

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.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fstack-protector-explicit-LINK_SSP_SPEC.patch --]
[-- Type: text/x-patch, Size: 605 bytes --]

# 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
 

[-- Attachment #3: Type: text/plain, Size: 143 bytes --]


-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [patch] gcc fstack-protector-explicit

[Jakub Jelinek]

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

Re: [patch] gcc fstack-protector-explicit

[Jeff Law]

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

Re: [patch] gcc fstack-protector-explicit

[Jakub Jelinek]

On Thu, Jan 22, 2015 at 09:35:45AM -0700, Jeff Law wrote:
> >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.

Here is what I've committed.

2015-01-22  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>
	    Jakub Jelinek  <jakub@redhat.com>

	* gcc.c (LINK_SSP_SPEC): Handle -fstack-protector-explicit
	for !TARGET_LIBC_PROVIDES_SSP version and
	-fstack-protector-{all,strong,explicit} otherwise.
	* config/freebsd.h (LINK_SSP_SPEC): Handle
	-fstack-protector-{strong,explicit}.

--- gcc/gcc.c.jj	2015-01-22 11:59:32.000000000 +0100
+++ gcc/gcc.c	2015-01-22 17:42:17.965531999 +0100
@@ -728,9 +728,12 @@ proper position among the other output f
 
 #ifndef LINK_SSP_SPEC
 #ifdef TARGET_LIBC_PROVIDES_SSP
-#define LINK_SSP_SPEC "%{fstack-protector:}"
+#define LINK_SSP_SPEC "%{fstack-protector|fstack-protector-all" \
+		       "|fstack-protector-strong|fstack-protector-explicit:}"
 #else
-#define LINK_SSP_SPEC "%{fstack-protector|fstack-protector-strong|fstack-protector-all:-lssp_nonshared -lssp}"
+#define LINK_SSP_SPEC "%{fstack-protector|fstack-protector-all" \
+		       "|fstack-protector-strong|fstack-protector-explicit" \
+		       ":-lssp_nonshared -lssp}"
 #endif
 #endif
 
--- gcc/config/freebsd.h.jj	2015-01-05 13:07:15.000000000 +0100
+++ gcc/config/freebsd.h	2015-01-22 17:43:12.504603764 +0100
@@ -49,7 +49,9 @@ along with GCC; see the file COPYING3.
 #endif
 
 #ifdef TARGET_LIBC_PROVIDES_SSP
-#define LINK_SSP_SPEC "%{fstack-protector|fstack-protector-all:-lssp_nonshared}"
+#define LINK_SSP_SPEC "%{fstack-protector|fstack-protector-all" \
+		       "|fstack-protector-strong|fstack-protector-explicit" \
+		       ":-lssp_nonshared}"
 #endif
 
 #undef TARGET_LIBC_HAS_FUNCTION


	Jakub