[C PATCH] Handle C SWITCH_EXPR in block_may_fallthru (PR sanitizer/81275)

[C PATCH] Handle C SWITCH_EXPR in block_may_fallthru (PR sanitizer/81275)

[Jakub Jelinek]

Hi!

This is the C version of the switch block_may_fallthru handling.
Unlike C++ SWITCH_STMT, break; is represented in SWITCH_EXPR by a goto
to a label emitted after the SWITCH_EXPR, so either block_may_fallthru
finds such label (but then doesn't find the SWITCH_EXPR), or it
finds SWITCH_EXPR, in which case if the body doesn't fall through (e.g.
ends with a return stmt), then it may fall through only if it doesn't
cover all the cases.

This patch adds a bit that signals that, and computes whether all cases
are covered (either if default: is present, or by walking the splay tree).

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

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

	PR sanitizer/81275
	* tree.c (block_may_fallthru): Return false if SWITCH_ALL_CASES_P
	is set on SWITCH_EXPR and !block_may_fallthru (SWITCH_BODY ()).
c/
	* c-typeck.c (c_finish_case): Set SWITCH_ALL_CASES_P if
	c_switch_covers_all_cases_p returns true.
c-family/
	* c-common.c (c_switch_covers_all_cases_p_1,
	c_switch_covers_all_cases_p): New functions.
	* c-common.h (c_switch_covers_all_cases_p): Declare.
testsuite/
	* c-c++-common/tsan/pr81275.c: New test.

--- gcc/tree.c.jj	2017-11-27 14:36:09.000000000 +0100
+++ gcc/tree.c	2017-11-27 15:11:15.715131528 +0100
@@ -12339,6 +12339,12 @@ block_may_fallthru (const_tree block)
       return false;
 
     case SWITCH_EXPR:
+      /* If there is a default: label or case labels cover all possible
+	 SWITCH_COND values, then the SWITCH_EXPR will transfer control
+	 to some case label in all cases and all we care is whether the
+	 SWITCH_BODY falls through.  */
+      if (SWITCH_ALL_CASES_P (stmt))
+	return block_may_fallthru (SWITCH_BODY (stmt));
       return true;
 
     case COND_EXPR:
--- gcc/tree.h.jj	2017-11-27 14:34:38.000000000 +0100
+++ gcc/tree.h	2017-11-27 15:08:23.510250289 +0100
@@ -1175,6 +1175,10 @@ extern void protected_set_expr_location
 /* SWITCH_EXPR accessors. These give access to the condition and body.  */
 #define SWITCH_COND(NODE)       TREE_OPERAND (SWITCH_EXPR_CHECK (NODE), 0)
 #define SWITCH_BODY(NODE)       TREE_OPERAND (SWITCH_EXPR_CHECK (NODE), 1)
+/* True if there all case labels for all possible values of SWITCH_COND, either
+   because there is a default: case label or because the case label ranges cover
+   all values.  */
+#define SWITCH_ALL_CASES_P(NODE) (SWITCH_EXPR_CHECK (NODE)->base.private_flag)
 
 /* CASE_LABEL_EXPR accessors. These give access to the high and low values
    of a case label, respectively.  */
--- gcc/c/c-typeck.c.jj	2017-11-27 14:27:53.000000000 +0100
+++ gcc/c/c-typeck.c	2017-11-27 16:00:03.180982468 +0100
@@ -10407,6 +10407,8 @@ c_finish_case (tree body, tree type)
 			type ? type : TREE_TYPE (cs->switch_expr),
 			SWITCH_COND (cs->switch_expr),
 			cs->bool_cond_p, cs->outside_range_p);
+  if (c_switch_covers_all_cases_p (cs->cases, TREE_TYPE (cs->switch_expr)))
+    SWITCH_ALL_CASES_P (cs->switch_expr) = 1;
 
   /* Pop the stack.  */
   c_switch_stack = cs->next;
--- gcc/c-family/c-common.h.jj	2017-11-20 19:55:39.000000000 +0100
+++ gcc/c-family/c-common.h	2017-11-27 16:08:30.740827358 +0100
@@ -975,6 +975,7 @@ extern int case_compare (splay_tree_key,
 
 extern tree c_add_case_label (location_t, splay_tree, tree, tree, tree, tree,
 			      bool *);
+extern bool c_switch_covers_all_cases_p (splay_tree, tree);
 
 extern tree build_function_call (location_t, tree, tree);
 
--- gcc/c-family/c-common.c.jj	2017-11-21 14:56:50.000000000 +0100
+++ gcc/c-family/c-common.c	2017-11-27 16:09:52.555839861 +0100
@@ -4904,6 +4904,64 @@ c_add_case_label (location_t loc, splay_
   return error_mark_node;
 }
 
+/* Subroutine of c_switch_covers_all_cases_p, called via
+   splay_tree_foreach.  Return 1 if it doesn't cover all the cases.
+   ARGS[0] is initially NULL and after the first iteration is the
+   so far highest case label.  ARGS[1] is the minimum of SWITCH_COND's
+   type.  */
+
+static int
+c_switch_covers_all_cases_p_1 (splay_tree_node node, void *data)
+{
+  tree label = (tree) node->value;
+  tree *args = (tree *) data;
+
+  /* If there is a default case, we shouldn't have called this.  */
+  gcc_assert (CASE_LOW (label));
+
+  if (args[0] == NULL_TREE)
+    {
+      if (wi::to_widest (args[1]) < wi::to_widest (CASE_LOW (label)))
+	return 1;
+    }
+  else if (wi::add (wi::to_widest (args[0]), 1)
+	   != wi::to_widest (CASE_LOW (label)))
+    return 1;
+  if (CASE_HIGH (label))
+    args[0] = CASE_HIGH (label);
+  else
+    args[0] = CASE_LOW (label);
+  return 0;
+}
+
+/* Return true if switch with CASES and switch condition with type
+   covers all possible values in the case labels.  */
+
+bool
+c_switch_covers_all_cases_p (splay_tree cases, tree type)
+{
+  /* If there is default:, this is always the case.  */
+  splay_tree_node default_node
+    = splay_tree_lookup (cases, (splay_tree_key) NULL);
+  if (default_node)
+    return true;
+
+  if (!INTEGRAL_TYPE_P (type))
+    return false;
+
+  tree args[2] = { NULL_TREE, TYPE_MIN_VALUE (type) };
+  if (splay_tree_foreach (cases, c_switch_covers_all_cases_p_1, args))
+    return false;
+
+  /* If there are no cases at all, or if the highest case label
+     is smaller than TYPE_MAX_VALUE, return false.  */
+  if (args[0] == NULL_TREE
+      || wi::to_widest (args[0]) < wi::to_widest (TYPE_MAX_VALUE (type)))
+    return false;
+
+  return true;
+}
+
 /* Finish an expression taking the address of LABEL (an
    IDENTIFIER_NODE).  Returns an expression for the address.
 
--- gcc/testsuite/c-c++-common/tsan/pr81275.c.jj	2017-11-27 16:22:25.674749806 +0100
+++ gcc/testsuite/c-c++-common/tsan/pr81275.c	2017-11-27 16:20:42.000000000 +0100
@@ -0,0 +1,111 @@
+/* PR sanitizer/81275 */
+/* { dg-do compile } */
+/* { dg-options "-Wreturn-type -fsanitize=thread" } */
+
+int
+f1 (int a, int b)
+{
+  switch (a)
+    {
+    case 0:
+      switch (b)
+        {
+        case 5:
+	  return 6;
+	case 7:
+	  return 8;
+	default:
+	  return 0;
+	}
+      break;
+    default:
+      return 0;
+    }
+}	/* { dg-bogus "control reaches end of non-void function" } */
+
+int
+f2 (int a, int b)
+{
+  switch (a)
+    {
+    case 0:
+      switch (b)
+        {
+        case 5:
+	  return 6;
+	case 7:
+	  return 8;
+	default:
+	  return 0;
+	}
+    default:
+      return 0;
+    }
+}	/* { dg-bogus "control reaches end of non-void function" } */
+
+int
+f3 (int a, int b)
+{
+  switch (a)
+    {
+    case 0:
+      switch (b)
+        {
+        case 5:
+	  return 6;
+	case 7:
+	  return 8;
+	case 8:
+	  break;
+	default:
+	  return 0;
+	}
+      break;
+    default:
+      return 0;
+    }
+}	/* { dg-warning "control reaches end of non-void function" } */
+
+int
+f4 (int a, int b)
+{
+  switch (a)
+    {
+    case 0:
+      switch (b)
+        {
+        case 5:
+	  return 6;
+	case 7:
+	  return 8;
+	}
+      break;
+    default:
+      return 0;
+    }
+}	/* { dg-warning "control reaches end of non-void function" } */
+
+int
+f5 (int a, unsigned char b)
+{
+  switch (a)
+    {
+    case 0:
+      switch (b)
+        {
+	case 0:
+	  return 1;
+	case 3 ... 10:
+	  return 2;
+	case 1 ... 2:
+	  return 3;
+	case 126 ... (unsigned char) ~0:
+	  return 4;
+	case 11 ... 125:
+	  return 5;
+	}
+      break;
+    default:
+      return 0;
+    }
+}	/* { dg-bogus "control reaches end of non-void function" } */

	Jakub

Re: [C PATCH] Handle C SWITCH_EXPR in block_may_fallthru (PR sanitizer/81275)

[Jeff Law]

On 11/28/2017 01:49 AM, Jakub Jelinek wrote:
> Hi!
> 
> This is the C version of the switch block_may_fallthru handling.
> Unlike C++ SWITCH_STMT, break; is represented in SWITCH_EXPR by a goto
> to a label emitted after the SWITCH_EXPR, so either block_may_fallthru
> finds such label (but then doesn't find the SWITCH_EXPR), or it
> finds SWITCH_EXPR, in which case if the body doesn't fall through (e.g.
> ends with a return stmt), then it may fall through only if it doesn't
> cover all the cases.
> 
> This patch adds a bit that signals that, and computes whether all cases
> are covered (either if default: is present, or by walking the splay tree).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2017-11-28  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR sanitizer/81275
> 	* tree.c (block_may_fallthru): Return false if SWITCH_ALL_CASES_P
> 	is set on SWITCH_EXPR and !block_may_fallthru (SWITCH_BODY ()).
> c/
> 	* c-typeck.c (c_finish_case): Set SWITCH_ALL_CASES_P if
> 	c_switch_covers_all_cases_p returns true.
> c-family/
> 	* c-common.c (c_switch_covers_all_cases_p_1,
> 	c_switch_covers_all_cases_p): New functions.
> 	* c-common.h (c_switch_covers_all_cases_p): Declare.
> testsuite/
> 	* c-c++-common/tsan/pr81275.c: New test.
OK.
jeff