From patchwork Thu Jun 10 12:23:27 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Brook X-Patchwork-Id: 55206 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 91AA2B7D90 for ; Thu, 10 Jun 2010 22:25:25 +1000 (EST) Received: (qmail 9870 invoked by alias); 10 Jun 2010 12:25:14 -0000 Received: (qmail 9600 invoked by uid 22791); 10 Jun 2010 12:24:47 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL, BAYES_00, TW_FN, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 10 Jun 2010 12:24:29 +0000 Received: (qmail 3266 invoked from network); 10 Jun 2010 12:24:27 -0000 Received: from unknown (HELO wren.localnet) (paul@127.0.0.2) by mail.codesourcery.com with ESMTPA; 10 Jun 2010 12:24:27 -0000 From: Paul Brook To: gcc-patches@gcc.gnu.org Subject: [PATCH] Invalid warnings for naked functions Date: Thu, 10 Jun 2010 13:23:27 +0100 User-Agent: KMail/1.13.3 (Linux/2.6.33-2-amd64; KDE/4.4.4; x86_64; ; ) MIME-Version: 1.0 Message-Id: <201006101323.27637.paul@codesourcery.com> Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org When using the "naked" function attribute, gcc still emits warnings about missing return values and returning from a noreturn function. The mcore port has a nasty hack to workaround one of these warnings. The comment indicates that this functionality was rejected, however the PR number is bogus. The closest I've found is PR11775. This seems to be mostly confusion about the supported use-cases for __attribute__((naked)), which has since been clarified. The whole point of these functions is to suppress the function prologue/epilogue, allowing the user implement their own return sequence in a way that the compiler doesn't understand. Requiring that the user explicitly suppress the warning seems unreasonable as this warning will always trigger. This and the ad-hoc mcore hack seem like sufficient justification for making this a feature. Tested on arm-non-eabi, plus basic smoke-tests on avr-elf,mcore-elf and x86_64-linux. I also build rx-elf and spu-elf, but they segfault both before and after the patch. Ok? Paul 2010-10-10 Paul Brook gcc/ * doc/tm.texi: Document TARGET_WARN_FUNC_RETURN. * target.h (gcc_target): Add warn_func_return. * target-def.h (TARGET_WARN_FUNC_RETURN): Define. (TARGET_INITIALIZER): Use it. * tree-cfg.h (execute_warn_function_return): Check targetm.warn_func_return. * config/spu/spu.c (spu_warn_func_return): New. (TARGET_WARN_FUNC_RETURN): Define. * config/rx/rx.c (rx_warn_func_return): New. (TARGET_WARN_FUNC_RETURN): Define. * config/avr/avr.c (avr_warn_func_return): New. (TARGET_WARN_FUNC_RETURN): Define. * config/arm/arm.c (arm_warn_func_return): New. (TARGET_WARN_FUNC_RETURN): Define. * config/mcore/mcore.c (mcore_warn_func_return): New. (TARGET_WARN_FUNC_RETURN): Define. (saved_warn_return_type, saved_warn_return_type_count): Remove. (mcore_reorg, mcore_handle_naked_attribute): Remove warn_return hack. gcc/testsuite/ * gcc.target/arm/naked-3.c: New test. Index: gcc/doc/tm.texi =================================================================== --- gcc/doc/tm.texi (revision 160360) +++ gcc/doc/tm.texi (working copy) @@ -11181,3 +11181,11 @@ value of @code{TARGET_CONST_ANCHOR} is a MIPS, where add-immediate takes a 16-bit signed value, @code{TARGET_CONST_ANCHOR} is set to @samp{0x8000}. The default value is zero, which disables this optimization. @end deftypevr + +@deftypefn {Target Hook} bool TARGET_WARN_FUNC_RETURN (void) +This hook should return @code{false} to suppress warning about missing +return statements or suspect @code{noreturn} attributes for the current +function. The is typically appropriate for functions declared with +@code{__attribute__((naked))}. +The default implementation always returns @code{true}. +@end deftypefn Index: gcc/target.h =================================================================== --- gcc/target.h (revision 160360) +++ gcc/target.h (working copy) @@ -1239,6 +1239,10 @@ struct gcc_target bits in the bitmap passed in. */ void (*live_on_entry) (bitmap); + /* Return false if warnings about missing return statements or suspect + noreturn attributes should be suppressed for the current function. */ + bool (*warn_func_return) (void); + /* True if unwinding tables should be generated by default. */ bool unwind_tables_default; Index: gcc/testsuite/gcc.target/arm/naked-3.c =================================================================== --- gcc/testsuite/gcc.target/arm/naked-3.c (revision 0) +++ gcc/testsuite/gcc.target/arm/naked-3.c (revision 0) @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -Wall" } */ +/* Check that we do not get warnings about missing return statements + or bogus looking noreturn functions. */ +int __attribute__((naked)) +foo(void) +{ + __asm__ volatile ("mov r0, #1\r\nbx lr\n"); +} + +int __attribute__((naked,noreturn)) +bar(void) +{ + __asm__ volatile ("frob r0\n"); +} Index: gcc/target-def.h =================================================================== --- gcc/target-def.h (revision 160360) +++ gcc/target-def.h (working copy) @@ -220,6 +220,10 @@ #define TARGET_EXTRA_LIVE_ON_ENTRY hook_void_bitmap #endif +#ifndef TARGET_WARN_FUNC_RETURN +#define TARGET_WARN_FUNC_RETURN hook_bool_void_true +#endif + #ifndef TARGET_ASM_FILE_START_APP_OFF #define TARGET_ASM_FILE_START_APP_OFF false #endif @@ -1074,6 +1078,7 @@ TARGET_EMUTLS, \ TARGET_OPTION_HOOKS, \ TARGET_EXTRA_LIVE_ON_ENTRY, \ + TARGET_WARN_FUNC_RETURN, \ TARGET_UNWIND_TABLES_DEFAULT, \ TARGET_HAVE_NAMED_SECTIONS, \ TARGET_HAVE_SWITCHABLE_BSS_SECTIONS, \ Index: gcc/tree-cfg.c =================================================================== --- gcc/tree-cfg.c (revision 160360) +++ gcc/tree-cfg.c (working copy) @@ -45,6 +45,7 @@ along with GCC; see the file COPYING3. #include "value-prof.h" #include "pointer-set.h" #include "tree-inline.h" +#include "target.h" /* This file contains functions for building the Control Flow Graph (CFG) for a function tree. */ @@ -7163,6 +7164,9 @@ execute_warn_function_return (void) edge e; edge_iterator ei; + if (!targetm.warn_func_return ()) + return 0; + /* If we have a path to EXIT, then we do return. */ if (TREE_THIS_VOLATILE (cfun->decl) && EDGE_COUNT (EXIT_BLOCK_PTR->preds) > 0) Index: gcc/config/spu/spu.c =================================================================== --- gcc/config/spu/spu.c (revision 160360) +++ gcc/config/spu/spu.c (working copy) @@ -280,6 +280,8 @@ spu_libgcc_cmp_return_mode (void); static enum machine_mode spu_libgcc_shift_count_mode (void); +static bool spu_warn_func_return (void); + /* Pointer mode for __ea references. */ #define EAmode (spu_ea_model != 32 ? DImode : SImode) @@ -465,6 +467,9 @@ static const struct attribute_spec spu_a #undef TARGET_TRAMPOLINE_INIT #define TARGET_TRAMPOLINE_INIT spu_trampoline_init +#undef TARGET_WARN_FUNC_RETURN +#define TARGET_WARN_FUNC_RETURN spu_warn_func_return + struct gcc_target targetm = TARGET_INITIALIZER; void @@ -7089,4 +7094,12 @@ spu_function_profiler (FILE * file, int fprintf (file, "brsl $75, _mcount\n"); } +static bool +spu_warn_func_return (void) +{ + /* Naked functions are implemented entirely in assembly, including the + return sequence, so suppress warnings about this. */ + return !spu_naked_function_p (current_function_decl); +} + #include "gt-spu.h" Index: gcc/config/rx/rx.c =================================================================== --- gcc/config/rx/rx.c (revision 160360) +++ gcc/config/rx/rx.c (working copy) @@ -2227,6 +2227,14 @@ rx_func_attr_inlinable (const_tree decl) && ! is_naked_func (decl); } +static bool +rx_warn_func_return (void) +{ + /* Naked functions are implemented entirely in assembly, including the + return sequence, so suppress warnings about this. */ + return !is_naked_func (NULL_TREE); +} + /* Return nonzero if it is ok to make a tail-call to DECL, a function_decl or NULL if this is an indirect call, using EXP */ @@ -2573,6 +2581,9 @@ rx_trampoline_init (rtx tramp, tree fnde #undef TARGET_TRAMPOLINE_INIT #define TARGET_TRAMPOLINE_INIT rx_trampoline_init +#undef TARGET_WARN_FUNC_RETURN +#define TARGET_WARN_FUNC_RETURN rx_warn_func_return + struct gcc_target targetm = TARGET_INITIALIZER; /* #include "gt-rx.h" */ Index: gcc/config/avr/avr.c =================================================================== --- gcc/config/avr/avr.c (revision 160360) +++ gcc/config/avr/avr.c (working copy) @@ -90,6 +90,7 @@ static bool avr_hard_regno_scratch_ok (u static unsigned int avr_case_values_threshold (void); static bool avr_frame_pointer_required_p (void); static bool avr_can_eliminate (const int, const int); +static bool avr_warn_func_return (void); /* Allocate registers from r25 to r8 for parameters for function calls. */ #define FIRST_CUM_REG 26 @@ -191,6 +192,9 @@ static const struct attribute_spec avr_a #undef TARGET_CAN_ELIMINATE #define TARGET_CAN_ELIMINATE avr_can_eliminate +#undef TARGET_WARN_FUNC_RETURN +#define TARGET_WARN_FUNC_RETURN avr_warn_func_return + struct gcc_target targetm = TARGET_INITIALIZER; void @@ -6144,4 +6148,12 @@ unsigned int avr_case_values_threshold ( return (!AVR_HAVE_JMP_CALL || TARGET_CALL_PROLOGUES) ? 8 : 17; } +static bool +avr_warn_func_return (void) +{ + /* Naked functions are implemented entirely in assembly, including the + return sequence, so suppress warnings about this. */ + return !avr_naked_function_p (current_function_decl); +} + #include "gt-avr.h" Index: gcc/config/mcore/mcore.c =================================================================== --- gcc/config/mcore/mcore.c (revision 160360) +++ gcc/config/mcore/mcore.c (working copy) @@ -145,6 +145,7 @@ static int mcore_arg_partial_byte tree, bool); static void mcore_asm_trampoline_template (FILE *); static void mcore_trampoline_init (rtx, tree, rtx); +static bool mcore_warn_func_return (void); /* MCore specific attributes. */ @@ -214,6 +215,9 @@ static const struct attribute_spec mcore #undef TARGET_TRAMPOLINE_INIT #define TARGET_TRAMPOLINE_INIT mcore_trampoline_init +#undef TARGET_WARN_FUNC_RETURN +#define TARGET_WARN_FUNC_RETURN mcore_warn_func_return + struct gcc_target targetm = TARGET_INITIALIZER; /* Adjust the stack and return the number of bytes taken to do it. */ @@ -2557,9 +2561,6 @@ conditionalize_optimization (void) continue; } -static int saved_warn_return_type = -1; -static int saved_warn_return_type_count = 0; - /* This is to handle loads from the constant pool. */ static void @@ -2568,21 +2569,6 @@ mcore_reorg (void) /* Reset this variable. */ current_function_anonymous_args = 0; - /* Restore the warn_return_type if it has been altered. */ - if (saved_warn_return_type != -1) - { - /* Only restore the value if we have reached another function. - The test of warn_return_type occurs in final_function () in - c-decl.c a long time after the code for the function is generated, - so we need a counter to tell us when we have finished parsing that - function and can restore the flag. */ - if (--saved_warn_return_type_count == 0) - { - warn_return_type = saved_warn_return_type; - saved_warn_return_type = -1; - } - } - if (optimize == 0) return; @@ -3012,25 +2998,7 @@ static tree mcore_handle_naked_attribute (tree * node, tree name, tree args ATTRIBUTE_UNUSED, int flags ATTRIBUTE_UNUSED, bool * no_add_attrs) { - if (TREE_CODE (*node) == FUNCTION_DECL) - { - /* PR14310 - don't complain about lack of return statement - in naked functions. The solution here is a gross hack - but this is the only way to solve the problem without - adding a new feature to GCC. I did try submitting a patch - that would add such a new feature, but it was (rightfully) - rejected on the grounds that it was creeping featurism, - so hence this code. */ - if (warn_return_type) - { - saved_warn_return_type = warn_return_type; - warn_return_type = 0; - saved_warn_return_type_count = 2; - } - else if (saved_warn_return_type_count) - saved_warn_return_type_count = 2; - } - else + if (TREE_CODE (*node) != FUNCTION_DECL) { warning (OPT_Wattributes, "%qE attribute only applies to functions", name); @@ -3082,6 +3050,14 @@ mcore_naked_function_p (void) return lookup_attribute ("naked", DECL_ATTRIBUTES (current_function_decl)) != NULL_TREE; } +static bool +mcore_warn_func_return (void) +{ + /* Naked functions are implemented entirely in assembly, including the + return sequence, so suppress warnings about this. */ + return !mcore_naked_function_p (); +} + #ifdef OBJECT_FORMAT_ELF static void mcore_asm_named_section (const char *name, Index: gcc/config/arm/arm.c =================================================================== --- gcc/config/arm/arm.c (revision 160360) +++ gcc/config/arm/arm.c (working copy) @@ -212,6 +212,7 @@ static bool arm_tls_symbol_p (rtx x); static int arm_issue_rate (void); static void arm_output_dwarf_dtprel (FILE *, int, rtx) ATTRIBUTE_UNUSED; static bool arm_allocate_stack_slots_for_args (void); +static bool arm_warn_func_return (void); static const char *arm_invalid_parameter_type (const_tree t); static const char *arm_invalid_return_type (const_tree t); static tree arm_promoted_type (const_tree t); @@ -377,6 +378,9 @@ static const struct attribute_spec arm_a #undef TARGET_TRAMPOLINE_ADJUST_ADDRESS #define TARGET_TRAMPOLINE_ADJUST_ADDRESS arm_trampoline_adjust_address +#undef TARGET_WARN_FUNC_RETURN +#define TARGET_WARN_FUNC_RETURN arm_warn_func_return + #undef TARGET_DEFAULT_SHORT_ENUMS #define TARGET_DEFAULT_SHORT_ENUMS arm_default_short_enums @@ -1992,6 +1996,14 @@ arm_allocate_stack_slots_for_args (void) return !IS_NAKED (arm_current_func_type ()); } +static bool +arm_warn_func_return (void) +{ + /* Naked functions are implemented entirely in assembly, including the + return sequence, so suppress warnings about this. */ + return !IS_NAKED (arm_current_func_type ()); +} + /* Output assembler code for a block containing the constant parts of a trampoline, leaving space for the variable parts.