On 24 May 2015 4:57 pm, "Nuno Lopes" <[email protected]> wrote:
In http://reviews.llvm.org/D9673#170543, @samsonov wrote:
> In http://reviews.llvm.org/D9673#170474, @nlopes wrote:
>
> > > Wait a second, won't UBSan handle this automatically if
> > > memcpy/memmove are
> >
> > > declared with __attribute__((nonnull)) in the header? Otherwise,
> > > is there
> >
> > > a change to the standard that imposes these additional constraints
> > > on
> >
> > > memcpy/memmove?
> >
> >
> > Not really. memcpy/memmove calls are handled by CGBultin and not
> > CGCall.
> > It's a different code path.
> > Nuno
>
>
> Interesting. I think that if we decide to implement such a check, we
> shouldn't depend on
> attributes specified in the headers, so `nonnull-attribute` is no
> longer relevant. There are another
> kind of compiler builtins which worth extra checks, and which don't
> even require headers - e.g. behavior of __builtin_ctz(0)
> is undefined. I think we should implement another check kind
> `-fsanitize=builtin` that would verify arguments of
> various builtin functions.
Well, I think in the end they are all in the UB category, so I think they
fall in the scope of -fsanitize=undefined.
We have sanitizer groups (much like warning groups); nothing goes directly
in -fsanitize=undefined.
Sure. This patch reuses the same code path as checking for null arguments in
functions. Not sure if it justifies a separate flag in -fsanitize for
builtins instead of splitting these checks amongst the current set of
categories.
BTW I've already used this patch to catch multiple crashes in an application
due to gcc 4.9 optimizing memcpy with null args. Hence I think we should
have those checks in place to help people (myself included :).
Are you ok with the patch?
Nuno
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
