Aw: Re: Re: Re: [PATCH] PR fortran/100950 - ICE in output_constructor_regular_field, at varasm.c:5514

Tobias Burnus tobias@codesourcery.com
Fri Aug 20 14:19:20 GMT 2021


Hi Harald,

On 20.08.21 14:17, Harald Anlauf wrote:
>> I can confirm this. – I think in order to reduce the clutter, the
>> diagnostic probably should be removed.
> I am unable to prove that we will never that check.  So how about:
> diff --git a/gcc/fortran/simplify.c b/gcc/fortran/simplify.c
> index eaabbffca4d..04722a8640c 100644
> --- a/gcc/fortran/simplify.c
> +++ b/gcc/fortran/simplify.c
> @@ -4552,27 +4552,13 @@ substring_has_constant_len (gfc_expr *e)
>
>     if (istart <= iend)
>       {
> -      char buffer[21];
> -      if (istart < 1)
> -       {
> -         sprintf (buffer, HOST_WIDE_INT_PRINT_DEC, istart);
> -         gfc_error ("Substring start index (%s) at %L below 1",
> -                    buffer, &ref->u.ss.start->where);
> -         return false;
> -       }
> -
>         /* For deferred strings use end index as proxy for length.  */
>         if (e->ts.deferred)
>          length = iend;
>         else
>          length = gfc_mpz_get_hwi (ref->u.ss.length->length->value.integer);
> -      if (iend > length)
> -       {
> -         sprintf (buffer, HOST_WIDE_INT_PRINT_DEC, iend);
> -         gfc_error ("Substring end index (%s) at %L exceeds string length",
> -                    buffer, &ref->u.ss.end->where);
> -         return false;
> -       }
> +
> +      gcc_assert (istart >= 1 && iend <= length);
>         length = iend - istart + 1;
>       }
>     else
>
> Or skip the gcc_assert and cross fingers... (we then would not even need to
> verify ref->u.ss.length in that depth).

LGTM – I am fine with either variant, but I am slightly inclined to
removing the gcc_assert*
– as I believe that the existing checks come early enough and do seem to
work well.

Can you check ('grep') whether we already have sufficient coverage of
substring out of bounds?
We presumably have, but if you spot something, I think it makes sense to
add those to the testsuite.

Tobias
*I think GCC won't complain but if ENABLE_ASSERT_CHECKING is not defined,
there is then a pointless 'length =' assignment, overridden before it is
ever used.

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955


More information about the Gcc-patches mailing list