This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Possible patch for tree-optimization/13000


Richard Henderson <rth@redhat.com> writes:

> On Wed, Jan 19, 2005 at 03:08:51PM -0800, Richard Henderson wrote:
> > (At minimum, the 
> > behaviour of SWITCH_EXPR isn't the same until gimplification is 
> > complete.  Perhaps it's a mistake to give the two behaviours the
> > same name...)
> 
> Not quite true.  We could check SWITCH_LABELS vs SWITCH_BODY in 
> block_may_fallthru.  Only one of the two should be non-null, and
> which is set determines its behaviour.

Thanks for the pointer.

I found another false positive, on cp_parser_selection_statement in
cp/parser.c.  This is a case where a break statement follows a return
statement, causing the simple tests in block_may_fallthru to think
that the function does return.  I hacked around this by discarding
break statements which are being appended to blocks which do not fall
through.  I could have changed the C frontend to discard all
statements appended to blocks which do not fall through, but that
seemed too expensive.  This patch assumes that block_may_fallthru is
safe when called with language specific tree nodes, which is currently
the case, as block_may_fallthru simply assumes that anything it does
not specifically handle will fall through.

So this is my current patch for PR 13000.  This adds the warning, and
also works around the two false positives found in a gcc bootstrap.
With this patch, I completed a bootstrap (c/c++/f95/java/objc) on
i686-pc-linux-gnu, and completed a testsuite run with no additional
failures.

Is this patch OK for mainline?  Or is the code working around the
false positives to hacked up?

Ian


gcc/ChangeLog
2005-01-20  Ian Lance Taylor  <ian@airs.com>

	PR tree-optimization/13000
	* tree-inline.c: Include "tree-flow.h".
	(expand_call_inline): If warn_return_type, warn if non-void inline
	function falls through.
	* tree-cfg.c (execute_warn_function_return): Don't warn about
	control reaching end if TREE_NO_WARNING is set.  Set
	TREE_NO_WARNING.
	* gimple-low.c (block_may_fallthru): Don't assume that SWITCH_EXPR
	has been lowered.
	* gimplify.c (shortcut_cond_expr): Don't emit a jump over the else
	branch if we don't need one.
	* c-typeck.c: Include "tree-flow.h"
	(c_finish_bc_stmt): Don't add a goto if the current statement
	list doesn't fall through to the current point.


testsuite/ChangeLog:
2005-01-20  Ian Lance Taylor  <ian@c2micro.com>

	PR tree-optimization/13000
	* gcc.dg/20040206-1.c: Change warning to point where function is
	being inlined.


Index: tree-inline.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-inline.c,v
retrieving revision 1.166
diff -p -u -r1.166 tree-inline.c
--- tree-inline.c	18 Jan 2005 23:06:59 -0000	1.166
+++ tree-inline.c	21 Jan 2005 02:35:00 -0000
@@ -41,6 +41,7 @@ Boston, MA 02111-1307, USA.  */
 #include "cgraph.h"
 #include "intl.h"
 #include "tree-mudflap.h"
+#include "tree-flow.h"
 #include "function.h"
 #include "diagnostic.h"
 #include "debug.h"
@@ -1607,9 +1608,22 @@ expand_call_inline (tree *tp, int *walk_
      function itself.  */
   {
     struct cgraph_node *old_node = id->current_node;
+    tree copy;
 
     id->current_node = edge->callee;
-    append_to_statement_list (copy_body (id), &BIND_EXPR_BODY (expr));
+    copy = copy_body (id);
+
+    if (warn_return_type
+	&& !TREE_NO_WARNING (fn)
+	&& !VOID_TYPE_P (TREE_TYPE (TREE_TYPE (fn)))
+	&& block_may_fallthru (copy))
+      {
+	warning ("control may reach end of non-void function %qD being inlined",
+		 fn);
+	TREE_NO_WARNING (fn) = 1;
+      }
+
+    append_to_statement_list (copy, &BIND_EXPR_BODY (expr));
     id->current_node = old_node;
   }
   inlined_body = &BIND_EXPR_BODY (expr);
Index: tree-cfg.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-cfg.c,v
retrieving revision 2.143
diff -p -u -r2.143 tree-cfg.c
--- tree-cfg.c	20 Jan 2005 23:42:38 -0000	2.143
+++ tree-cfg.c	21 Jan 2005 02:34:59 -0000
@@ -5677,6 +5677,7 @@ execute_warn_function_return (void)
   /* If we see "return;" in some basic block, then we do reach the end
      without returning a value.  */
   else if (warn_return_type
+	   && !TREE_NO_WARNING (cfun->decl)
 	   && EDGE_COUNT (EXIT_BLOCK_PTR->preds) > 0
 	   && !VOID_TYPE_P (TREE_TYPE (TREE_TYPE (cfun->decl))))
     {
@@ -5697,6 +5698,7 @@ execute_warn_function_return (void)
 		locus = &cfun->function_end_locus;
 	      warning ("%Hcontrol reaches end of non-void function", locus);
 #endif
+	      TREE_NO_WARNING (cfun->decl) = 1;
 	      break;
 	    }
 	}
Index: gimple-low.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/gimple-low.c,v
retrieving revision 2.17
diff -p -u -r2.17 gimple-low.c
--- gimple-low.c	18 Jan 2005 11:36:14 -0000	2.17
+++ gimple-low.c	21 Jan 2005 02:34:56 -0000
@@ -278,11 +278,17 @@ block_may_fallthru (tree block)
     case GOTO_EXPR:
     case RETURN_EXPR:
     case RESX_EXPR:
-    case SWITCH_EXPR:
       /* Easy cases.  If the last statement of the block implies 
 	 control transfer, then we can't fall through.  */
       return false;
 
+    case SWITCH_EXPR:
+      /* If SWITCH_LABELS is set, this is lowered, and represents a
+	 branch to a selected label and hence can not fall through.
+	 Otherwise SWITCH_BODY is set, and the switch can fall
+	 through.  */
+      return SWITCH_LABELS (stmt) != NULL_TREE;
+
     case COND_EXPR:
       if (block_may_fallthru (COND_EXPR_THEN (stmt)))
 	return true;
Index: gimplify.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/gimplify.c,v
retrieving revision 2.104
diff -p -u -r2.104 gimplify.c
--- gimplify.c	19 Jan 2005 09:27:18 -0000	2.104
+++ gimplify.c	21 Jan 2005 02:34:56 -0000
@@ -1911,7 +1911,7 @@ shortcut_cond_expr (tree expr)
   tree true_label, false_label, end_label, t;
   tree *true_label_p;
   tree *false_label_p;
-  bool emit_end, emit_false;
+  bool emit_end, emit_false, jump_over_else;
   bool then_se = then_ && TREE_SIDE_EFFECTS (then_);
   bool else_se = else_ && TREE_SIDE_EFFECTS (else_);
 
@@ -2013,6 +2013,16 @@ shortcut_cond_expr (tree expr)
   emit_end = (end_label == NULL_TREE);
   emit_false = (false_label == NULL_TREE);
 
+  /* We only emit the jump over the else clause if we have to--if the
+     then clause may fall through.  Otherwise we can wind up with a
+     useless jump and a useless label at the end of gimplified code,
+     which will cause us to think that this conditional as a whole
+     falls through even if it doesn't.  If we then inline a function
+     which ends with such a condition, that can cause us to issue an
+     inappropriate warning about control reaching the end of a
+     non-void function.  */
+  jump_over_else = block_may_fallthru (then_);
+
   pred = shortcut_cond_r (pred, true_label_p, false_label_p);
 
   expr = NULL;
@@ -2021,8 +2031,11 @@ shortcut_cond_expr (tree expr)
   append_to_statement_list (then_, &expr);
   if (else_se)
     {
-      t = build_and_jump (&end_label);
-      append_to_statement_list (t, &expr);
+      if (jump_over_else)
+	{
+	  t = build_and_jump (&end_label);
+	  append_to_statement_list (t, &expr);
+	}
       if (emit_false)
 	{
 	  t = build1 (LABEL_EXPR, void_type_node, false_label);
Index: c-typeck.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/c-typeck.c,v
retrieving revision 1.411
diff -p -u -r1.411 c-typeck.c
--- c-typeck.c	19 Jan 2005 09:27:17 -0000	1.411
+++ c-typeck.c	21 Jan 2005 02:34:53 -0000
@@ -43,6 +43,7 @@ Software Foundation, 59 Temple Place - S
 #include "target.h"
 #include "tree-iterator.h"
 #include "tree-gimple.h"
+#include "tree-flow.h"
 
 /* Possible cases of implicit bad conversions.  Used to select
    diagnostic messages in convert_for_assignment.  */
@@ -6764,6 +6765,16 @@ c_finish_bc_stmt (tree *label_p, bool is
 {
   tree label = *label_p;
 
+  /* In switch statements break is sometimes stylistically used after
+     a return statement.  This can lead to spurious warnings about
+     control reaching the end of a non-void function when it is
+     inlined.  Note that we are calling block_may_fallthru with
+     language specific tree nodes; this works because
+     block_may_fallthru returns true when given something it does not
+     understand.  */
+  if (! block_may_fallthru (cur_stmt_list))
+    return NULL_TREE;
+
   if (!label)
     *label_p = label = create_artificial_label ();
   else if (TREE_CODE (label) != LABEL_DECL)
Index: testsuite/gcc.dg/20040206-1.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/testsuite/gcc.dg/20040206-1.c,v
retrieving revision 1.4
diff -p -u -r1.4 20040206-1.c
--- testsuite/gcc.dg/20040206-1.c	17 Jan 2005 17:17:02 -0000	1.4
+++ testsuite/gcc.dg/20040206-1.c	21 Jan 2005 02:35:06 -0000
@@ -7,5 +7,5 @@
     The warning about "no return statement in function
     returning non-void" is PR 13000. */
 
-static int foo (int a __attribute__((unused)) ) { }  /* { dg-warning "return" "" { xfail *-*-* } } */
-int main (void) { return foo (0); }
+static int foo (int a __attribute__((unused)) ) { }
+int main (void) { return foo (0); } /* { dg-warning "control may reach end" } */


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]