Re: C++ PATCH to tweak cp_fully_fold (PR c++/84590)
Ok. On Mar 1, 2018 4:38 PM, "Marek Polacek"wrote: > On Thu, Mar 01, 2018 at 02:29:47PM -0500, Jason Merrill wrote: > > I think this should happen in the block with maybe_constant_value; > > cp_fold_rvalue doesn't do this. > > This, then: > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > 2018-03-01 Marek Polacek > > PR c++/84590 > * cp-gimplify.c (cp_fully_fold): Unwrap TARGET_EXPR or a > CONSTRUCTOR > wrapped in VIEW_CONVERT_EXPR. > > * c-c++-common/ubsan/shift-11.c: New test. > > diff --git gcc/cp/cp-gimplify.c gcc/cp/cp-gimplify.c > index 55a9d278dbe..0ddd435454c 100644 > --- gcc/cp/cp-gimplify.c > +++ gcc/cp/cp-gimplify.c > @@ -2037,7 +2037,17 @@ cp_fully_fold (tree x) >/* FIXME cp_fold ought to be a superset of maybe_constant_value so we > don't > have to call both. */ >if (cxx_dialect >= cxx11) > -x = maybe_constant_value (x); > +{ > + x = maybe_constant_value (x); > + /* Sometimes we are given a CONSTRUCTOR but the call above wraps it > into > +a TARGET_EXPR; undo that here. */ > + if (TREE_CODE (x) == TARGET_EXPR) > + x = TARGET_EXPR_INITIAL (x); > + else if (TREE_CODE (x) == VIEW_CONVERT_EXPR > + && TREE_CODE (TREE_OPERAND (x, 0)) == CONSTRUCTOR > + && TREE_TYPE (TREE_OPERAND (x, 0)) == TREE_TYPE (x)) > + x = TREE_OPERAND (x, 0); > +} >return cp_fold_rvalue (x); > } > > diff --git gcc/testsuite/c-c++-common/ubsan/shift-11.c > gcc/testsuite/c-c++-common/ubsan/shift-11.c > index e69de29bb2d..03a72e217a2 100644 > --- gcc/testsuite/c-c++-common/ubsan/shift-11.c > +++ gcc/testsuite/c-c++-common/ubsan/shift-11.c > @@ -0,0 +1,13 @@ > +/* PR c++/84590 */ > +/* { dg-do compile } */ > +/* { dg-options "-fsanitize=shift" } */ > + > +struct S { > + int b; > +}; > + > +void > +fn (void) > +{ > + struct S c1 = { 1 << -1 }; /* { dg-warning "left shift" } */ > +} > > Marek >
Re: C++ PATCH to tweak cp_fully_fold (PR c++/84590)
On Thu, Mar 01, 2018 at 02:29:47PM -0500, Jason Merrill wrote: > I think this should happen in the block with maybe_constant_value; > cp_fold_rvalue doesn't do this. This, then: Bootstrapped/regtested on x86_64-linux, ok for trunk? 2018-03-01 Marek PolacekPR c++/84590 * cp-gimplify.c (cp_fully_fold): Unwrap TARGET_EXPR or a CONSTRUCTOR wrapped in VIEW_CONVERT_EXPR. * c-c++-common/ubsan/shift-11.c: New test. diff --git gcc/cp/cp-gimplify.c gcc/cp/cp-gimplify.c index 55a9d278dbe..0ddd435454c 100644 --- gcc/cp/cp-gimplify.c +++ gcc/cp/cp-gimplify.c @@ -2037,7 +2037,17 @@ cp_fully_fold (tree x) /* FIXME cp_fold ought to be a superset of maybe_constant_value so we don't have to call both. */ if (cxx_dialect >= cxx11) -x = maybe_constant_value (x); +{ + x = maybe_constant_value (x); + /* Sometimes we are given a CONSTRUCTOR but the call above wraps it into +a TARGET_EXPR; undo that here. */ + if (TREE_CODE (x) == TARGET_EXPR) + x = TARGET_EXPR_INITIAL (x); + else if (TREE_CODE (x) == VIEW_CONVERT_EXPR + && TREE_CODE (TREE_OPERAND (x, 0)) == CONSTRUCTOR + && TREE_TYPE (TREE_OPERAND (x, 0)) == TREE_TYPE (x)) + x = TREE_OPERAND (x, 0); +} return cp_fold_rvalue (x); } diff --git gcc/testsuite/c-c++-common/ubsan/shift-11.c gcc/testsuite/c-c++-common/ubsan/shift-11.c index e69de29bb2d..03a72e217a2 100644 --- gcc/testsuite/c-c++-common/ubsan/shift-11.c +++ gcc/testsuite/c-c++-common/ubsan/shift-11.c @@ -0,0 +1,13 @@ +/* PR c++/84590 */ +/* { dg-do compile } */ +/* { dg-options "-fsanitize=shift" } */ + +struct S { + int b; +}; + +void +fn (void) +{ + struct S c1 = { 1 << -1 }; /* { dg-warning "left shift" } */ +} Marek
Re: C++ PATCH to tweak cp_fully_fold (PR c++/84590)
On Thu, Mar 1, 2018 at 2:12 PM, Marek Polacekwrote: > In this testcase we find ourselves in split_nonconstant_init: > >init = cp_fully_fold (init); >code = push_stmt_list (); >if (split_nonconstant_init_1 (dest, init)) > > where initially INIT was a CONSTRUCTOR, but cp_fully_fold returned > a TARGET_EXPR. This confuses split_nonconstant_init_1 which only expects > a CONSTRUCTOR or a VECTOR_CST. Jason suggested stripping TARGET_EXPRs / > VIEW_CONVERT_EXPRs, so here it is. > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > 2018-03-01 Marek Polacek > > PR c++/84590 > * cp-gimplify.c (cp_fully_fold): Unwrap TARGET_EXPR or a CONSTRUCTOR > wrapped in VIEW_CONVERT_EXPR. > > * c-c++-common/ubsan/shift-11.c: New test. > > diff --git gcc/cp/cp-gimplify.c gcc/cp/cp-gimplify.c > index 55a9d278dbe..1fa9c466e6b 100644 > --- gcc/cp/cp-gimplify.c > +++ gcc/cp/cp-gimplify.c > @@ -2038,7 +2038,18 @@ cp_fully_fold (tree x) > have to call both. */ >if (cxx_dialect >= cxx11) > x = maybe_constant_value (x); > - return cp_fold_rvalue (x); > + x = cp_fold_rvalue (x); > + > + /* Sometimes we are given a CONSTRUCTOR but > cxx_eval_outermost_constant_expr > + wraps it into a TARGET_EXPR; undo that here. */ > + if (TREE_CODE (x) == TARGET_EXPR) > +x = TARGET_EXPR_INITIAL (x); > + else if (TREE_CODE (x) == VIEW_CONVERT_EXPR > + && TREE_CODE (TREE_OPERAND (x, 0)) == CONSTRUCTOR > + && TREE_TYPE (TREE_OPERAND (x, 0)) == TREE_TYPE (x)) > +x = TREE_OPERAND (x, 0); I think this should happen in the block with maybe_constant_value; cp_fold_rvalue doesn't do this. Jason
C++ PATCH to tweak cp_fully_fold (PR c++/84590)
In this testcase we find ourselves in split_nonconstant_init: init = cp_fully_fold (init); code = push_stmt_list (); if (split_nonconstant_init_1 (dest, init)) where initially INIT was a CONSTRUCTOR, but cp_fully_fold returned a TARGET_EXPR. This confuses split_nonconstant_init_1 which only expects a CONSTRUCTOR or a VECTOR_CST. Jason suggested stripping TARGET_EXPRs / VIEW_CONVERT_EXPRs, so here it is. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2018-03-01 Marek PolacekPR c++/84590 * cp-gimplify.c (cp_fully_fold): Unwrap TARGET_EXPR or a CONSTRUCTOR wrapped in VIEW_CONVERT_EXPR. * c-c++-common/ubsan/shift-11.c: New test. diff --git gcc/cp/cp-gimplify.c gcc/cp/cp-gimplify.c index 55a9d278dbe..1fa9c466e6b 100644 --- gcc/cp/cp-gimplify.c +++ gcc/cp/cp-gimplify.c @@ -2038,7 +2038,18 @@ cp_fully_fold (tree x) have to call both. */ if (cxx_dialect >= cxx11) x = maybe_constant_value (x); - return cp_fold_rvalue (x); + x = cp_fold_rvalue (x); + + /* Sometimes we are given a CONSTRUCTOR but cxx_eval_outermost_constant_expr + wraps it into a TARGET_EXPR; undo that here. */ + if (TREE_CODE (x) == TARGET_EXPR) +x = TARGET_EXPR_INITIAL (x); + else if (TREE_CODE (x) == VIEW_CONVERT_EXPR + && TREE_CODE (TREE_OPERAND (x, 0)) == CONSTRUCTOR + && TREE_TYPE (TREE_OPERAND (x, 0)) == TREE_TYPE (x)) +x = TREE_OPERAND (x, 0); + + return x; } /* c-common interface to cp_fold. If IN_INIT, this is in a static initializer diff --git gcc/testsuite/c-c++-common/ubsan/shift-11.c gcc/testsuite/c-c++-common/ubsan/shift-11.c index e69de29bb2d..03a72e217a2 100644 --- gcc/testsuite/c-c++-common/ubsan/shift-11.c +++ gcc/testsuite/c-c++-common/ubsan/shift-11.c @@ -0,0 +1,13 @@ +/* PR c++/84590 */ +/* { dg-do compile } */ +/* { dg-options "-fsanitize=shift" } */ + +struct S { + int b; +}; + +void +fn (void) +{ + struct S c1 = { 1 << -1 }; /* { dg-warning "left shift" } */ +} Marek