A question about detecting array bounds for case Warray-bounds-3.c

A question about detecting array bounds for case Warray-bounds-3.c

[Jiangning Liu]

Hi,

For case gcc/testsuite/gcc.dg/Warray-bounds-3.c, obviously it is an invalid
C program, because the last iterations of all the loops cause the access of
arrays is beyond the max size of corresponding array declarations. The
condition of checking upper bound should be "<" rather than "<=". 

Right now, GCC compiler doesn't report any warning messages for this case,
should it be a bug in both test case and compiler?

But looking at http://gcc.gnu.org/PR31227 , it seems this test case is
designed to be like this on purpose. Anybody can explain about this?

The case is like below,

/* { dg-do compile } */
/* { dg-options "-O2 -Warray-bounds" } */
/* based on PR 31227 */

struct S
{
  const char *abday[7];
  const char *day[7];
  const char *abmon[12];
  const char *mon[12];
  const char *am_pm[2];
};

...

  for (cnt = 0; cnt <= 7; ++cnt)
    {
      iov[2 + cnt].iov_base = (void *) (time->abday[cnt] ?: "");
      iov[2 + cnt].iov_len = strlen (iov[2 + cnt].iov_base) + 1;
    }

  for (; cnt <= 14; ++cnt)
    {
      iov[2 + cnt].iov_base = (void *) (time->day[cnt - 7] ?: "");
      iov[2 + cnt].iov_len = strlen (iov[2 + cnt].iov_base) + 1;
    }

  for (; cnt <= 26; ++cnt)
    {
      iov[2 + cnt].iov_base = (void *) (time->abmon[cnt - 14] ?: "");
      iov[2 + cnt].iov_len = strlen (iov[2 + cnt].iov_base) + 1;
    }

  for (; cnt <= 38; ++cnt)
    {
      iov[2 + cnt].iov_base = (void *) (time->mon[cnt - 26] ?: "");
      iov[2 + cnt].iov_len = strlen (iov[2 + cnt].iov_base) + 1;
    }

  for (; cnt <= 40; ++cnt)
    {
      iov[2 + cnt].iov_base =  (void *) (time->am_pm[cnt - 38] ?: "");
      iov[2 + cnt].iov_len = strlen (iov[2 + cnt].iov_base) + 1;
    }

Thanks,
-Jiangning

Re: A question about detecting array bounds for case Warray-bounds-3.c

[Dirk Müller]

On Monday 26 September 2011, Matthew Gretton-Dann wrote:

> As far as I understand it -Warray-bounds should be emitting a warning
> for this case, but PR31227 seemed to be about removing these warnings.
> 
> The PR comments do not explain why the array accesses are valid and I'm
> hoping someone can shed some light on the situation - what are we missing?

The fix for PR was when the address of an element beyond the array is taken, 
but not actually dereferenced (used). For cases where the element is 
dereferenced it should warn IMHO. 

Note however that in this case it accesses an adjacent array of the same type 
in memory, and it is arguable if it should give a warning there or not. I have 
no strong opinion about this (I suspect that choosing for one variant gives 
false positives, and the other false negatives). It seems fortify_source has a 
similar problem, which is why they have added an option for it (1/2): 

http://gcc.gnu.org/ml/gcc-patches/2004-09/msg02055.html

I guess we need the same approach here. 

Thanks,
Dirk

Re: A question about detecting array bounds for case Warray-bounds-3.c

[Matthew Gretton-Dann]

On 26/09/11 10:03, Jonathan Wakely wrote:
> On 26 September 2011 08:13, Jiangning Liu wrote:
>> PING...
>>
>>> -----Original Message-----
>>> From: Jiangning Liu [mailto:jiangning.liu@arm.com]
>>> Sent: Thursday, September 22, 2011 10:19 AM
>>> To: gcc@gcc.gnu.org
>>> Cc: 'jakub@gcc.gnu.org'; 'mueller@gcc.gnu.org'; 'rguenth@gcc.gnu.org';
>>> Matthew Gretton-Dann
>>> Subject: A question about detecting array bounds for case Warray-
>>> bounds-3.c
>>>
>>> Hi,
>>>
>>> For case gcc/testsuite/gcc.dg/Warray-bounds-3.c, obviously it is an
>>> invalid C program, because the last iterations of all the loops cause
>>> the access of arrays is beyond the max size of corresponding array
>>> declarations. The condition of checking upper bound should be "<"
>>> rather than "<=".
>
> Which loops are you referring to?
>
>    struct iovec iov[43];
> ...
>    for (; cnt<= 40; ++cnt)
>      {
>        iov[2 + cnt].iov_base =  (void *) (time->am_pm[cnt - 38] ?: "");
>        iov[2 + cnt].iov_len = strlen (iov[2 + cnt].iov_base) + 1;
>      }
>
> What's wrong with that?  The last element accessed is iov[42] which is ok.

This isn't about access to iov - but rather access to the arrays in 
struct S *time:

struct S
{
   const char *abday[7];
   const char *day[7];
   const char *abmon[12];
   const char *mon[12];
   const char *am_pm[2];
};

...

   for (cnt = 0; cnt <= 7; ++cnt)
     {
       iov[2 + cnt].iov_base = (void *) (time->abday[cnt] ?: "");
       iov[2 + cnt].iov_len = strlen (iov[2 + cnt].iov_base) + 1;
     }

The last iteration (cnt == 7) will dereference time->abday[7] which is 
one past the end of the array.

As far as I understand it -Warray-bounds should be emitting a warning 
for this case, but PR31227 seemed to be about removing these warnings.

The PR comments do not explain why the array accesses are valid and I'm 
hoping someone can shed some light on the situation - what are we missing?

Thanks,

Matt

-- 
Matthew Gretton-Dann
Principal Engineer, PD Software - Tools, ARM Ltd

Re: A question about detecting array bounds for case Warray-bounds-3.c

[Jonathan Wakely]

On 26 September 2011 08:13, Jiangning Liu wrote:
> PING...
>
>> -----Original Message-----
>> From: Jiangning Liu [mailto:jiangning.liu@arm.com]
>> Sent: Thursday, September 22, 2011 10:19 AM
>> To: gcc@gcc.gnu.org
>> Cc: 'jakub@gcc.gnu.org'; 'mueller@gcc.gnu.org'; 'rguenth@gcc.gnu.org';
>> Matthew Gretton-Dann
>> Subject: A question about detecting array bounds for case Warray-
>> bounds-3.c
>>
>> Hi,
>>
>> For case gcc/testsuite/gcc.dg/Warray-bounds-3.c, obviously it is an
>> invalid C program, because the last iterations of all the loops cause
>> the access of arrays is beyond the max size of corresponding array
>> declarations. The condition of checking upper bound should be "<"
>> rather than "<=".

Which loops are you referring to?

  struct iovec iov[43];
...
  for (; cnt <= 40; ++cnt)
    {
      iov[2 + cnt].iov_base =  (void *) (time->am_pm[cnt - 38] ?: "");
      iov[2 + cnt].iov_len = strlen (iov[2 + cnt].iov_base) + 1;
    }

What's wrong with that?  The last element accessed is iov[42] which is ok.

RE: A question about detecting array bounds for case Warray-bounds-3.c

[Jiangning Liu]

PING...

> -----Original Message-----
> From: Jiangning Liu [mailto:jiangning.liu@arm.com]
> Sent: Thursday, September 22, 2011 10:19 AM
> To: gcc@gcc.gnu.org
> Cc: 'jakub@gcc.gnu.org'; 'mueller@gcc.gnu.org'; 'rguenth@gcc.gnu.org';
> Matthew Gretton-Dann
> Subject: A question about detecting array bounds for case Warray-
> bounds-3.c
> 
> Hi,
> 
> For case gcc/testsuite/gcc.dg/Warray-bounds-3.c, obviously it is an
> invalid C program, because the last iterations of all the loops cause
> the access of arrays is beyond the max size of corresponding array
> declarations. The condition of checking upper bound should be "<"
> rather than "<=".
> 
> Right now, GCC compiler doesn't report any warning messages for this
> case, should it be a bug in both test case and compiler?
> 
> But looking at http://gcc.gnu.org/PR31227 , it seems this test case is
> designed to be like this on purpose. Anybody can explain about this?
> 
> The case is like below,
> 
> /* { dg-do compile } */
> /* { dg-options "-O2 -Warray-bounds" } */
> /* based on PR 31227 */
> 
> struct S
> {
>   const char *abday[7];
>   const char *day[7];
>   const char *abmon[12];
>   const char *mon[12];
>   const char *am_pm[2];
> };
> 
> ...
> 
>   for (cnt = 0; cnt <= 7; ++cnt)
>     {
>       iov[2 + cnt].iov_base = (void *) (time->abday[cnt] ?: "");
>       iov[2 + cnt].iov_len = strlen (iov[2 + cnt].iov_base) + 1;
>     }
> 
>   for (; cnt <= 14; ++cnt)
>     {
>       iov[2 + cnt].iov_base = (void *) (time->day[cnt - 7] ?: "");
>       iov[2 + cnt].iov_len = strlen (iov[2 + cnt].iov_base) + 1;
>     }
> 
>   for (; cnt <= 26; ++cnt)
>     {
>       iov[2 + cnt].iov_base = (void *) (time->abmon[cnt - 14] ?: "");
>       iov[2 + cnt].iov_len = strlen (iov[2 + cnt].iov_base) + 1;
>     }
> 
>   for (; cnt <= 38; ++cnt)
>     {
>       iov[2 + cnt].iov_base = (void *) (time->mon[cnt - 26] ?: "");
>       iov[2 + cnt].iov_len = strlen (iov[2 + cnt].iov_base) + 1;
>     }
> 
>   for (; cnt <= 40; ++cnt)
>     {
>       iov[2 + cnt].iov_base =  (void *) (time->am_pm[cnt - 38] ?: "");
>       iov[2 + cnt].iov_len = strlen (iov[2 + cnt].iov_base) + 1;
>     }
> 
> Thanks,
> -Jiangning