Re: RFA: new pass to warn on questionable uses of alloca() and VLAs

[Aldy Hernandez <aldyh at redhat dot com>]

On 07/18/2016 11:14 PM, Martin Sebor wrote:
> > How does this look?
>
> I think it's 99% there.  You've addressed all of my comments so
> far -- thanks for that and for being so patient.  I realize it
> would be a lot more efficient to get all the feedback (or as much
> of it as possible) up front.  Unfortunately, some things don't get
> noticed until round 2 or 3 (or even 4).  Please take this in lieu
> of an apology for not spotting the issues below until now(*).

No problem, although I think we're getting to the point of diminishing 
returns with regards to functionality.  It may be best to involve Jeff 
or another global reviewer at this point for a review.  We can address 
any more minor things as a follow-up patch.  (Unless you find any show 
stoppers before then :)).

> For this code:
>
>    void f (void*);
>
>    void g (int n)
>    {
>      int a [n];
>      f (a);
>    }
>
> -Wvla-larger-than=32 prints:
>
>    warning: argument to variable-length array may be too large
>    note: limit is 32 bytes, but argument may be 18446744073709551612
>
> An int argument cannot be that large.  I suspect the printed value
> is actually the size of the VLA in bytes when N is -1, truncated
> to size_t, rather than the value of the VLA bound.  To avoid
> confusion the note should be corrected to say something like:
>
>    note: limit is 32 bytes, but the variable-length array may be
>    as large as 18446744073709551612

Note adjusted.

> Also, the checker prints false positives for code like:
>
>    void f (void*);
>
>    void g (unsigned x, int *y)
>    {
>      if (1000 < x) return;
>
>      while (*y) {
>        char a [x];
>        f (a);
>      }
>    }
>
> With -Wvla-larger-than=1000 and greater it prints:
>
>    warning: unbounded use of variable-length array
>
> (Same thing with alloca).  There should be no warning for VLAs,
> and for alloca, the warning should say "use of variable-length
> array within a loop."  The VRP dump suggests the range information
> is available within the loop.  Is the get_range_info() function
> not returning the corresponding bounds?

This is a false positive, but there's little we can do with the current 
range infrastructure.  The range information becomes less precise the 
further down the optimization pipeline we get.  So, even though as far 
as *.c.126t.crited1, we still see appropriate range information:

   # RANGE [0, 1000] NONZERO 1023
   _10 = (sizetype) x_3(D);
...
   a.1_12 = __builtin_alloca_with_align (_10, 8);

The PRE pass cleans things up in such a way that we end up with:

   <bb 2>:
   if (x_3(D) > 1000)
     goto <bb 8>;
   else
     goto <bb 3>;
...
   <bb 3>:
   # VUSE <.MEM_2(D)>
   _16 = *y_1(D);
   if (_16 != 0)
     goto <bb 9>;
   else
     goto <bb 8>;

   <bb 9>:

   <bb 4>:
   # <-----------------NO RANGE INFO--------->
   _4 = (sizetype) x_3(D);

...

a.1_12 = __builtin_alloca_with_align (_4, 8);

The -Walloca pass comes after PRE, which means we no longer have any 
range information for _4, and chasing the IL to glean this information 
would be fragile at best.  We will just have to live with this until we 
have better pervasive range information.

Updated patch tested on x86-64 Linux.

Aldy

Attachment: curr
Description: Text document