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

[Jason Merrill]

On 6/11/19 3:59 PM, Marek Polacek wrote:
> On Tue, Jun 11, 2019 at 03:05:26PM -0400, Jason Merrill wrote:
>> 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.
> 
> Ah, nice.
> 
>>> 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.
> 
> Done.
> 
> Luckily, this also fixed c++/90832 so I'm adding a test for that, too.
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk?

OK.

Jason