Re: [PATCH] Fix unsafe function attributes for special functions (PR 71876)

[Bernd Edlinger <bernd dot edlinger at hotmail dot de>]

On 07/21/16 09:09, Richard Biener wrote:
> On Wed, 20 Jul 2016, Bernd Edlinger wrote:
>
>> On 07/20/16 20:08, Richard Biener wrote:
>>> On July 20, 2016 6:54:48 PM GMT+02:00, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote:
>>>>
>>>> But I think that alloca just should not be recognized by name any
>>>> more.
>>>
>>> It was introduced to mark calls that should not be duplicated by inlining or unrolling to avoid increasing stack usage too much.  Sth worthwhile to keep even with -ffreestanding.
>>>
>>> Richard.
>>>
>>
>> On second thought I start to think that an external alloca function
>> might still work.  And returning ECF_MAY_BE_ALLOCA just based on the
>> name could be made safe by checking the malloc attribute at the right
>> places.
>>
>> With this new incremental patch the example
>>
>> extern "C"
>> void *alloca(unsigned long);
>> void bar(unsigned long n)
>> {
>>     char *x = (char*) alloca(n);
>>     if (x)
>>       *x = 0;
>> }
>>
>> might actually work when -ansi is used,
>> i.e. it does no longer assume that alloca cannot return null,
>> but still creates a frame pointer, which it would not have done
>> for allocb for instance.
>>
>> But the built-in alloca is still recognized because the builtin
>> does have ECF_MAY_BE_ALLOCA and ECF_MALLOC.
>>
>>
>> Is it OK for trunk after boot-strap and reg-testing?
>
> Hmm, but ECF_MALLOC doesn't guarantee non-NULL return.  I think the
> two calls you patched simply shouldn't use the predicates (which
> are misnamed as they check for _maybe_alloca_call_p).  Instead they
> have to check for the respective builtins (BUILT_IN_ALLOCA,
> BUILT_IN_ALLOCA_WITH_ALIGN).
>

Yes, I agree.  The name should really be gimple_maybe_alloca_call_p if
we are just guessing.

How about this updated patch?

Is it OK for trunk after boot-strap and reg-testing?


Thanks
Bernd.

> Richard.
>
>>
>> Thanks
>> Bernd.
>>
>

2016-07-21  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR middle-end/71876
	* calls.c (gimple_maybe_alloca_call_p): New function.  Return true
	if STMT may be an alloca call.
	(gimple_alloca_call_p, alloca_call_p): Return only true for the
	builtin alloca call.
	* calls.h (gimple_maybe_alloca_call_p): New function.
	* tree-inline.c (inline_forbidden_p_stmt): Use
	gimple_maybe_alloca_call_p here.

Index: gcc/calls.c
===================================================================
--- gcc/calls.c	(revision 238584)
+++ gcc/calls.c	(working copy)
@@ -617,10 +617,10 @@ setjmp_call_p (const_tree fndecl)
 }
 
 
-/* Return true if STMT is an alloca call.  */
+/* Return true if STMT may be an alloca call.  */
 
 bool
-gimple_alloca_call_p (const gimple *stmt)
+gimple_maybe_alloca_call_p (const gimple *stmt)
 {
   tree fndecl;
 
@@ -634,16 +634,44 @@ bool
   return false;
 }
 
-/* Return true when exp contains alloca call.  */
+/* Return true if STMT is a builtin alloca call.  */
 
 bool
+gimple_alloca_call_p (const gimple *stmt)
+{
+  tree fndecl;
+
+  if (!is_gimple_call (stmt))
+    return false;
+
+  fndecl = gimple_call_fndecl (stmt);
+  if (fndecl && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
+    switch (DECL_FUNCTION_CODE (fndecl))
+      {
+      case BUILT_IN_ALLOCA:
+      case BUILT_IN_ALLOCA_WITH_ALIGN:
+        return true;
+      }
+
+  return false;
+}
+
+/* Return true when exp contains a builtin alloca call.  */
+
+bool
 alloca_call_p (const_tree exp)
 {
   tree fndecl;
   if (TREE_CODE (exp) == CALL_EXPR
       && (fndecl = get_callee_fndecl (exp))
-      && (special_function_p (fndecl, 0) & ECF_MAY_BE_ALLOCA))
-    return true;
+      && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL)
+    switch (DECL_FUNCTION_CODE (fndecl))
+      {
+      case BUILT_IN_ALLOCA:
+      case BUILT_IN_ALLOCA_WITH_ALIGN:
+        return true;
+      }
+
   return false;
 }
 
Index: gcc/calls.h
===================================================================
--- gcc/calls.h	(revision 238584)
+++ gcc/calls.h	(working copy)
@@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.  If not see
 extern int flags_from_decl_or_type (const_tree);
 extern int call_expr_flags (const_tree);
 extern int setjmp_call_p (const_tree);
+extern bool gimple_maybe_alloca_call_p (const gimple *);
 extern bool gimple_alloca_call_p (const gimple *);
 extern bool alloca_call_p (const_tree);
 extern bool must_pass_in_stack_var_size (machine_mode, const_tree);
Index: gcc/tree-inline.c
===================================================================
--- gcc/tree-inline.c	(revision 238584)
+++ gcc/tree-inline.c	(working copy)
@@ -3577,7 +3577,7 @@ inline_forbidden_p_stmt (gimple_stmt_iterator *gsi
 	 RAM instead of 256MB.  Don't do so for alloca calls emitted for
 	 VLA objects as those can't cause unbounded growth (they're always
 	 wrapped inside stack_save/stack_restore regions.  */
-      if (gimple_alloca_call_p (stmt)
+      if (gimple_maybe_alloca_call_p (stmt)
 	  && !gimple_call_alloca_for_var_p (as_a <gcall *> (stmt))
 	  && !lookup_attribute ("always_inline", DECL_ATTRIBUTES (fn)))
 	{