On Wed, Sep 18, 2013 at 03:15:05PM +0200, Jakub Jelinek wrote:
> On Wed, Sep 18, 2013 at 03:10:42PM +0200, Marek Polacek wrote:
> > --- gcc/cp/typeck.c.mp 2013-09-18 14:00:14.303869196 +0200
> > +++ gcc/cp/typeck.c 2013-09-18 14:08:21.287770112 +0200
> > @@ -4884,7 +4884,7 @@ cp_build_binary_op (location_t location,
> > if (build_type == NULL_TREE)
> > build_type = result_type;
> >
> > - if ((flag_sanitize & SANITIZE_UNDEFINED)
> > + if ((flag_sanitize & SANITIZE_SHIFT || flag_sanitize & SANITIZE_DIVIDE)
>
> I'd suggest to use
> if ((flag & (SANITIZE_SHIFT | SANITIZE_DIVIDE))
> instead.
>
> > && !processing_template_decl
> > && current_function_decl != 0
> > && !lookup_attribute ("no_sanitize_undefined",
> > @@ -4898,7 +4898,7 @@ cp_build_binary_op (location_t location,
> > tf_none));
> > op1 = maybe_constant_value (fold_non_dependent_expr_sfinae (op1,
> > tf_none));
> > - if (doing_div_or_mod)
> > + if (doing_div_or_mod && flag_sanitize & SANITIZE_DIVIDE)
>
> And, while the operator precedence is right, I think it would be better
> to use if (doing_div_or_mod && (flag_sanitize & SANITIZE_DIVIDE))
Thanks, fixed in both FEs. Re-ran the ubsan testsuite. Ok now?
2013-09-18 Marek Polacek <pola...@redhat.com>
PR sanitize/58443
cp/
* typeck.c (cp_build_binary_op): Properly honor -fsanitize options.
Remove unnecessary check.
c/
* c-typeck.c (build_binary_op): Properly honor -fsanitize options.
Remove unnecessary check.
testsuite/
* g++.dg/ubsan/div-by-zero-1.C: Use the integer-divide-by-zero option
instead of the shift option.
* c-c++-common/ubsan/pr58443-1.c: New test.
* c-c++-common/ubsan/pr58443-3.c: New test.
* c-c++-common/ubsan/pr58443-2.c: New test.
--- gcc/cp/typeck.c.mp 2013-09-18 14:00:14.303869196 +0200
+++ gcc/cp/typeck.c 2013-09-18 15:19:56.601920206 +0200
@@ -4884,7 +4884,7 @@ cp_build_binary_op (location_t location,
if (build_type == NULL_TREE)
build_type = result_type;
- if ((flag_sanitize & SANITIZE_UNDEFINED)
+ if ((flag_sanitize & (SANITIZE_SHIFT | SANITIZE_DIVIDE))
&& !processing_template_decl
&& current_function_decl != 0
&& !lookup_attribute ("no_sanitize_undefined",
@@ -4898,7 +4898,7 @@ cp_build_binary_op (location_t location,
tf_none));
op1 = maybe_constant_value (fold_non_dependent_expr_sfinae (op1,
tf_none));
- if (doing_div_or_mod)
+ if (doing_div_or_mod && (flag_sanitize & SANITIZE_DIVIDE))
{
/* For diagnostics we want to use the promoted types without
shorten_binary_op. So convert the arguments to the
@@ -4912,7 +4912,7 @@ cp_build_binary_op (location_t location,
}
instrument_expr = ubsan_instrument_division (location, cop0, cop1);
}
- else if (doing_shift)
+ else if (doing_shift && (flag_sanitize & SANITIZE_SHIFT))
instrument_expr = ubsan_instrument_shift (location, code, op0, op1);
}
@@ -4926,7 +4926,7 @@ cp_build_binary_op (location_t location,
&& !TREE_OVERFLOW_P (op1))
overflow_warning (location, result);
- if ((flag_sanitize & SANITIZE_UNDEFINED) && instrument_expr != NULL)
+ if (instrument_expr != NULL)
result = fold_build2 (COMPOUND_EXPR, TREE_TYPE (result),
instrument_expr, result);
--- gcc/c/c-typeck.c.mp 2013-09-18 14:00:06.704837584 +0200
+++ gcc/c/c-typeck.c 2013-09-18 15:20:10.263972487 +0200
@@ -10496,7 +10496,7 @@ build_binary_op (location_t location, en
return error_mark_node;
}
- if (flag_sanitize & SANITIZE_UNDEFINED
+ if ((flag_sanitize & (SANITIZE_SHIFT | SANITIZE_DIVIDE))
&& current_function_decl != 0
&& !lookup_attribute ("no_sanitize_undefined",
DECL_ATTRIBUTES (current_function_decl))
@@ -10507,9 +10507,9 @@ build_binary_op (location_t location, en
op1 = c_save_expr (op1);
op0 = c_fully_fold (op0, false, NULL);
op1 = c_fully_fold (op1, false, NULL);
- if (doing_div_or_mod)
+ if (doing_div_or_mod && (flag_sanitize & SANITIZE_DIVIDE))
instrument_expr = ubsan_instrument_division (location, op0, op1);
- else if (doing_shift)
+ else if (doing_shift && (flag_sanitize & SANITIZE_SHIFT))
instrument_expr = ubsan_instrument_shift (location, code, op0, op1);
}
@@ -10537,7 +10537,7 @@ build_binary_op (location_t location, en
ret = build1 (EXCESS_PRECISION_EXPR, semantic_result_type, ret);
protected_set_expr_location (ret, location);
- if ((flag_sanitize & SANITIZE_UNDEFINED) && instrument_expr != NULL)
+ if (instrument_expr != NULL)
ret = fold_build2 (COMPOUND_EXPR, TREE_TYPE (ret),
instrument_expr, ret);
--- gcc/testsuite/g++.dg/ubsan/div-by-zero-1.C.mp 2013-09-18
14:24:42.735312422 +0200
+++ gcc/testsuite/g++.dg/ubsan/div-by-zero-1.C 2013-09-18 14:25:28.965491706
+0200
@@ -1,5 +1,5 @@
/* { dg-do compile } */
-/* { dg-options "-fsanitize=shift -w" } */
+/* { dg-options "-fsanitize=integer-divide-by-zero -w" } */
void
foo (int i)
--- gcc/testsuite/c-c++-common/ubsan/pr58443-1.c.mp 2013-09-18
14:20:18.633348043 +0200
+++ gcc/testsuite/c-c++-common/ubsan/pr58443-1.c 2013-09-18
14:30:45.403701302 +0200
@@ -0,0 +1,11 @@
+/* PR sanitizer/58443 */
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=shift,unreachable -w" } */
+
+int
+foo (int u, int o)
+{
+ return u / o;
+}
+
+/* { dg-final { scan-assembler-not "__ubsan_handle_divrem_overflow" } } */
--- gcc/testsuite/c-c++-common/ubsan/pr58443-3.c.mp 2013-09-18
14:20:31.724398731 +0200
+++ gcc/testsuite/c-c++-common/ubsan/pr58443-3.c 2013-09-18
14:30:23.959623394 +0200
@@ -0,0 +1,18 @@
+/* PR sanitizer/58443 */
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=undefined -w" } */
+
+int
+foo (int u, int o)
+{
+ return u >> o;
+}
+
+int
+bar (int u, int o)
+{
+ return u / o;
+}
+
+/* { dg-final { scan-assembler "__ubsan_handle_divrem_overflow" } } */
+/* { dg-final { scan-assembler "__ubsan_handle_shift_out_of_bounds" } } */
--- gcc/testsuite/c-c++-common/ubsan/pr58443-2.c.mp 2013-09-18
14:20:27.692384073 +0200
+++ gcc/testsuite/c-c++-common/ubsan/pr58443-2.c 2013-09-18
14:28:17.272155763 +0200
@@ -0,0 +1,11 @@
+/* PR sanitizer/58443 */
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=unreachable,integer-divide-by-zero -w" } */
+
+int
+foo (int u, int o)
+{
+ return u >> o;
+}
+
+/* { dg-final { scan-assembler-not "__ubsan_handle_shift_out_of_bounds" } } */
Marek
