OK.
On Tue, Sep 13, 2016 at 3:55 AM, Jakub Jelinek <ja...@redhat.com> wrote:
> Hi!
>
> The testcase below shows a bug in POINTER_PLUS_EXPR constexpr handling.
> It is handled by 2 different functions, cxx_eval_pointer_plus_expression
> and cxx_eval_binary_expression, where the first one calls
> cxx_eval_constant_expression on both operands and in some cases optimizes
> it, in other cases returns NULL_TREE. The other function is called
> only if the former returned NULL_TREE, and calls
> cxx_eval_constant_expression on both operands again and always builds
> something. For C++11 I guess it is fine, but for C++14 if there are any
> side-effects in the operands of POINTER_PLUS_EXPR, like i++ in the testcase,
> this means that i++ is evaluated twice if the first function doesn't find
> the &foo[bar] + baz pattern it is looking for.
>
> Fixed by adding this folding into cxx_eval_binary_expression instead,
> actually as it is large enough into a helper function of it, which is
> invoked on the already cxx_eval_constant_expression processed arguments.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/6.3?
>
> 2016-09-13 Jakub Jelinek <ja...@redhat.com>
>
> PR c++/77553
> * constexpr.c (cxx_fold_pointer_plus_expression): New function.
> (cxx_eval_binary_expression): Use it for POINTER_PLUS_EXPR.
> (cxx_eval_pointer_plus_expression): Remove.
> (cxx_eval_constant_expression) <case POINTER_PLUS_EXPR>: Don't
> call cxx_eval_pointer_plus_expression.
>
> * g++.dg/cpp1y/constexpr-77553.C: New test.
>
> --- gcc/cp/constexpr.c.jj 2016-09-06 20:00:36.000000000 +0200
> +++ gcc/cp/constexpr.c 2016-09-11 15:18:24.103581105 +0200
> @@ -1813,6 +1813,63 @@ cxx_eval_unary_expression (const constex
> return r;
> }
>
> +/* Helper function for cxx_eval_binary_expression. Try to optimize
> + original POINTER_PLUS_EXPR T, LHS p+ RHS, return NULL_TREE if the
> + generic folding should be used. */
> +
> +static tree
> +cxx_fold_pointer_plus_expression (const constexpr_ctx *ctx, tree t,
> + tree lhs, tree rhs, bool *non_constant_p,
> + bool *overflow_p)
> +{
> + STRIP_NOPS (lhs);
> + if (TREE_CODE (lhs) != ADDR_EXPR)
> + return NULL_TREE;
> +
> + lhs = TREE_OPERAND (lhs, 0);
> +
> + /* &A[i] p+ j => &A[i + j] */
> + if (TREE_CODE (lhs) == ARRAY_REF
> + && TREE_CODE (TREE_OPERAND (lhs, 1)) == INTEGER_CST
> + && TREE_CODE (rhs) == INTEGER_CST
> + && TYPE_SIZE_UNIT (TREE_TYPE (lhs))
> + && TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (lhs))) == INTEGER_CST)
> + {
> + tree orig_type = TREE_TYPE (t);
> + location_t loc = EXPR_LOCATION (t);
> + tree type = TREE_TYPE (lhs);
> +
> + t = fold_convert_loc (loc, ssizetype, TREE_OPERAND (lhs, 1));
> + tree nelts = array_type_nelts_top (TREE_TYPE (TREE_OPERAND (lhs, 0)));
> + nelts = cxx_eval_constant_expression (ctx, nelts, false,
> non_constant_p,
> + overflow_p);
> + if (*non_constant_p)
> + return NULL_TREE;
> + /* Don't fold an out-of-bound access. */
> + if (!tree_int_cst_le (t, nelts))
> + return NULL_TREE;
> + rhs = cp_fold_convert (ssizetype, rhs);
> + /* Don't fold if rhs can't be divided exactly by TYPE_SIZE_UNIT.
> + constexpr int A[1]; ... (char *)&A[0] + 1 */
> + if (!integer_zerop (fold_build2_loc (loc, TRUNC_MOD_EXPR, sizetype,
> + rhs, TYPE_SIZE_UNIT (type))))
> + return NULL_TREE;
> + /* Make sure to treat the second operand of POINTER_PLUS_EXPR
> + as signed. */
> + rhs = fold_build2_loc (loc, EXACT_DIV_EXPR, ssizetype, rhs,
> + TYPE_SIZE_UNIT (type));
> + t = size_binop_loc (loc, PLUS_EXPR, rhs, t);
> + t = build4_loc (loc, ARRAY_REF, type, TREE_OPERAND (lhs, 0),
> + t, NULL_TREE, NULL_TREE);
> + t = cp_build_addr_expr (t, tf_warning_or_error);
> + t = cp_fold_convert (orig_type, t);
> + return cxx_eval_constant_expression (ctx, t, /*lval*/false,
> + non_constant_p, overflow_p);
> + }
> +
> + return NULL_TREE;
> +}
> +
> /* Subroutine of cxx_eval_constant_expression.
> Like cxx_eval_unary_expression, except for binary expressions. */
>
> @@ -1865,6 +1922,9 @@ cxx_eval_binary_expression (const conste
> error ("arithmetic involving a null pointer in %qE", lhs);
> return t;
> }
> + else if (code == POINTER_PLUS_EXPR)
> + r = cxx_fold_pointer_plus_expression (ctx, t, lhs, rhs, non_constant_p,
> + overflow_p);
>
> if (r == NULL_TREE)
> r = fold_binary_loc (loc, code, type, lhs, rhs);
> @@ -3579,69 +3639,6 @@ cxx_eval_switch_expr (const constexpr_ct
> return NULL_TREE;
> }
>
> -/* Subroutine of cxx_eval_constant_expression.
> - Attempt to reduce a POINTER_PLUS_EXPR expression T. */
> -
> -static tree
> -cxx_eval_pointer_plus_expression (const constexpr_ctx *ctx, tree t,
> - bool lval, bool *non_constant_p,
> - bool *overflow_p)
> -{
> - tree orig_type = TREE_TYPE (t);
> - tree op00 = TREE_OPERAND (t, 0);
> - tree op01 = TREE_OPERAND (t, 1);
> - location_t loc = EXPR_LOCATION (t);
> -
> - op00 = cxx_eval_constant_expression (ctx, op00, lval,
> - non_constant_p, overflow_p);
> -
> - STRIP_NOPS (op00);
> - if (TREE_CODE (op00) != ADDR_EXPR)
> - return NULL_TREE;
> -
> - op01 = cxx_eval_constant_expression (ctx, op01, lval,
> - non_constant_p, overflow_p);
> - op00 = TREE_OPERAND (op00, 0);
> -
> - /* &A[i] p+ j => &A[i + j] */
> - if (TREE_CODE (op00) == ARRAY_REF
> - && TREE_CODE (TREE_OPERAND (op00, 1)) == INTEGER_CST
> - && TREE_CODE (op01) == INTEGER_CST
> - && TYPE_SIZE_UNIT (TREE_TYPE (op00))
> - && TREE_CODE (TYPE_SIZE_UNIT (TREE_TYPE (op00))) == INTEGER_CST)
> - {
> - tree type = TREE_TYPE (op00);
> - t = fold_convert_loc (loc, ssizetype, TREE_OPERAND (op00, 1));
> - tree nelts = array_type_nelts_top (TREE_TYPE (TREE_OPERAND (op00, 0)));
> - nelts = cxx_eval_constant_expression (ctx, nelts, false,
> non_constant_p,
> - overflow_p);
> - if (*non_constant_p)
> - return NULL_TREE;
> - /* Don't fold an out-of-bound access. */
> - if (!tree_int_cst_le (t, nelts))
> - return NULL_TREE;
> - op01 = cp_fold_convert (ssizetype, op01);
> - /* Don't fold if op01 can't be divided exactly by TYPE_SIZE_UNIT.
> - constexpr int A[1]; ... (char *)&A[0] + 1 */
> - if (!integer_zerop (fold_build2_loc (loc, TRUNC_MOD_EXPR, sizetype,
> - op01, TYPE_SIZE_UNIT (type))))
> - return NULL_TREE;
> - /* Make sure to treat the second operand of POINTER_PLUS_EXPR
> - as signed. */
> - op01 = fold_build2_loc (loc, EXACT_DIV_EXPR, ssizetype, op01,
> - TYPE_SIZE_UNIT (type));
> - t = size_binop_loc (loc, PLUS_EXPR, op01, t);
> - t = build4_loc (loc, ARRAY_REF, type, TREE_OPERAND (op00, 0),
> - t, NULL_TREE, NULL_TREE);
> - t = cp_build_addr_expr (t, tf_warning_or_error);
> - t = cp_fold_convert (orig_type, t);
> - return cxx_eval_constant_expression (ctx, t, lval, non_constant_p,
> - overflow_p);
> - }
> -
> - return NULL_TREE;
> -}
> -
> /* Attempt to reduce the expression T to a constant value.
> On failure, issue diagnostic and return error_mark_node. */
> /* FIXME unify with c_fully_fold */
> @@ -3984,12 +3981,6 @@ cxx_eval_constant_expression (const cons
> break;
>
> case POINTER_PLUS_EXPR:
> - r = cxx_eval_pointer_plus_expression (ctx, t, lval, non_constant_p,
> - overflow_p);
> - if (r)
> - break;
> - /* fall through */
> -
> case PLUS_EXPR:
> case MINUS_EXPR:
> case MULT_EXPR:
> --- gcc/testsuite/g++.dg/cpp1y/constexpr-77553.C.jj 2016-09-11
> 15:31:41.692253287 +0200
> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-77553.C 2016-09-11
> 15:31:07.000000000 +0200
> @@ -0,0 +1,29 @@
> +// PR c++/77553
> +// { dg-do compile { target c++14 } }
> +
> +constexpr void
> +bar (int *x)
> +{
> + int i = 0;
> + x[i++] = 1;
> + x[3] = i;
> +}
> +
> +constexpr int
> +foo ()
> +{
> + int a[] = { 0, 0, 0, 0 };
> + bar (a);
> +
> + return a[0] + 8 * a[1] + 64 * a[2] + 512 * a[3];
> +}
> +
> +constexpr int b = foo ();
> +
> +int
> +main ()
> +{
> + static_assert (b == 513, "");
> + if (foo () != 513)
> + __builtin_abort ();
> +}
>
> Jakub
