Am Thu, 21 May 2015 07:45:19 -0500
schrieb mark maule <mark.ma...@oracle.com>:
>
>
> 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?
>
Sorry, I don't know. I think 4.8 already has this warning,
but maybe isn't smart enough to detect this case. Also I think
there is no garantuee that all errors of this type are detected
with GCC 5. And this may also depend on your optimizer flags
and the phase of the moon.
If you want to detect this kind of bugs, you may find
the -fsanitize=undefined useful to detect undefined behaviour
at runtime. Also there other code analyzer tools which may
help.
Martin
