public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [C++ PATCH] Avoid -Wreturn-type warnings if a switch has default label, no breaks inside of it, but is followed by a break (PR sanitizer/81275)
@ 2017-11-25  0:37 Jakub Jelinek
  2017-11-25 10:16 ` Jakub Jelinek
  2017-11-27 12:44 ` Nathan Sidwell
  0 siblings, 2 replies; 9+ messages in thread
From: Jakub Jelinek @ 2017-11-25  0:37 UTC (permalink / raw)
  To: Jason Merrill, Nathan Sidwell; +Cc: gcc-patches

Hi!

The testcase below has a useless break; that causes a bogus -Wreturn-type
warning.  The C++ FE already has code to avoid adding a BREAK_STMT
after a return or similar sequence that is known not to return.
The following patch extends block_may_fallthrough to also return false
for SWITCH_STMTs that can't fall through.

Those are ones with non-empty body where the whole body can't fallthrough,
additionally they need to have a default: case label (or cover the whole
range of values, but that is not what this patch can compute, that would
be too big duplication of the gimplification processing) and no BREAK_STMT.

For the default: case label we need to look in all SWITCH_BODY children
except for nested SWITCH_STMTs, for BREAK_STMTs also not in
{FOR,DO,WHILE}_BODY.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2017-11-24  Jakub Jelinek  <jakub@redhat.com>

	PR sanitizer/81275
	* cp-objcp-common.c (struct find_default_and_break_s): New type.
	(find_default_and_break): New function.
	(cxx_block_may_fallthru): Return false for SWITCH_STMT which
	contains no BREAK_STMTs and contains a default: CASE_LABEL_EXPR.

	* g++.dg/warn/pr81275.C: New test.

--- gcc/cp/cp-objcp-common.c.jj	2017-11-15 09:38:25.000000000 +0100
+++ gcc/cp/cp-objcp-common.c	2017-11-24 20:08:35.047132170 +0100
@@ -335,6 +335,96 @@ init_shadowed_var_for_decl (void)
     = hash_table<tree_decl_map_cache_hasher>::create_ggc (512);
 }
 
+/* Data structure for find_default_and_break.  */
+
+struct find_default_and_break_s
+{
+  hash_set<tree> *pset;
+  bool found_default;
+  bool look_for_break;
+};
+
+/* Helper function for cxx_block_may_fallthru SWITCH_STMT processing.
+   Determine if SWITCH_STMT body contains a BREAK_STMT and also record
+   if it contains default: CASE_LABEL_EXPR.  */
+
+static tree
+find_default_and_break (tree *stmt_p, int *walk_subtrees, void *d)
+{
+  find_default_and_break_s *data = (find_default_and_break_s *) d;
+  tree stmt = *stmt_p;
+  switch (TREE_CODE (stmt))
+    {
+    case FOR_STMT:
+      if (cp_walk_tree (&FOR_INIT_STMT (stmt), find_default_and_break,
+			d, data->pset)
+	  || cp_walk_tree (&FOR_COND (stmt), find_default_and_break,
+			   d, data->pset)
+	  || cp_walk_tree (&FOR_EXPR (stmt), find_default_and_break,
+			   d, data->pset))
+	return stmt;
+      stmt = FOR_BODY (stmt);
+      goto loop_body;
+    case WHILE_STMT:
+      if (cp_walk_tree (&WHILE_COND (stmt), find_default_and_break,
+			d, data->pset))
+	return stmt;
+      stmt = WHILE_BODY (stmt);
+      goto loop_body;
+    case DO_STMT:
+      if (cp_walk_tree (&DO_COND (stmt), find_default_and_break,
+			d, data->pset))
+	return stmt;
+      stmt = DO_BODY (stmt);
+    loop_body:
+      *walk_subtrees = 0;
+      /* Inside of {FOR,WHILE,DO}_BODY we can ignore BREAK_STMTs that
+	 are related to the corresponding loop, not the outer SWITCH_STMT.
+	 If we haven't still found a default:, we need to look for it
+	 though.  */
+      if (!data->found_default)
+	{
+	  if (data->look_for_break)
+	    {
+	      if (cp_walk_tree (&stmt, find_default_and_break, d, data->pset))
+		return *stmt_p;
+	    }
+	  else
+	    {
+	      data->look_for_break = true;
+	      if (cp_walk_tree (&stmt, find_default_and_break, d, data->pset))
+		{
+		  data->look_for_break = false;
+		  return *stmt_p;
+		}
+	      data->look_for_break = false;
+	    }
+	}
+      return NULL_TREE;
+    case CASE_LABEL_EXPR:
+      if (CASE_LOW (stmt) == NULL_TREE && CASE_HIGH (stmt) == NULL_TREE)
+	{
+	  /* Found default:.  */
+	  data->found_default = true;
+	  if (!data->look_for_break)
+	    return stmt;
+	}
+      return NULL_TREE;
+    case BREAK_STMT:
+      /* Found a break that can fall through out of the SWITCH_STMT.  */
+      return data->look_for_break ? stmt : NULL_TREE;
+    case SWITCH_STMT:
+      /* Nested switch can't contain a break; nor default: for the outer
+	 switch.  */
+      *walk_subtrees = 0;
+      return NULL_TREE;
+    default:
+      if (TYPE_P (stmt))
+	*walk_subtrees = 0;
+      return NULL_TREE;
+    }
+}
+
 /* Return true if stmt can fall through.  Used by block_may_fallthru
    default case.  */
 
@@ -349,6 +439,25 @@ cxx_block_may_fallthru (const_tree stmt)
     case THROW_EXPR:
       return false;
 
+    case SWITCH_STMT:
+      if (SWITCH_STMT_BODY (stmt) == NULL_TREE
+	  || block_may_fallthru (SWITCH_STMT_BODY (stmt)))
+	return true;
+      else
+	{
+	  /* If the switch stmt body can't fall through, it can still
+	     fall through if there is no default: label (checking whether
+	     without default: case labels cover the whole range might be
+	     too expensive) or if there are any break; stmts.  */
+	  hash_set<tree> pset;
+	  find_default_and_break_s data = { &pset, false, true };
+	  if (cp_walk_tree (&SWITCH_STMT_BODY (stmt), find_default_and_break,
+			    &data, &pset)
+	      || !data.found_default)
+	    return true;
+	  return false;
+	}
+
     default:
       return true;
     }
--- gcc/testsuite/g++.dg/warn/pr81275.C.jj	2017-11-24 20:24:08.232869743 +0100
+++ gcc/testsuite/g++.dg/warn/pr81275.C	2017-11-24 20:23:38.000000000 +0100
@@ -0,0 +1,29 @@
+// PR sanitizer/81875
+// { dg-do compile }
+// { dg-options "-Wreturn-type" }
+
+struct C { C (); ~C (); };
+
+int
+foo (int a, int b)
+{
+  C c;
+  switch (a)
+    {
+    case 0:
+      switch (b)
+        {
+	case 13:
+	  return 7;
+	case 24:
+	  return 19;
+	default:
+	  return 0;
+	}
+      break;
+    default:
+      return 0;
+    case 9:
+      return 17;
+    }
+}	// { dg-bogus "control reaches end of non-void function" }

	Jakub

^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-11-28 11:55 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-25  0:37 [C++ PATCH] Avoid -Wreturn-type warnings if a switch has default label, no breaks inside of it, but is followed by a break (PR sanitizer/81275) Jakub Jelinek
2017-11-25 10:16 ` Jakub Jelinek
2017-11-26  9:05   ` Jakub Jelinek
2017-11-27 13:39     ` Nathan Sidwell
2017-11-27 14:10       ` Jakub Jelinek
2017-11-28  9:14         ` [C++ PATCH] Avoid -Wreturn-type warnings if a switch has default label, no breaks inside of it, but is followed by a break (PR sanitizer/81275, take 2) Jakub Jelinek
2017-11-28 11:58           ` Nathan Sidwell
2017-11-27 12:49   ` [C++ PATCH] Avoid -Wreturn-type warnings if a switch has default label, no breaks inside of it, but is followed by a break (PR sanitizer/81275) Nathan Sidwell
2017-11-27 12:44 ` Nathan Sidwell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).