lebedev.ri added inline comments.
================
Comment at: clang/docs/UndefinedBehaviorSanitizer.rst:177
``-fsanitize=undefined``.
+ - ``-fsanitize=pointer-offsetting``: Enables ``nullptr-and-nonzero-offset``
+ and ``pointer-overflow``.
----------------
vsk wrote:
> Why does this need to be a separate sanitizer, as opposed to being a part of
> -fsanitize=pointer-overflow? Is there a need for a new group?
That ties in with the question i'm asking myself in last update:
https://reviews.llvm.org/D67122#1656418
I.e., would it be okay to produce *all* these checks for **every** `gep
inbounds` we codegen, indiscriminantly,
as opposed to only always producing this `nullptr-and-nonzero-offset`?
================
Comment at: compiler-rt/lib/ubsan/ubsan_handlers.cpp:715
+ "applying non-zero offset to non-null pointer %0 producing null "
+ "pointer is undefined behaviour")
+ << (void *)Base;
----------------
vsk wrote:
> I don't think these diagnostics generally need to repeat the fact that there
> is UB: that seems implicit.
>
> How about: 'non-zero offset %0 applied to null pointer', and 'non-zero offset
> %0 applied to non-null pointer %1 produced null'?
> I don't think these diagnostics generally need to repeat the fact that there
> is UB: that seems implicit.
Sure, ok.
> 'non-zero offset %0 applied to non-null pointer %1 produced null'?
I don't want to spell any more info that it already does, it isn't really
useful //here//.
Also, we only receive the base pointer and the computed pointer.
We don't know the actual offset, we may have done `ptr - intptr_t(ptr)`,
or maybe it was `ptr + (0-intptr_t(ptr))`.
(Sure, we could receive more info, but i'm not sure worth it here.)
================
Comment at: llvm/docs/ReleaseNotes.rst:59
+ ``getelementptr inbounds`` can not change the null status of a pointer,
+ meaning it can not produce non-null pointer given null base pointer, and
+ likewise given non-null base pointer it can not produce null pointer; if it
----------------
xbolva00 wrote:
> lebedev.ri wrote:
> > xbolva00 wrote:
> > > lebedev.ri wrote:
> > > > xbolva00 wrote:
> > > > > Please add a reference to C/C++ standard that this is a UB.
> > > > It is irrelevant what semantics C/C++ standard has/doesn't have for
> > > > whatever C/C++ construct,
> > > > this is talking about LLVM IR.
> > > Ok, so Clang docs?
> > >
> > > “The compiler now optimizes ... since ... is a UB according to ... Use
> > > UBSan to catch these cases.”
> > ... see `clang/docs/ReleaseNotes.rst` then?
> > Those words were present in the first revision of the differential already,
> > https://reviews.llvm.org/D67122?id=218504#change-RNUd6wICb9iD
> Oh, sorry.
NP :)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67122/new/
https://reviews.llvm.org/D67122
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
