Re: optimization question

[mark maule]

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