C++ PATCH for c++/90825 - endless recursion when evaluating sizeof

[Jason Merrill]

On 6/11/19 2:28 PM, Marek Polacek wrote:
> On Tue, Jun 11, 2019 at 08:37:27AM -0400, Jason Merrill wrote:
>> On 6/11/19 7:47 AM, Jakub Jelinek wrote:
>>> On Mon, Jun 10, 2019 at 09:59:46PM -0400, Marek Polacek wrote:
>>>> This test segvs since r269078, this hunk in particular:
>>>>
>>>> @@ -4581,8 +4713,9 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
>>>>          break;
>>>>
>>>>        case SIZEOF_EXPR:
>>>> -      r = fold_sizeof_expr (t);
>>>> -      VERIFY_CONSTANT (r);
>>>> +      r = cxx_eval_constant_expression (ctx, fold_sizeof_expr (t), lval,
>>>> +                   non_constant_p, overflow_p,
>>>> +                   jump_target);
>>>>          break;
>>>>
>>>> In a template, fold_sizeof_expr will just create a new SIZEOF_EXPR, that is the
>>>> same, but not identical; see cxx_sizeof_expr.  Then cxx_eval_constant_expression
>>>
>>> Not always, if it calls cxx_sizeof_expr, it will, but if it calls
>>> cxx_sizeof_or_alignof_type it will only if the type is dependent or VLA.
>>>
>>> So, I'd think you should call cxx_eval_constant_expression if TREE_CODE (r)
>>> != SIZEOF_EXPR, otherwise probably *non_constant_p = true; is in order,
>>> maybe together with gcc_assert (ctx->quiet); ?  I'd hope that if we really
>>> require a constant expression we evaluate it in !processing_template_decl
>>> contexts.
> 
> Ok, I had been meaning to add the *non_constant_p bit but never did.  :(
> 
>> Makes sense.  Also, cxx_sizeof_expr should probably only return a
>> SIZEOF_EXPR if the operand is instantiation-dependent.
> 
> That results in
> 
>    FAIL: g++.dg/template/incomplete6.C  -std=c++98 (internal compiler error)
>    FAIL: g++.dg/template/incomplete6.C  -std=c++98 (test for excess errors)
>    FAIL: g++.dg/template/overload13.C  -std=c++98 (internal compiler error)
>    FAIL: g++.dg/template/overload13.C  -std=c++98 (test for excess errors)
> 
> because we trigger an assert in value_dependent_expression_p:
> 
>              /* If there are no operands, it must be an expression such
>                 as "int()". This should not happen for aggregate types
>                 because it would form non-constant expressions.  */
>              gcc_assert (cxx_dialect >= cxx11
>                          || INTEGRAL_OR_ENUMERATION_TYPE_P (type));
> 
>              return false;
> 
> and in this case we have T() where T is a class, and it's in C++98.
> 
> It's not needed to fix this PR so perhaps the following could go in,
> but is there anything I should do about that?

instantiation_dependent_uneval_expression_p shouldn't have that problem.

> Bootstrapped/regtested on x86_64-linux, ok for trunk and 9?
> 
> 2019-06-11  Marek Polacek  <polacek@redhat.com>
> 
> 	PR c++/90825 - endless recursion when evaluating sizeof.
> 	* constexpr.c (cxx_eval_constant_expression): Don't recurse on the
> 	result of fold_sizeof_expr if is returns a SIZEOF_EXPR.
> 
> 	* g++.dg/cpp0x/constexpr-sizeof2.C: New test.
> 
> diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
> index a2f29694462..443e1c7899f 100644
> --- gcc/cp/constexpr.c
> +++ gcc/cp/constexpr.c
> @@ -4808,9 +4808,16 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
>         break;
>   
>       case SIZEOF_EXPR:
> -      r = cxx_eval_constant_expression (ctx, fold_sizeof_expr (t), lval,
> -					non_constant_p, overflow_p,
> -					jump_target);
> +      r = fold_sizeof_expr (t);
> +      /* In a template, fold_sizeof_expr may merely create a new SIZEOF_EXPR,
> +	 which could lead to an infinite recursion.  */
> +      if (TREE_CODE (r) != SIZEOF_EXPR)
> +	r = cxx_eval_constant_expression (ctx, r, lval,
> +					  non_constant_p, overflow_p,
> +					  jump_target);
> +      else
> +	*non_constant_p = true;

Let's also add the assert Jakub suggested.

Jason