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

[Richard Sandiford <richard dot sandiford at linaro dot org>]

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.

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.

Thanks,
Richard