Patchwork PR target/53633; disable return value warnings for naked functions

login
register
mail settings
Submitter Sandra Loosemore
Date July 23, 2012, 4:37 p.m.
Message ID <500D7DE2.7090204@codesourcery.com>
Download mbox | patch
Permalink /patch/172711/
State New
Headers show

Comments

Sandra Loosemore - July 23, 2012, 4:37 p.m.
This is a revised version of Paul Brook's patch from two years ago:

http://gcc.gnu.org/ml/gcc-patches/2010-06/msg01088.html

I've updated the patch per the review comments from that time, and also extended it to handle a similar warning from the C++ front end.

I have so far only tested this on arm-none-eabi and by bootstrapping and regression-testing x86_64-linux-gnu native.  I'm not set up to test on spu, rx, avr, or mcore, but I'm thinking that this needs at least an mcore build and hand-testing of the new compilation test cases due to the removal of the existing hack for this problem in that backend.  Nick, you're listed as mcore port maintainer; can you help?

-Sandra


2012-07-23  Sandra Loosemore  <sandra@codesourcery.com>
	    Paul Brook  <paul@codesourcery.com>

	PR target/53633

	gcc/
	* target.def (warn_func_return): New hook.
	* doc/tm.texi.in (TARGET_WARN_FUNC_RETURN): New hook.
	* doc/tm.texi: Regenerate.
	* target.h (gcc_target): Add warn_func_return.
	* ipa-pure-const.c (warn_function_noreturn): Check
	targetm.warn_func_return.
	* tree-cfg.h (execute_warn_function_return): Likewise.
	* 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/cp/
	* decl.c (finish_function): Check targetm.warn_func_return.

	gcc/testsuite/
	* gcc.target/arm/naked-3.c: New test.
	* g++.dg/ext/attr-naked.C: New test.
Nick Clifton - July 24, 2012, 11:18 a.m.
Hi Sandra,

> I've updated the patch

One suggestion - rather than having architecture specific test files, 
why not just have a single generic test case with a new 
dg-require-naked-attribute qualifier.  That way the mcore port would be 
tested as well as the ARM port.

> I'm not set up to test on spu, rx, avr, or mcore,

The RX part of the port is OK too.

> but I'm thinking that this needs at least an mcore build
> and hand-testing of the new compilation test cases due to
> the removal of the existing hack for this problem in that
> backend.

I have done this and the mcore part of the patch works.  (The mcore port 
itself does not build libgcc, but this is a separate, known problem that 
I am working on in my spare time).

Cheers
   Nick

Patch

Index: gcc/target.def
===================================================================
--- gcc/target.def	(revision 189734)
+++ gcc/target.def	(working copy)
@@ -2710,6 +2710,15 @@  DEFHOOK
  void, (struct hard_reg_set_container *),
  NULL)
 
+/* For targets that have attributes that can affect whether a
+   function's return statements need checking.  For instance a 'naked'
+   function attribute.  */
+DEFHOOK
+(warn_func_return,
+ "True if a function's return statements should be checked for matching the function's return type.  This includes checking for falling off the end of a non-void function.  Return false if no such check should be made.",
+ bool, (tree),
+ hook_bool_tree_true)
+
 /* Determine the type of unwind info to emit for debugging.  */
 DEFHOOK
 (debug_unwind_info,
Index: gcc/doc/tm.texi.in
===================================================================
--- gcc/doc/tm.texi.in	(revision 189734)
+++ gcc/doc/tm.texi.in	(working copy)
@@ -4918,6 +4918,8 @@  FRAME_POINTER_REGNUM, ARG_POINTER_REGNUM
 
 @hook TARGET_SET_UP_BY_PROLOGUE
 
+@hook TARGET_WARN_FUNC_RETURN
+
 @node Stack Smashing Protection
 @subsection Stack smashing protection
 @cindex stack smashing protection
Index: gcc/doc/tm.texi
===================================================================
--- gcc/doc/tm.texi	(revision 189734)
+++ gcc/doc/tm.texi	(working copy)
@@ -4977,6 +4977,10 @@  FRAME_POINTER_REGNUM, ARG_POINTER_REGNUM
 This hook should add additional registers that are computed by the prologue to the hard regset for shrink-wrapping optimization purposes.
 @end deftypefn
 
+@deftypefn {Target Hook} bool TARGET_WARN_FUNC_RETURN (tree)
+True if a function's return statements should be checked for matching the function's return type.  This includes checking for falling off the end of a non-void function.  Return false if no such check should be made.
+@end deftypefn
+
 @node Stack Smashing Protection
 @subsection Stack smashing protection
 @cindex stack smashing protection
Index: gcc/ipa-pure-const.c
===================================================================
--- gcc/ipa-pure-const.c	(revision 189734)
+++ gcc/ipa-pure-const.c	(working copy)
@@ -186,7 +186,8 @@  void
 warn_function_noreturn (tree decl)
 {
   static struct pointer_set_t *warned_about;
-  if (!lang_hooks.missing_noreturn_ok_p (decl))
+  if (!lang_hooks.missing_noreturn_ok_p (decl)
+      && targetm.warn_func_return (decl))
     warned_about 
       = suggest_attribute (OPT_Wsuggest_attribute_noreturn, decl,
 			   true, warned_about, "noreturn");
Index: gcc/tree-cfg.c
===================================================================
--- gcc/tree-cfg.c	(revision 189734)
+++ gcc/tree-cfg.c	(working copy)
@@ -40,6 +40,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.  */
@@ -7613,6 +7614,9 @@  execute_warn_function_return (void)
   edge e;
   edge_iterator ei;
 
+  if (!targetm.warn_func_return (cfun->decl))
+    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 189734)
+++ gcc/config/spu/spu.c	(working copy)
@@ -5881,6 +5881,14 @@  spu_trampoline_init (rtx m_tramp, tree f
   emit_insn (gen_sync ());
 }
 
+static bool
+spu_warn_func_return (tree decl)
+{
+  /* Naked functions are implemented entirely in assembly, including the
+     return sequence, so suppress warnings about this.  */
+  return !spu_naked_function_p (decl);
+}
+
 void
 spu_expand_sign_extend (rtx ops[])
 {
@@ -7267,6 +7275,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
+
 #undef TARGET_OPTION_OVERRIDE
 #define TARGET_OPTION_OVERRIDE spu_option_override
 
Index: gcc/config/rx/rx.c
===================================================================
--- gcc/config/rx/rx.c	(revision 189734)
+++ gcc/config/rx/rx.c	(working copy)
@@ -2629,6 +2629,14 @@  rx_func_attr_inlinable (const_tree decl)
     &&   ! is_naked_func (decl);  
 }
 
+static bool
+rx_warn_func_return (tree decl)
+{
+  /* Naked functions are implemented entirely in assembly, including the
+     return sequence, so suppress warnings about this.  */
+  return !is_naked_func (decl);
+}
+
 /* 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  */
 
@@ -3282,6 +3290,9 @@  rx_adjust_insn_length (rtx insn, int cur
 #undef  TARGET_LEGITIMIZE_ADDRESS
 #define TARGET_LEGITIMIZE_ADDRESS		rx_legitimize_address
 
+#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 189734)
+++ gcc/config/avr/avr.c	(working copy)
@@ -686,6 +686,17 @@  avr_can_eliminate (const int from, const
               && !frame_pointer_needed));
 }
 
+
+/* Implement TARGET_WARN_FUNC_RETURN.  */
+
+static bool
+avr_warn_func_return (tree decl)
+{
+  /* Naked functions are implemented entirely in assembly, including the
+     return sequence, so suppress warnings about this.  */
+  return !avr_naked_function_p (decl);
+}
+
 /* Compute offset between arg_pointer and frame_pointer.  */
 
 int
@@ -10790,6 +10801,9 @@  avr_fold_builtin (tree fndecl, int n_arg
 #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
+
 #undef  TARGET_CLASS_LIKELY_SPILLED_P
 #define TARGET_CLASS_LIKELY_SPILLED_P avr_class_likely_spilled_p
 
Index: gcc/config/arm/arm.c
===================================================================
--- gcc/config/arm/arm.c	(revision 189734)
+++ gcc/config/arm/arm.c	(working copy)
@@ -236,6 +236,7 @@  static int arm_issue_rate (void);
 static void arm_output_dwarf_dtprel (FILE *, int, rtx) ATTRIBUTE_UNUSED;
 static bool arm_output_addr_const_extra (FILE *, rtx);
 static bool arm_allocate_stack_slots_for_args (void);
+static bool arm_warn_func_return (tree);
 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);
@@ -458,6 +459,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
 
@@ -2168,6 +2172,14 @@  arm_allocate_stack_slots_for_args (void)
   return !IS_NAKED (arm_current_func_type ());
 }
 
+static bool
+arm_warn_func_return (tree decl)
+{
+  /* Naked functions are implemented entirely in assembly, including the
+     return sequence, so suppress warnings about this.  */
+  return lookup_attribute ("naked", DECL_ATTRIBUTES (decl)) == NULL_TREE;
+}
+
 
 /* Output assembler code for a block containing the constant parts
    of a trampoline, leaving space for the variable parts.
Index: gcc/config/mcore/mcore.c
===================================================================
--- gcc/config/mcore/mcore.c	(revision 189734)
+++ gcc/config/mcore/mcore.c	(working copy)
@@ -138,6 +138,7 @@  static unsigned int mcore_function_arg_b
 						 const_tree);
 static void       mcore_asm_trampoline_template (FILE *);
 static void       mcore_trampoline_init		(rtx, tree, rtx);
+static bool       mcore_warn_func_return        (tree);
 static void       mcore_option_override		(void);
 static bool       mcore_legitimate_constant_p   (enum machine_mode, rtx);
 
@@ -228,6 +229,9 @@  static const struct attribute_spec mcore
 #undef TARGET_LEGITIMATE_CONSTANT_P
 #define TARGET_LEGITIMATE_CONSTANT_P mcore_legitimate_constant_p
 
+#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.  */
@@ -2580,9 +2584,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
@@ -2591,21 +2592,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;
   
@@ -3056,25 +3042,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);
@@ -3126,6 +3094,14 @@  mcore_naked_function_p (void)
   return lookup_attribute ("naked", DECL_ATTRIBUTES (current_function_decl)) != NULL_TREE;
 }
 
+static bool
+mcore_warn_func_return (tree decl)
+{
+  /* Naked functions are implemented entirely in assembly, including the
+     return sequence, so suppress warnings about this.  */
+  return lookup_attribute ("naked", DECL_ATTRIBUTES (decl)) == NULL_TREE;
+}
+
 #ifdef OBJECT_FORMAT_ELF
 static void
 mcore_asm_named_section (const char *name, 
Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(revision 189734)
+++ gcc/cp/decl.c	(working copy)
@@ -13588,7 +13588,8 @@  finish_function (int flags)
       && !TREE_NO_WARNING (fndecl)
       /* Structor return values (if any) are set by the compiler.  */
       && !DECL_CONSTRUCTOR_P (fndecl)
-      && !DECL_DESTRUCTOR_P (fndecl))
+      && !DECL_DESTRUCTOR_P (fndecl)
+      && targetm.warn_func_return (fndecl))
     {
       warning (OPT_Wreturn_type,
  	       "no return statement in function returning non-void");
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/testsuite/g++.dg/ext/attr-naked.C
===================================================================
--- gcc/testsuite/g++.dg/ext/attr-naked.C	(revision 0)
+++ gcc/testsuite/g++.dg/ext/attr-naked.C	(revision 0)
@@ -0,0 +1,15 @@ 
+/* { dg-do compile { target arm*-*-* } } */
+/* { 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");
+}