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 <ja...@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
