[PATCH] Fix -fsanitize=float-cast-overflow with C FE (PR sanitizer/64344)

Jakub Jelinek jakub@redhat.com
Thu Dec 18 12:15:00 GMT 2014


On Tue, Dec 16, 2014 at 06:58:47PM +0000, Joseph Myers wrote:
> On Fri, 12 Dec 2014, Jakub Jelinek wrote:
> 
> > Hi!
> > 
> > -fsanitize=float-cast-overflow sanitization is done in convert.c and calls
> > there save_expr.  Unfortunately, save_expr is a no-go for the C FE, we need
> > c_save_expr, but as convert.c is shared by all FEs, the only way to arrange
> > that would be a new langhook.  This patch attempts to fix it the same way
> > as PR54428 did (the other save_expr in c-convert.c).
> > 
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> OK.

Unfortunately this broke stuff, as reported by Tobias.

The problems are:
1) sometimes convert is called after c_fully_fold, so we want
   in_late_binary_op set in that case and use save_expr instead of c_save_expr
2) if not in_late_binary_op, ubsan_instrument_float_cast wants to use the
   result of c_save_expr in comparisons (ok) and also as argument to
   a function call; but c_fully_fold will handle the former, but not look
   into CALL_EXPR arguments, so those need to be c_fully_folded
and, preexisting problems, where constant is required, things didn't work
well, because while the check was optimized out, we ended up with
COMPOUND_EXPR of 0 and some constant.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

2014-12-18  Jakub Jelinek  <jakub@redhat.com>

	PR sanitizer/64344
	* ubsan.h (ubsan_instrument_float_cast): Add ARG argument.
	* ubsan.c (ubsan_instrument_float_cast): Add ARG argument, pass
	it to libubsan handler instead of EXPR.  Fold comparisons earlier,
	if the result is integer_zerop, return NULL_TREE.
	* convert.c (convert_to_integer): Pass expr as ARG.
c/
	* c-typeck.c (convert_for_assignment, c_finish_return): For
	-fsanitize=float-cast-overflow casts from REAL_TYPE to integer/enum
	types also set in_late_binary_op around convert call.
	* c-convert.c (convert): For -fsanitize=float-cast-overflow REAL_TYPE
	to integral type casts, if not in_late_binary_op, pass c_fully_fold
	result on expr as last argument to ubsan_instrument_float_cast,
	if in_late_binary_op, don't use c_save_expr but save_expr.
testsuite/
	* c-c++-common/ubsan/pr64344-1.c: New test.
	* c-c++-common/ubsan/pr64344-2.c: New test.

--- gcc/ubsan.h.jj	2014-12-03 16:33:05.000000000 +0100
+++ gcc/ubsan.h	2014-12-18 09:57:12.934732565 +0100
@@ -47,7 +47,7 @@ extern tree ubsan_type_descriptor (tree,
 extern tree ubsan_encode_value (tree, bool = false);
 extern bool is_ubsan_builtin_p (tree);
 extern tree ubsan_build_overflow_builtin (tree_code, location_t, tree, tree, tree);
-extern tree ubsan_instrument_float_cast (location_t, tree, tree);
+extern tree ubsan_instrument_float_cast (location_t, tree, tree, tree);
 extern tree ubsan_get_source_location_type (void);
 
 #endif  /* GCC_UBSAN_H  */
--- gcc/ubsan.c.jj	2014-12-03 16:33:05.000000000 +0100
+++ gcc/ubsan.c	2014-12-18 10:47:00.045039211 +0100
@@ -1252,10 +1252,11 @@ instrument_bool_enum_load (gimple_stmt_i
 }
 
 /* Instrument float point-to-integer conversion.  TYPE is an integer type of
-   destination, EXPR is floating-point expression.  */
+   destination, EXPR is floating-point expression.  ARG is what to pass
+   the libubsan call as value, often EXPR itself.  */
 
 tree
-ubsan_instrument_float_cast (location_t loc, tree type, tree expr)
+ubsan_instrument_float_cast (location_t loc, tree type, tree expr, tree arg)
 {
   tree expr_type = TREE_TYPE (expr);
   tree t, tt, fn, min, max;
@@ -1348,6 +1349,12 @@ ubsan_instrument_float_cast (location_t
   else
     return NULL_TREE;
 
+  t = fold_build2 (UNLE_EXPR, boolean_type_node, expr, min);
+  tt = fold_build2 (UNGE_EXPR, boolean_type_node, expr, max);
+  t = fold_build2 (TRUTH_OR_EXPR, boolean_type_node, t, tt);
+  if (integer_zerop (t))
+    return NULL_TREE;
+
   if (flag_sanitize_undefined_trap_on_error)
     fn = build_call_expr_loc (loc, builtin_decl_explicit (BUILT_IN_TRAP), 0);
   else
@@ -1364,14 +1371,10 @@ ubsan_instrument_float_cast (location_t
       fn = builtin_decl_explicit (bcode);
       fn = build_call_expr_loc (loc, fn, 2,
 				build_fold_addr_expr_loc (loc, data),
-				ubsan_encode_value (expr, false));
+				ubsan_encode_value (arg, false));
     }
 
-  t = fold_build2 (UNLE_EXPR, boolean_type_node, expr, min);
-  tt = fold_build2 (UNGE_EXPR, boolean_type_node, expr, max);
-  return fold_build3 (COND_EXPR, void_type_node,
-		      fold_build2 (TRUTH_OR_EXPR, boolean_type_node, t, tt),
-		      fn, integer_zero_node);
+  return fold_build3 (COND_EXPR, void_type_node, t, fn, integer_zero_node);
 }
 
 /* Instrument values passed to function arguments with nonnull attribute.  */
--- gcc/convert.c.jj	2014-12-12 16:32:29.000000000 +0100
+++ gcc/convert.c	2014-12-18 09:57:27.742475996 +0100
@@ -890,7 +890,7 @@ convert_to_integer (tree type, tree expr
 				DECL_ATTRIBUTES (current_function_decl)))
 	{
 	  expr = save_expr (expr);
-	  tree check = ubsan_instrument_float_cast (loc, type, expr);
+	  tree check = ubsan_instrument_float_cast (loc, type, expr, expr);
 	  expr = build1 (FIX_TRUNC_EXPR, type, expr);
 	  if (check == NULL)
 	    return expr;
--- gcc/c/c-typeck.c.jj	2014-11-29 12:35:01.000000000 +0100
+++ gcc/c/c-typeck.c	2014-12-18 10:18:00.399158144 +0100
@@ -5828,12 +5828,14 @@ convert_for_assignment (location_t locat
     {
       tree ret;
       bool save = in_late_binary_op;
-      if (codel == BOOLEAN_TYPE || codel == COMPLEX_TYPE)
+      if (codel == BOOLEAN_TYPE || codel == COMPLEX_TYPE
+	  || (coder == REAL_TYPE
+	      && (codel == INTEGER_TYPE || codel == ENUMERAL_TYPE)
+	      && (flag_sanitize & SANITIZE_FLOAT_CAST)))
 	in_late_binary_op = true;
       ret = convert_and_check (expr_loc != UNKNOWN_LOCATION
 			       ? expr_loc : location, type, orig_rhs);
-      if (codel == BOOLEAN_TYPE || codel == COMPLEX_TYPE)
-	in_late_binary_op = save;
+      in_late_binary_op = save;
       return ret;
     }
 
@@ -9285,7 +9287,11 @@ c_finish_return (location_t loc, tree re
 
       save = in_late_binary_op;
       if (TREE_CODE (TREE_TYPE (res)) == BOOLEAN_TYPE
-          || TREE_CODE (TREE_TYPE (res)) == COMPLEX_TYPE)
+	  || TREE_CODE (TREE_TYPE (res)) == COMPLEX_TYPE
+	  || (TREE_CODE (TREE_TYPE (t)) == REAL_TYPE
+	      && (TREE_CODE (TREE_TYPE (res)) == INTEGER_TYPE
+		  || TREE_CODE (TREE_TYPE (res)) == ENUMERAL_TYPE)
+	      && (flag_sanitize & SANITIZE_FLOAT_CAST)))
         in_late_binary_op = true;
       inner = t = convert (TREE_TYPE (res), t);
       in_late_binary_op = save;
--- gcc/c/c-convert.c.jj	2014-12-17 10:26:03.000000000 +0100
+++ gcc/c/c-convert.c	2014-12-18 10:47:11.142847098 +0100
@@ -117,8 +117,18 @@ convert (tree type, tree expr)
 	  && !lookup_attribute ("no_sanitize_undefined",
 				DECL_ATTRIBUTES (current_function_decl)))
 	{
-	  expr = c_save_expr (expr);
-	  tree check = ubsan_instrument_float_cast (loc, type, expr);
+	  tree arg;
+	  if (in_late_binary_op)
+	    {
+	      expr = save_expr (expr);
+	      arg = expr;
+	    }
+	  else
+	    {
+	      expr = c_save_expr (expr);
+	      arg = c_fully_fold (expr, false, NULL);
+	    }
+	  tree check = ubsan_instrument_float_cast (loc, type, expr, arg);
 	  expr = fold_build1 (FIX_TRUNC_EXPR, type, expr);
 	  if (check == NULL)
 	    return expr;
--- gcc/testsuite/c-c++-common/ubsan/pr64344-1.c.jj	2014-12-18 10:59:02.030539698 +0100
+++ gcc/testsuite/c-c++-common/ubsan/pr64344-1.c	2014-12-18 10:59:46.310773017 +0100
@@ -0,0 +1,9 @@
+/* PR sanitizer/64344 */
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=float-cast-overflow" } */
+
+int
+foo (float x)
+{
+  return __builtin_log ((double ) x);
+}
--- gcc/testsuite/c-c++-common/ubsan/pr64344-2.c.jj	2014-12-18 10:59:09.794405272 +0100
+++ gcc/testsuite/c-c++-common/ubsan/pr64344-2.c	2014-12-18 10:59:58.826556315 +0100
@@ -0,0 +1,11 @@
+/* PR sanitizer/64344 */
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=float-cast-overflow" } */
+
+int
+foo (void)
+{
+  static const int a = 0.5;
+  static const int b = (int) 13.5 + 1;
+  return a + b;
+}


	Jakub



More information about the Gcc-patches mailing list