Re: [patch] Fix segfault caused by spurious constant overflow

[Richard Biener <richard dot guenther at gmail dot com>]

On Fri, May 31, 2019 at 11:56 AM Eric Botcazou <ebotcazou@adacore.com> wrote:
>
> Hi,
>
> this is a regression present for 32-bit targets on mainline and 9 branch only,
> but the underlying issue dates back from the removal of the TYPE_IS_SIZETYPE
> machinery.  Since the removal of this machinery, there is an internal conflict
> for sizetype: it's an unsigned type so with wrap-around semantics on overflow
> but overflow nevertheless needs to be tracked for it in order to avoid buffer
> overflows for large objects.
>
> The constant folder contains various tricks to that effect and the Ada front-
> end even more so, because VLAs are first-class citizens in the language and
> their lower bound is not necessarily 0.  In particular, you need to be able to
> distinguish large constants in sizetype from small negative constants turned
> into even larger constants, because the latter appear in the length of e.g.:
>
>   type Arr is array (2 .. N) of Long_Long_Integer;
>
> This works when the expressions haven't been too much mangled by the constant
> folder and here we have a counter-example for 32-bit targets.  Starting with:
>
>   (N + 4294967295) * 8
>
> which is the sizetype expression of the size of Arr in bytes, extract_muldiv_1
> distributes the multiplication:
>
>   N * 8 + 4294967288
>
> and, immediately after, fold_plusminus_mult_expr factors it back:
>
>   (N + 536870911) * 8
>
> At this point, there is no way for gigi to tell that it's the expression of a
> simple array with no overflow in sight because the 536870911 constant is large
> but not large enough, so gigi flags it as overflowing for small values of N.
>
> I don't see any other way out than disabling the back-and-forth mathematical
> game played by the constant folder in this case, which very likely brings no
> benefit in practice, hence the proposed fix.
>
> Tested on x86_64-suse-linux, OK for the mainline and 9 branch?

Hmm, ISTR we had such mitigations in place (or have) elsewhere keying
on the most significant bit set instead of power-of-two.  But your case
likely recurses and runs into the extract_multiv limiting to eventually
stop, even for (N + 4) * 8, right?  If so shouldn't we prevent this
even for !TYPE_OVERFLOW_WRAPS?  Also

+         && !(tree_fits_shwi_p (c)
+              && exact_log2 (absu_hwi (tree_to_shwi (c))) > 0))

is better written as

           && exact_log2 (wi::to_wide (c)) > 0

not sure why the sizetype constant for you fits in a signed HWI
or you need to compute its absolute value.  Eventually you
need to use wi::abs(wide_int::from (wi::to_wide (c), TYPE_PRECISION
(TREE_TYPE (c)), SIGNED))
or so.

Thanks,
Richard.

>
> 2019-05-31  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * fold-const.c (extract_muldiv_1) <PLUS_EXPR>: Do not distribute a
>         multiplication by a power-of-two value.
>
>
> 2019-05-31  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * gnat.dg/specs/discr6.ads: New test.
>
> --
> Eric Botcazou