mark maule <[email protected]>:
> On 5/20/2015 3:27 AM, Richard Biener wrote:
> > On Mon, May 18, 2015 at 10:01 PM, mark maule <[email protected]> wrote:
> > The usual issue with this kind of behavior is out-of-bound accesses of
> > arrays in a loop
> > or invoking undefined behavior when signed integer operations wrap.
> >
> >
> > uint32_t outLun[ BS_CFG_DRIVE_GROUPS ];
> >
> > and
> >
> > while ( ( dgHandle < ( BS_CFG_DRIVE_GROUPS + 1 ) ) &&
> > ...
> > dgDestageOut = bs_destageData.outLun[ dgHandle ];
> >
> > looks like this might access outLun[BS_CFG_DRIVE_GROUPS] which is
> > out-of-bounds.
> >
> > Richard.
>
> You are correct, and when I change outLun[] to be size
> BS_CFG_DRIVE_GROUPS+1, the generated asm looks like it will account for
> dgHandle in the while() loop. I will pass this back to our development
> team to get a proper fix.
>
> Now, the followon: Something in the compiler/optimizer recognized this
> out of bounds situation - should a warning have been emitted? Or are
> there ambiguities which make a warning generation here inappropriate?
Yes, ideally a compiler should emit a warning. C compilers traditionally
were not very good at this, but it turns out very recent versions of GCC
can do this:
test.c:14:23: warning: iteration 10u invokes undefined behavior
[-Waggressive-loop-optimizations]
dgDestageOut = outLun[ dgHandle ];
^
test.c:11:13: note: containing loop
while ( ( dgHandle < ( BS_CFG_DRIVE_GROUPS + 1 ) ) )
For this simplified test case:
#include <stdint.h>
#define BS_CFG_DRIVE_GROUPS 10
uint32_t dgDestageLimit = 0;
uint32_t outLun[ BS_CFG_DRIVE_GROUPS ];
void test(void)
{
int dgHandle = 0;
while ( ( dgHandle < ( BS_CFG_DRIVE_GROUPS + 1 ) ) )
{
uint32_t dgDestageOut;
dgDestageOut = outLun[ dgHandle ];
dgHandle++;
}
}
Martin