[patch] Stack corruption in naked functions

[Paul Brook <paul at codesourcery dot com>]

The patch below fixed a stack corruption bug in __attribute__((naked)) 
functions. On supported targets this attribute can be used to suppress the 
normal function prologue/epilogue. This allows a function to be implemented 
in assembly without requiring the user to put everything in a separate .S 
file.

The problem is that at -O0 the compiler assigns all decls to a local stack 
slot and the value will be copied to this slot even if not used. This is 
undesirable in naked function because we don't allocate a stack frame.

The best solution we could come up with is to suppress stack slot allocation 
for these functions.

I can't find evidence that this is a regression, however it does make this 
attribute effectively useless.

I've also updated the user documentation to clarify the intended use of 
__attribute__((naked)).

Tested on arm-nopne-eabi.
Ok?

Paul

2007-11-08  Paul Brook  <paul@codesourcery.com>

	gcc/
	* doc/extend.texi: Clarify use of __attribute__((naked)).
	* doc/tm.texi: Document TARGET_USE_REG_FOR_FUNC.
	* target.h (gcc_target): Add use_reg_for_func.
	* function.c (use_register_for_decl): Use
	targetm.calls.use_reg_for_func.
	* target-def.h (TARGET_CALLS): Add TARGET_USE_REG_FOR_FUNC.
	* config/arm/arm.c (arm_use_reg_for_func): New function.
	(TARGET_USE_REG_FOR_FUNC): Define.

	gcc/testsuite/
	* gcc.target/arm/naked-1.c: New test.

Index: gcc/doc/extend.texi
===================================================================
--- gcc/doc/extend.texi	(revision 129981)
+++ gcc/doc/extend.texi	(working copy)
@@ -2477,7 +2477,13 @@ defined by shared libraries.
 @cindex function without a prologue/epilogue code
 Use this attribute on the ARM, AVR, C4x, IP2K and SPU ports to indicate that
 the specified function does not need prologue/epilogue sequences generated by
-the compiler.  It is up to the programmer to provide these sequences.
+the compiler.  It is up to the programmer to provide these sequences.  The
+only statements that can be safely included in naked functions are 
+@code{asm} statements that do not have operands.  All other statements,
+including declarations of local variables, @code{if} statements, and so 
+forth, should be avoided.  Naked functions should be used to implement the 
+body of an assembly function, while allowing the compiler to construct
+the requisite function declaration for the assembler.
 
 @item near
 @cindex functions which do not handle memory bank switching on 68HC11/68HC12
Index: gcc/doc/tm.texi
===================================================================
--- gcc/doc/tm.texi	(revision 129981)
+++ gcc/doc/tm.texi	(working copy)
@@ -4297,6 +4297,18 @@ or 3-byte structure is returned at the m
 @code{SImode} rtx.
 @end deftypefn
 
+@deftypefn {Target Hook} bool TARGET_USE_REG_FOR_FUNC (void)
+When optimization is disabled most decls (including function arguments) 
+will be allocated local stack slots.
+
+This hook should return true if they should be allocated pseudo registers
+for the current function.  Typically this is desirable for functions
+annotated with @code{__attribute__((naked))}.  This avoids generating
+stores to a non-existant stack frame.
+
+Under normal curcumstances this hook should return false.
+@end deftypefn
+
 @node Aggregate Return
 @subsection How Large Values Are Returned
 @cindex aggregates as return values
Index: gcc/target.h
===================================================================
--- gcc/target.h	(revision 129981)
+++ gcc/target.h	(working copy)
@@ -827,6 +827,11 @@ struct gcc_target
     /* Return an rtx for the argument pointer incoming to the
        current function.  */
     rtx (*internal_arg_pointer) (void);
+
+    /* Return true if all function parameters should remain in registers,
+       rather than being spilled to the stack.  */
+    bool (*use_reg_for_func) (void);
+    
   } calls;
 
   /* Return the diagnostic message string if conversion from FROMTYPE
Index: gcc/testsuite/gcc.target/arm/naked-1.c
===================================================================
--- gcc/testsuite/gcc.target/arm/naked-1.c	(revision 0)
+++ gcc/testsuite/gcc.target/arm/naked-1.c	(revision 0)
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* Check that function arguments aren't assigned and copied to stack slots
+   in naked functions.  This ususally happens at -O0 (presumably for
+   better debugging), but is highly undesirable if we haven't created
+   a stack frame.  */
+void __attribute__((naked))
+foo(int n)
+{
+  __asm__ volatile ("frob r0\n");
+}
+
+/* { dg-final { scan-assembler-not "\tstr" } } */
Index: gcc/function.c
===================================================================
--- gcc/function.c	(revision 129981)
+++ gcc/function.c	(working copy)
@@ -1850,7 +1850,8 @@ use_register_for_decl (const_tree decl)
   if (DECL_IGNORED_P (decl))
     return true;
 
-  return (optimize || DECL_REGISTER (decl));
+  return (optimize || DECL_REGISTER (decl)
+	  || targetm.calls.use_reg_for_func ());
 }
 
 /* Return true if TYPE should be passed by invisible reference.  */
Index: gcc/target-def.h
===================================================================
--- gcc/target-def.h	(revision 129981)
+++ gcc/target-def.h	(working copy)
@@ -565,6 +565,7 @@
 
 #define TARGET_FUNCTION_VALUE default_function_value
 #define TARGET_INTERNAL_ARG_POINTER default_internal_arg_pointer
+#define TARGET_USE_REG_FOR_FUNC hook_bool_void_false
 
 #define TARGET_CALLS {						\
    TARGET_PROMOTE_FUNCTION_ARGS,				\
@@ -584,7 +585,8 @@
    TARGET_ARG_PARTIAL_BYTES,					\
    TARGET_INVALID_ARG_FOR_UNPROTOTYPED_FN,			\
    TARGET_FUNCTION_VALUE,					\
-   TARGET_INTERNAL_ARG_POINTER					\
+   TARGET_INTERNAL_ARG_POINTER,					\
+   TARGET_USE_REG_FOR_FUNC					\
    }
 
 #ifndef TARGET_UNWIND_TABLES_DEFAULT
Index: gcc/config/arm/arm.c
===================================================================
--- gcc/config/arm/arm.c	(revision 129981)
+++ gcc/config/arm/arm.c	(working copy)
@@ -189,6 +189,7 @@ static unsigned HOST_WIDE_INT arm_shift_
 static bool arm_cannot_copy_insn_p (rtx);
 static bool arm_tls_symbol_p (rtx x);
 static void arm_output_dwarf_dtprel (FILE *, int, rtx) ATTRIBUTE_UNUSED;
+static bool arm_use_reg_for_func (void);
 
 
 /* Initialize the GCC target structure.  */
@@ -289,6 +290,9 @@ static void arm_output_dwarf_dtprel (FIL
 #undef  TARGET_SETUP_INCOMING_VARARGS
 #define TARGET_SETUP_INCOMING_VARARGS arm_setup_incoming_varargs
 
+#undef TARGET_USE_REG_FOR_FUNC
+#define TARGET_USE_REG_FOR_FUNC arm_use_reg_for_func
+
 #undef TARGET_DEFAULT_SHORT_ENUMS
 #define TARGET_DEFAULT_SHORT_ENUMS arm_default_short_enums
 
@@ -1614,6 +1618,15 @@ arm_current_func_type (void)
 
   return cfun->machine->func_type;
 }
+
+bool
+arm_use_reg_for_func (void)
+{
+  /* Never spill function parameters to the stack if the function
+     is naked.  */
+  return IS_NAKED (arm_current_func_type ());
+}
+
 
 /* Return 1 if it is possible to return using a single instruction.
    If SIBLING is non-null, this is a test for a return before a sibling