https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92825

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2019-12-05
                 CC|                            |jakub at gcc dot gnu.org,
                   |                            |law at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The stack protection in there is because the expansion sees the color variable
(not optimized away) in there and that variable's type contains arrays.
I guess this boils down to:
int
foo (int r, int g, int b)
{
  union U { int rgba; char p[4]; } u;
  u.p[0] = r;
  u.p[1] = g;
  u.p[2] = b;
  u.p[3] = -1;
  return u.rgba;
}

While stack_vars_num is 0 here, we don't force u into memory, it gets assigned
a pseudo instead, stack protection is still forced because of
stack_protect_decl_p function.
Now, why it is done this way rather than only do such checks on the vars we
force into memory, like moving the stack_protect_decl_p call from where it is
currently to around:
      /* If stack protection is enabled, we don't share space between
         vulnerable data and non-vulnerable data.  */
      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 ();
and iterate on stack_vars[0].decl ... stack_vars[stack_vars_num - 1].decl
instead of
  FOR_EACH_LOCAL_DECL (cfun, i, var)
    if (!is_global_var (var))
I have no idea.
We'd instrument fewer functions, sure, but vars that don't really live in
memory shouldn't result in stack overflows.
CCing Jeff as reviewer of the -fstack-protector-strong patches.

Completely untested patch that fixes the short testcase mentioned above:
--- gcc/cfgexpand.c.jj  2019-12-03 20:21:30.542464581 +0100
+++ gcc/cfgexpand.c     2019-12-05 15:19:08.578613685 +0100
@@ -2021,19 +2021,18 @@ static bool
 stack_protect_decl_p ()
 {
   unsigned i;
-  tree var;

-  FOR_EACH_LOCAL_DECL (cfun, i, var)
-    if (!is_global_var (var))
-      {
-       tree var_type = TREE_TYPE (var);
-       if (VAR_P (var)
-           && (TREE_CODE (var_type) == ARRAY_TYPE
-               || TREE_ADDRESSABLE (var)
-               || (RECORD_OR_UNION_TYPE_P (var_type)
-                   && record_or_union_type_has_array_p (var_type))))
-         return true;
-      }
+  for (i = 0; i < stack_vars_num; i++)
+    {
+      tree var = stack_vars[i].decl;
+      tree var_type = TREE_TYPE (var);
+      if (VAR_P (var)
+         && (TREE_CODE (var_type) == ARRAY_TYPE
+             || TREE_ADDRESSABLE (var)
+             || (RECORD_OR_UNION_TYPE_P (var_type)
+                 && record_or_union_type_has_array_p (var_type))))
+       return true;
+    }
   return false;
 }

@@ -2103,8 +2102,7 @@ expand_used_vars (void)
     }

   if (flag_stack_protect == SPCT_FLAG_STRONG)
-      gen_stack_protect_signal
-       = stack_protect_decl_p () || stack_protect_return_slot_p ();
+    gen_stack_protect_signal = stack_protect_return_slot_p ();

   /* At this point all variables on the local_decls with TREE_USED
      set are not associated with any block scope.  Lay them out.  */
@@ -2180,6 +2178,9 @@ expand_used_vars (void)

   if (stack_vars_num > 0)
     {
+      if (flag_stack_protect == SPCT_FLAG_STRONG)
+       gen_stack_protect_signal |= stack_protect_decl_p ();
+
       add_scope_conflicts ();

       /* If stack protection is enabled, we don't share space between
Of course, pedantically it changes the meaning of -fstack-protector-strong from
variables with arrays to variables with arrays emitted into memory.  Though, we
already don't count variables that are optimized away anyway.
Another question is if we need this stack_protect_decl_p function at all, I
mean, stack_protect_classify_type should already flag those variables as
needing protection and if it doesn't flag any, what is the point when no vars
would be emitted in the area of stack protected variables?

Reply via email to