Re: [C PATCH 3/4] introduce ubsan checking for assigment of VM types 3/4

[Martin Uecker <uecker@tugraz.at>]

Am Dienstag, dem 30.05.2023 um 22:59 +0000 schrieb Joseph Myers:
> On Mon, 29 May 2023, Martin Uecker via Gcc-patches wrote:
> 
> >     Support instrumentation of function arguments for functions
> >     called via a declaration.  We can support only simple size
> 
> What do you mean by "via a declaration"?
> 
> If the *definition* is visible (and known to be the definition used
> at runtime rather than being interposed) then you can determine in
> some cases  that there is UB from bad bounds.  If only some other
> declaration is visible, or the definition might be interposed, VLA
> sizes in the declaration are equivalent to [*]; it's suspicious if
> they don't match, but it's not UB and so it would seem rather
> questionable for UBSan to treat it as such (cf. the rejection in 
> GCC of sanitization for some questionable cases of unsigned integer
> overflow that aren't UB either).

You are right that it is UB only with the additional
assumption that the bounds in the seen declaration are
the same as the ones in the definition.   But we now warn
about any mismatch since GCC 11 with -Wall based on the
understanding  that any such mismatch should be considered
a bug. There also does not seem  to be any valid use case
for having mismatching bounds and I think the intention 
of WG14 is clearly that they can be used for checking 
(cf. WG14 charter). So I think this is a different
situation for unsigned integer overflow.

Fom a practial point of view is is certainly very useful 
for users to be able to verify these bounds at run-time. 
But we could make it a separate UBSan option if it is
really a concern.

BTW: There was a similar discussion years ago about making
certain bound checks for arrays part of UBSan because it is
not clear that the bounds in the type of 'x' in x[n] are
relevant rather than the ones of the underlying array
(which may be different).  In the end both GCC and clang
have these UBSan checks now and I think  everybody is
happy about it.


> > +         /*  Give up.  If we do not understand a size expression,
> > we can
> > +             also not instrument any of the others because it may
> > have
> > +             side effects affecting them.  (We could restart and
> > instrument
> > +             the only the ones with integer constants.)   */
> > +           warning_at (location, 0, "Function call not
> > instrumented.");
> > +           return void_node;
> 
> This is not a properly formatted diagnostic message (should start
> with a 
> lowercase letter and not end with '.').

Thanks. I would probably remove this warning and re-introduce it
with another patch that also adds an option fir it.

Martin