This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Fix ICE after sorry for big stack arguments (PR 84964)
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: GCC Patches <gcc-patches at gcc dot gnu dot org>, Richard Sandiford <richard dot sandiford at linaro dot org>
- Date: Wed, 21 Mar 2018 10:53:05 +0100
- Subject: Re: Fix ICE after sorry for big stack arguments (PR 84964)
- References: <871sgdzv2y.fsf@linaro.org>
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" } */