From 6b6ae9eb9c06b6911573bb9a13cf98b5a7c98b78 Mon Sep 17 00:00:00 2001 From: Marek Polacek Date: Tue, 16 May 2017 19:25:04 +0000 Subject: [PATCH] re PR sanitizer/80536 (UBSAN: compile time segfault) PR sanitizer/80536 PR sanitizer/80386 * cp-gimplify.c (cp_fold): Handle SAVE_EXPR. * tree.c (save_expr): Don't fold the expression. * c-c++-common/ubsan/pr80536.c: New test. * g++.dg/ubsan/pr80386.C: New test. From-SVN: r248124 --- gcc/ChangeLog | 6 ++++++ gcc/cp/ChangeLog | 6 ++++++ gcc/cp/cp-gimplify.c | 9 +++++++++ gcc/testsuite/ChangeLog | 7 +++++++ gcc/testsuite/c-c++-common/ubsan/pr80536.c | 9 +++++++++ gcc/testsuite/g++.dg/ubsan/pr80386.C | 13 +++++++++++++ gcc/tree.c | 16 +++++++--------- 7 files changed, 57 insertions(+), 9 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/ubsan/pr80536.c create mode 100644 gcc/testsuite/g++.dg/ubsan/pr80386.C diff --git a/gcc/ChangeLog b/gcc/ChangeLog index f5793208cc6..e5c872cc62b 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,9 @@ +2017-05-16 Marek Polacek + + PR sanitizer/80536 + PR sanitizer/80386 + * tree.c (save_expr): Don't fold the expression. + 2017-05-16 Uros Bizjak * config/i386.i386.md (*movsi_internal): Split (?rm,*y) alternative diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index 6ce4bbb4d9f..228f0adb5aa 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,3 +1,9 @@ +2017-05-16 Marek Polacek + + PR sanitizer/80536 + PR sanitizer/80386 + * cp-gimplify.c (cp_fold): Handle SAVE_EXPR. + 2017-05-16 Nathan Sidwell * name-lookup.c (check_local_shadow): New, broke out of ... diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c index de62414ec3c..38a8ed46519 100644 --- a/gcc/cp/cp-gimplify.c +++ b/gcc/cp/cp-gimplify.c @@ -2428,6 +2428,15 @@ cp_fold (tree x) x = fold (x); break; + case SAVE_EXPR: + /* A SAVE_EXPR might contain e.g. (0 * i) + (0 * j), which, after + folding, evaluates to an invariant. In that case no need to wrap + this folded tree with a SAVE_EXPR. */ + r = cp_fold (TREE_OPERAND (x, 0)); + if (tree_invariant_p (r)) + x = r; + break; + default: return org_x; } diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 9c037edc97e..13e8cf98fd4 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,10 @@ +2017-05-16 Marek Polacek + + PR sanitizer/80536 + PR sanitizer/80386 + * c-c++-common/ubsan/pr80536.c: New test. + * g++.dg/ubsan/pr80386.C: New test. + 2017-05-16 Tamar Christina * gcc.target/arm/armv8_2-fp16-neon-1.c (vceqz): Fix regex. diff --git a/gcc/testsuite/c-c++-common/ubsan/pr80536.c b/gcc/testsuite/c-c++-common/ubsan/pr80536.c new file mode 100644 index 00000000000..23913ad3b75 --- /dev/null +++ b/gcc/testsuite/c-c++-common/ubsan/pr80536.c @@ -0,0 +1,9 @@ +/* PR sanitizer/80536 */ +/* { dg-do compile } */ +/* { dg-options "-fsanitize=undefined" } */ + +int +foo (int i) +{ + return ((i * (unsigned long long) (-0 + 1UL)) * 2) % 1; +} diff --git a/gcc/testsuite/g++.dg/ubsan/pr80386.C b/gcc/testsuite/g++.dg/ubsan/pr80386.C new file mode 100644 index 00000000000..60122da3c19 --- /dev/null +++ b/gcc/testsuite/g++.dg/ubsan/pr80386.C @@ -0,0 +1,13 @@ +// PR sanitizer/80386 +// { dg-do run } +// { dg-options "-fsanitize=undefined -fno-sanitize-recover" } + +static unsigned long long int i = 13996271126042720493ULL; + +int +main () +{ + int r = (((2921 + 0) - short(i)) + 0x7fffffff) >> 0; + asm volatile ("" : "+g" (r)); + return 0; +} diff --git a/gcc/tree.c b/gcc/tree.c index b76b521df61..75067258313 100644 --- a/gcc/tree.c +++ b/gcc/tree.c @@ -3337,7 +3337,6 @@ tree_invariant_p (tree t) tree save_expr (tree expr) { - tree t = fold (expr); tree inner; /* If the tree evaluates to a constant, then we don't want to hide that @@ -3345,33 +3344,32 @@ save_expr (tree expr) However, a read-only object that has side effects cannot be bypassed. Since it is no problem to reevaluate literals, we just return the literal node. */ - inner = skip_simple_arithmetic (t); + inner = skip_simple_arithmetic (expr); if (TREE_CODE (inner) == ERROR_MARK) return inner; if (tree_invariant_p_1 (inner)) - return t; + return expr; /* If INNER contains a PLACEHOLDER_EXPR, we must evaluate it each time, since it means that the size or offset of some field of an object depends on the value within another field. - Note that it must not be the case that T contains both a PLACEHOLDER_EXPR + Note that it must not be the case that EXPR contains both a PLACEHOLDER_EXPR and some variable since it would then need to be both evaluated once and evaluated more than once. Front-ends must assure this case cannot happen by surrounding any such subexpressions in their own SAVE_EXPR and forcing evaluation at the proper time. */ if (contains_placeholder_p (inner)) - return t; + return expr; - t = build1 (SAVE_EXPR, TREE_TYPE (expr), t); - SET_EXPR_LOCATION (t, EXPR_LOCATION (expr)); + expr = build1_loc (EXPR_LOCATION (expr), SAVE_EXPR, TREE_TYPE (expr), expr); /* This expression might be placed ahead of a jump to ensure that the value was computed on both sides of the jump. So make sure it isn't eliminated as dead. */ - TREE_SIDE_EFFECTS (t) = 1; - return t; + TREE_SIDE_EFFECTS (expr) = 1; + return expr; } /* Look inside EXPR into any simple arithmetic operations. Return the -- 2.30.2