Patchwork Invalid warnings for naked functions

login
register
mail settings
Submitter Paul Brook
Date June 10, 2010, 12:23 p.m.
Message ID <201006101323.27637.paul@codesourcery.com>
Download mbox | patch
Permalink /patch/55206/
State New
Headers show

Comments

Paul Brook - June 10, 2010, 12:23 p.m.
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  <paul@codesourcery.com>
 
	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.
Paolo Bonzini - June 10, 2010, 5:54 p.m.
On 06/10/2010 02:23 PM, Paul Brook wrote:
> 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.

lang_hooks.missing_noreturn_ok_p is very similar.  Maybe the two uses of 
the langhook could check the target hook as well?  For example a naked 
function might seem to be noreturn but it could return via an asm 
statement or some other mystery.

Paolo
Paul Brook - June 11, 2010, 2:15 a.m.
> On 06/10/2010 02:23 PM, Paul Brook wrote:
> > 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.
> 
> lang_hooks.missing_noreturn_ok_p is very similar.  Maybe the two uses of
> the langhook could check the target hook as well?  For example a naked
> function might seem to be noreturn but it could return via an asm
> statement or some other mystery.

I don't think this will ever occur, given the only thing you're supposed to 
put in naked functions is asm statements.

Paul

Patch

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.