Fix ICE after sorry for big stack arguments (PR 84964)

[Richard Biener]

On Wed, Mar 21, 2018 at 12:16 PM, Richard Sandiford
<richard.sandiford@linaro.org> wrote:
> Richard Biener <richard.guenther@gmail.com> writes:
>> On Wed, Mar 21, 2018 at 11:48 AM, Richard Sandiford
>> <richard.sandiford@linaro.org> wrote:
>>> Richard Biener <richard.guenther@gmail.com> writes:
>>>> On Wed, Mar 21, 2018 at 9:33 AM, Richard Sandiford
>>>> <richard.sandiford@linaro.org> wrote:
>>>>> Index: gcc/diagnostic.c
>>>>> ===================================================================
>>>>> --- gcc/diagnostic.c    2018-03-01 08:20:43.589534337 +0000
>>>>> +++ gcc/diagnostic.c    2018-03-21 08:31:34.635677798 +0000
>>>>> @@ -1407,6 +1407,20 @@ sorry (const char *gmsgid, ...)
>>>>>    va_end (ap);
>>>>>  }
>>>>>
>>>>> +/* Same, but stop compilation immediately.  */
>>>>> +
>>>>> +void
>>>>> +fatal_sorry (const char *gmsgid, ...)
>>>>> +{
>>>>> +  va_list ap;
>>>>> +  va_start (ap, gmsgid);
>>>>> +  rich_location richloc (line_table, input_location);
>>>>> +  diagnostic_impl (&richloc, -1, gmsgid, &ap, DK_SORRY);
>>>>> +  va_end (ap);
>>>>> +
>>>>> +  exit (FATAL_EXIT_CODE);
>>>>
>>>> This is somewhat not in style of the other "fatal" handlers.  I guess
>>>> you don't get "sorry: " when using DK_FATAL, but ...
>>>
>>> I didn't add another DK_* because I don't think we want a different prefix
>>> (unliked "error:" vs. "fatal error:").
>>>
>>>>> +}
>>>>> +
>>>>>  /* Return true if an error or a "sorry" has been seen.  Various
>>>>>     processing is disabled after errors.  */
>>>>>  bool
>>>>> Index: gcc/calls.c
>>>>> ===================================================================
>>>>> --- gcc/calls.c 2018-03-17 08:30:20.937100705 +0000
>>>>> +++ gcc/calls.c 2018-03-21 08:31:34.635677798 +0000
>>>>> @@ -4052,13 +4052,10 @@ expand_call (tree exp, rtx target, int i
>>>>>               rtx_insn *before_arg = get_last_insn ();
>>>>>
>>>>>               /* We don't allow passing huge (> 2^30 B) arguments
>>>>> -                by value.  It would cause an overflow later on.  */
>>>>> +                by value.  It would cause an overflow later on.  */
>>>>>               if (constant_lower_bound (adjusted_args_size.constant)
>>>>>                   >= (1 << (HOST_BITS_PER_INT - 2)))
>>>>> -               {
>>>>> -                 sorry ("passing too large argument on stack");
>>>>> -                 continue;
>>>>> -               }
>>>>> +               fatal_sorry ("passing too large argument on stack");
>>>>
>>>> ... why not use fatal_error here?  It doesn't seem to be something
>>>> that is just not implemented but impossible to implement?
>>>
>>> It's HOST_BITS_PER_INT rather than _WIDE_INT, so I think we could
>>> implement it if we were tighter with types.  E.g. this PR occurred
>>> because we went from int to HWI for some things, which should have
>>> fixed part of the problem.  There might still be other uses of
>>> "int" that need to change though.
>>
>> Ah, missed the BITS part.  I wonder if we can get away with simply
>> setting adjusted_args_size.constant to some small constant?  For
>> bits we generally need offset_ints, just HWI isn't enough anyway.
>
> This is a byte quantity, so HWI should be OK.  We're just using
> HOST_BITS_PER_INT to calculate the range of int on the host.

Ok...

> I think we'd run into other problems if the accumulated size didn't
> match the sizes of the individual arguments.  That's why setting the
> sizes to zero and the number of arguments to zero seemed "safer".
> But I don't think anyone's really thought about what else could go wrong
> if we make arbitrary changes like that.  Just bailing out seems better.

Fine.  Then sorry () plus fatal_error () with a comment still sounds best here
(if we care at all about error-recovery issues - care to "paper over
them" instead
of fixing them for real, whatever that means)

Richard.

> Thanks,
> Richard