From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25762 invoked by alias); 8 Nov 2007 17:14:39 -0000 Received: (qmail 25750 invoked by uid 22791); 8 Nov 2007 17:14:37 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 08 Nov 2007 17:14:34 +0000 Received: (qmail 30151 invoked from network); 8 Nov 2007 17:14:32 -0000 Received: from unknown (HELO 81-179-181-133.dsl.pipex.com) (paul@127.0.0.2) by mail.codesourcery.com with ESMTPA; 8 Nov 2007 17:14:32 -0000 From: Paul Brook To: gcc-patches@gcc.gnu.org Subject: [patch] Stack corruption in naked functions Date: Thu, 08 Nov 2007 17:14:00 -0000 User-Agent: KMail/1.9.7 MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_zP0MHyadQs6KB0C" Message-Id: <200711081714.27475.paul@codesourcery.com> Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2007-11/txt/msg00436.txt.bz2 --Boundary-00=_zP0MHyadQs6KB0C Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline Content-length: 1372 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 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. --Boundary-00=_zP0MHyadQs6KB0C Content-Type: text/x-diff; charset="us-ascii"; name="patch.naked_reg" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="patch.naked_reg" Content-length: 5671 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 --Boundary-00=_zP0MHyadQs6KB0C--