On Wed, Aug 07, 2013 at 04:58:03PM +0200, Marek Polacek wrote:
> I actually don't know what I prefer more, but if you like this version
> more, I'll go with it. Maybe this is better because we don't have to
> create SAVE_EXPR and also we avoid one fold_build call. Thanks,
Not creating the SAVE_EXPRs turned to be a no-go: firstly, the logic
when to create the SAVE_EXPRs and when not is hard-to-follow (depends
on C/C++, depends on various standards and it also depends on whether
the types are signed or not) - both for shift and division. Moreover,
when we have nested expressions like e.g.
(i << u) << x
if any of the operands is wrapped in SAVE_EXPR, we get an
-Wuninitialized warning. So, what I did is to evaluate
the op0 always in the condition, like "if (op0, <cond>)",
which is safe and all the uninitialized warnings are gone,
now the bootstrap with -fsanitize=undefined finally finishes.
(albeit with
Comparing stages 2 and 3
warning: gcc/cc1plus-checksum.o differs
warning: gcc/cc1-checksum.o differs
Bootstrap comparison failure!
gcc/combine.o differs
gcc/tree-ssa-loop-ivopts.o differs
gcc/rtlanal.o differs
gcc/calls.o differs
gcc/emit-rtl.o differs
gcc/dbxout.o differs
gcc/function.o differs
gcc/cfgexpand.o differs
gcc/stor-layout.o differs
gcc/tree.o differs
gcc/tree-vect-generic.o differs
gcc/expr.o differs
gcc/fixed-value.o differs
gcc/fold-const.o differs
gcc/i386.o differs
libdecnumber/decimal64.o differs
libdecnumber/decNumber.o differs
make[2]: *** [compare] Error 1
).
Tested x86_64-pc-linux-gnu, applying to the ubsan branch.
2013-08-15 Marek Polacek <pola...@redhat.com>
* c-ubsan.c (ubsan_instrument_division): Evaluate SAVE_EXPRs
before the condition.
(ubsan_instrument_shift): Likewise.
* c-c++-common/ubsan/save-expr-1.c: New test.
* c-c++-common/ubsan/save-expr-2.c: New test.
* c-c++-common/ubsan/save-expr-3.c: New test.
* c-c++-common/ubsan/save-expr-4.c: New test.
--- gcc/c-family/c-ubsan.c.mp 2013-08-15 10:44:51.189966522 +0200
+++ gcc/c-family/c-ubsan.c 2013-08-15 12:43:30.454523049 +0200
@@ -73,6 +73,10 @@ ubsan_instrument_division (location_t lo
x = fold_build2 (TRUTH_AND_EXPR, boolean_type_node, x, tt);
t = fold_build2 (TRUTH_OR_EXPR, boolean_type_node, t, x);
}
+
+ /* In case we have a SAVE_EXPR in a conditional context, we need to
+ make sure it gets evaluated before the condition. */
+ t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (t), op0, t);
tree data = ubsan_create_data ("__ubsan_overflow_data",
loc, ubsan_type_descriptor (type),
NULL_TREE);
@@ -133,6 +137,10 @@ ubsan_instrument_shift (location_t loc,
build_int_cst (type0, 0));
tt = fold_build2 (TRUTH_OR_EXPR, boolean_type_node, x, tt);
}
+
+ /* In case we have a SAVE_EXPR in a conditional context, we need to
+ make sure it gets evaluated before the condition. */
+ t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (t), op0, t);
tree data = ubsan_create_data ("__ubsan_shift_data",
loc, ubsan_type_descriptor (type0),
ubsan_type_descriptor (type1), NULL_TREE);
--- gcc/testsuite/c-c++-common/ubsan/save-expr-1.c.mp 2013-08-15
10:45:37.377057713 +0200
+++ gcc/testsuite/c-c++-common/ubsan/save-expr-1.c 2013-08-15
10:53:01.601695980 +0200
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=shift -Wall -Werror -O" } */
+
+static int x;
+int
+main (void)
+{
+ int o = 1;
+ int y = x << o;
+ return y;
+}
--- gcc/testsuite/c-c++-common/ubsan/save-expr-2.c.mp 2013-08-15
10:58:37.505938910 +0200
+++ gcc/testsuite/c-c++-common/ubsan/save-expr-2.c 2013-08-15
10:58:30.258913139 +0200
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=shift -Wall -Werror -O" } */
+
+int
+foo (int i, unsigned int u)
+{
+ return u / i;
+}
+
+int
+bar (int i, unsigned int u)
+{
+ return u % i;
+}
--- gcc/testsuite/c-c++-common/ubsan/save-expr-3.c.mp 2013-08-15
11:02:39.111142875 +0200
+++ gcc/testsuite/c-c++-common/ubsan/save-expr-3.c 2013-08-15
11:02:34.874127652 +0200
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=shift -Wall -Werror -O" } */
+
+int x;
+
+int
+foo (int i, int u)
+{
+ return (i << u) << x;
+}
+
+int
+bar (int i, int u)
+{
+ return (i >> u) >> x;
+}
--- gcc/testsuite/c-c++-common/ubsan/save-expr-4.c.mp 2013-08-15
11:09:53.453746629 +0200
+++ gcc/testsuite/c-c++-common/ubsan/save-expr-4.c 2013-08-15
11:09:45.238716728 +0200
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=shift -Wall -Werror -O" } */
+
+int x;
+
+int
+foo (int i, unsigned int u)
+{
+ return (i % u) << (x / u);
+}
+
+int
+bar (int i, unsigned int u)
+{
+ return (((x % u) << (u / i)) >> x);
+}
Marek
