Re: optimization question

[mark maule <mark dot maule at oracle dot com>]

On 5/20/2015 2:13 PM, Martin Uecker wrote:
>   mark maule <mark.maule@oracle.com>:
> > On 5/20/2015 3:27 AM, Richard Biener wrote:
> > > On Mon, May 18, 2015 at 10:01 PM, mark maule <mark.maule@oracle.com> 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

Thanks Martin.

My gcc (4.8.3) must not be recent enough.  Any idea when this sort of 
checking went in?

#include <stdint.h>

#define BS_CFG_DRIVE_GROUPS 0x1e0

uint32_t dgDestageLimit = 0;
uint32_t outLun[ 10 ];

int
test(void)
{
        int dgHandle = 0;
        int total;

        while ( ( dgHandle < ( BS_CFG_DRIVE_GROUPS + 1 ) ) ) {
                uint32_t dgDestageOut;
                dgDestageOut = outLun[ dgHandle ];
                total += dgDestageOut;
                dgHandle++;
        }

        return dgHandle;
}

int
main(int argc, char **argv)
{
        test();
        return 0;
}


ca-bld-ol71-02$ cc -o foo foo.c -Wall
ca-bld-ol71-02$