On Thu, Feb 20, 2025 at 3:14 PM Jeff Law <jeffreya...@gmail.com> wrote:
>
>
>
> On 2/20/25 12:52 AM, Richard Biener wrote:
> > On Wed, Feb 19, 2025 at 11:29 PM Jeff Law <jeffreya...@gmail.com> wrote:
> >>
> >>
> >> An interesting little bug. We're just emitting what appears to me to be
> >> a silly warning.
> >>
> >> By the time the array-bounds checker runs we have this statement in the
> >> IL:
> >>
> >>> MEM[(int *)_42 + -4B] ={v} {CLOBBER(eob)};
> >>
> >> Naturally that looks like an out of bounds array index and we warn.
> >> Which seems dubious at best. My first thought is we shouldn't be trying
> >> to analyze a gimple clobber for out of bounds array indexes.
> >
> > OTOH all clobbers are inserted for actual allocated storage (even if not
> > accessed).
> True, but the clobbers seem like the wrong object to be checking for OOB
> access. We should be checking actual references.
>
> >
> >> But like most things in GCC, it's never that simple.
> >>
> >> It looks like we're relying on warning for that case for
> >> Warray-bounds-20.C.
> >>
> >>
> >>> <bb 2> [local count: 1073741824]:
> >>> MEM[(struct D1 *)&a + 1B] ={v} {CLOBBER(bob)};
> >>> MEM[(struct B *)&a + 1B]._vptr.B = &MEM <int (*) ()> [(void
> >>> *)&_ZTC2D10_1B + 24B];
> >>> MEM[(struct A *)&a + 9B]._vptr.A = &MEM <int (*) ()> [(void
> >>> *)&_ZTC2D10_1B + 64B];
> >>> C::C (&MEM[(struct D1 *)&a + 1B].D.3003, &MEM <const void *[8]> [(void
> >>> *)&_ZTT2D1 + 48B]);
> >>
> >> My sense is that this test is passing more by accident than by design
> >> and that the warning really should be triggered by one of the other
> >> statements.
> >
> > And it isn't? So A and B are smaller than D1 and only D1 overruns?
> > But it's not actually stored to?
> The overruns are completely missed AFAICT. The warning we get today is
> from the clobber statement.
>
> >
> >> Martin just got it wrong here AFAICT. I did go back and
> >> check the original BZ (pr98266). It's unaffected as it doesn't have the
> >> gimple clobbers.
> >
> > I'll note the unwanted diagnostic is on a EOB (end-of-object) while the
> > wanted is on a BOB (begin-of-object). Both are not begin/end-of-storage.
> >
> > Now find a convincing argument that any of those are not worth diagnosing...
> I think diagnosing overruns is good, but the clobber statement isn't the
> right thing to be analyzing for an overrun.
The very case of placement new made me question this.
struct x { int x; };
char a[1];
x *p = new (a) x;
should be diagnosed (ideally). That we diagnose it as an array bound
violation is of course bogus - the wording could be improved to
say (for bob or bos) 'constructing object of type X does not fit in
storage of size Y'
or so.
For
struct x[2];
x *p = new (&x[2]) x;
of course there's an array bound violation. But the way Martin added handling
of general MEM_REF to array-bound diagnostics of course makes this
susceptible to be misleading ...
So I think the patch is OK if you file a bugreport about the missed object
instantiation diagnostic from clobbers and for the missed diagnostic on
the actual access you showed above (we likely fail to handle component-ref
subsetted MEM_REFs ...), basically for the testcases you XFAIL.
Thanks,
Richard.
>
> Jeff
