Re: [PR middle-end/79123] cast false positive in -Walloca-larger-than=

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

On Mon, Jan 23, 2017 at 4:55 PM, Martin Sebor <msebor@gmail.com> wrote:
>>> Yes, I see that.  But when I change size_t to unsigned int (in LP64)
>>> like so:
>>>
>>>   void g (int *p, int *q)
>>>   {
>>>     unsigned n = (unsigned)(p - q);
>>>
>>>     if (n < 10)
>>>       f (__builtin_alloca (n));
>>>   }
>>>
>>> -Walloca-larger-than=100 still complains:
>>>
>>>   warning: argument to ‘alloca’ may be too large
>>>   note: limit is 100 bytes, but argument may be as large as 4294967295
>>>
>>> and the VRP dump shows
>>>
>>>   _5: [0, 4294967295]
>>>   _14: [0, 4294967295]
>>>   ...
>>>     _3 = p.0_1 - q.1_2;
>>>     _4 = _3 /[ex] 4;
>>>     n_10 = (unsigned int) _4;
>>>     if (n_10 <= 9)
>>>       goto <bb 3>; [36.64%]
>>>     else
>>>       goto <bb 4>; [63.36%]
>>>
>>>     <bb 3> [36.64%]:
>>>     _14 = _4 & 4294967295;
>>>     _5 = (long unsigned int) _14;
>>>     _6 = __builtin_alloca (_5);
>>>
>>> so it seems that even in VRP itself the range information isn't
>>> quite good enough to avoid false positives.  (Perhaps that's one
>>> the missed cases you mention below?)
>>
>>
>> Well, you see that there's no obvious way to use the n_10 <= 9 conditional
>> here.  VRP would need to register a [0, 9] range for the lower half of
>> n_10
>> and then figure after seeing
>>
>>>     _14 = _4 & 4294967295;
>>
>>
>> that _14 is now [0, 9].
>>
>> That's a neat trick VRP cannot handle at the moment (there's no way the
>> lattice can encode a range for a sub-set of all bits of a SSA name).
>
>
> Sure.  My point is just that it looks like there will be some false
> positives whether the warnings are implemented as part of the VRP
> pass or elsewhere.  (I admit I haven't studied the implementation
> of the VRP pass to understand whether these false positives are
> avoidable and I'm happy to take your word if if you say they can.)
>
>> Your source is bogus in the way that (unsigned)(p - q) might truncate
>> the pointer difference.
>
>
> The problem is independent of the pointer difference and can be
> reproduced by any conversion from a wider type to a narrower
> unsigned type (even the safe ones), as the test case in bug 79191
> I opened for it shows:
>
>   void f (unsigned long n)
>   {
>     if (n > 3)
>       __builtin_abort ();
>   }
>
>   void h (unsigned long m)
>   {
>     unsigned n = m;
>
>     if (n < 3)
>       f (n);
>   }

Sure, but that's essentially the same issue:

  <bb 2> [100.00%]:
  n_2 = (unsigned int) m_1(D);
  if (n_2 <= 2)
    goto <bb 3>; [54.00%]
  else
    goto <bb 5>; [46.00%]

  <bb 3> [54.00%]:
  m_6 = ASSERT_EXPR <m_1(D), m_1(D) <= 18446744069414584322>;
  _5 = m_6 & 4294967295;
  if (_5 > 3)

we can't express that m_6 has a known range for bits 0..31.

But of course this shows that the canonicalization of truncation to bit-and
is bad or happening too early.  w/o it we have

  <bb 2> [0.00%]:
  n_4 = (unsigned int) m_3(D);
  if (n_4 <= 2)
    goto <bb 3>; [0.00%]
  else
    goto <bb 5>; [0.00%]

  <bb 3> [100.00%]:
  _1 = (long unsigned int) n_4;
  if (_1 > 3)

which we can perfectly optimize (a much loved single-use test could
have us here).

Index: gcc/match.pd
===================================================================
--- gcc/match.pd        (revision 244859)
+++ gcc/match.pd        (working copy)
@@ -1840,7 +1840,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
         && final_int && inter_int && inside_int
         && final_prec == inside_prec
         && final_prec > inter_prec
-        && inter_unsignedp)
+        && inter_unsignedp
+        && single_use (@1))
      (convert (bit_and @0 { wide_int_to_tree
                              (inside_type,
                               wi::mask (inter_prec, false,


Richard.

> Martin
>