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

Richard Biener richard.guenther@gmail.com
Wed Mar 21 10:07:00 GMT 2018


On Wed, Mar 21, 2018 at 9:33 AM, Richard Sandiford
<richard.sandiford@linaro.org> wrote:
> After the sorry we'd skip storing the argument, but that just creates an
> inconsistency later when we try to deallocate the arguments.  This used to
> "work" because pending_stack_adjust and stack_pointer_delta were int rather
> than HWI, so 1<<33 got truncated to 0.
>
> It's not easy to back out at this point because there's so much global
> state floating around.  One option I tried was to put the sorry after:
>
>       unadjusted_args_size
>         = compute_argument_block_size (reg_parm_stack_space,
>                                        &adjusted_args_size,
>                                        fndecl, fntype,
>                                        (pass == 0 ? 0
>                                         : preferred_stack_boundary));
>
> and then zero the argument sizes and num_actual.  That avoids the
> ICE too, but who knows what other problems it creates.
>
> In the end it seemed easier just to stop.
>
> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?
>
> Richard
>
>
> 2018-03-21  Richard Sandiford  <richard.sandiford@linaro.org>
>
> gcc/
>         PR error-recovery/84964
>         * diagnostic-core.h (fatal_sorry): Declare.
>         * diagnostic.c (fatal_sorry): New function.
>         * calls.c (expand_call): Use it instead of sorry.
>
> gcc/testsuite/
>         PR error-recovery/84964
>         * g++.dg/diagnostic/pr84964.C: New test.
>
> Index: gcc/diagnostic-core.h
> ===================================================================
> --- gcc/diagnostic-core.h       2018-03-01 08:20:43.590534306 +0000
> +++ gcc/diagnostic-core.h       2018-03-21 08:31:34.635677798 +0000
> @@ -87,6 +87,8 @@ extern bool permerror (location_t, const
>  extern bool permerror (rich_location *, const char *,
>                                    ...) ATTRIBUTE_GCC_DIAG(2,3);
>  extern void sorry (const char *, ...) ATTRIBUTE_GCC_DIAG(1,2);
> +extern void fatal_sorry (const char *, ...) ATTRIBUTE_GCC_DIAG(1,2)
> +    ATTRIBUTE_NORETURN;
>  extern void inform (location_t, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3);
>  extern void inform (rich_location *, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3);
>  extern void inform_n (location_t, unsigned HOST_WIDE_INT, const char *,
> 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 ...

> +}
> +
>  /* 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?

So, OK with just using fatal_error here (please add a comment why
we're not continuing).

Richard.

>
>               if (store_one_arg (&args[i], argblock, flags,
>                                  adjusted_args_size.var != 0,
> Index: gcc/testsuite/g++.dg/diagnostic/pr84964.C
> ===================================================================
> --- /dev/null   2018-03-17 08:19:33.716019995 +0000
> +++ gcc/testsuite/g++.dg/diagnostic/pr84964.C   2018-03-21 08:31:34.635677798 +0000
> @@ -0,0 +1,6 @@
> +/* { dg-do compile { target lp64 } } */
> +
> +struct a {
> +  short b : 1ULL << 33; /* { dg-warning "width of 'a::b' exceeds its type" } */
> +};
> +void c(...) { c(a()); } /* { dg-message "sorry, unimplemented: passing too large argument on stack" } */



More information about the Gcc-patches mailing list