vsk added a comment.
@sberg I agree with @regehr's analysis, and do think that this is a real
overflow. Once https://reviews.llvm.org/D34121 lands, we will report this issue
in a better way:
runtime error: addition of unsigned offset to 0x7fff59dfe990 overflowed to
0x7fff59dfe980
regehr added a comment.
Well, my second program should subtract a multiple of sizeof(T). But anyway, my
view is that this is a real overflow and a nasty consequence of the unsigned
size_t and the usual arithmetic conversions and I don't think we want to try to
poke a hole in UBSan to allow
regehr added a comment.
Sorry, let's go with this example instead, which makes it clear that the
program is attempting to do something completely sensible:
#include
typedef struct {
int x[2];
} T;
int main(void) {
T f[1000];
T *p = [500];
ptrdiff_t d = -10;
p
regehr added a comment.
I'm taking a look. For reference here's the test program I'm using.
#include
typedef struct {
int x[2];
} T;
int main(void) {
T f;
T *p =
ptrdiff_t d = -3293184;
p += d / sizeof(T);
return 0;
}
Repository:
rL LLVM
sberg added a comment.
Just a heads up that I ran into an arguably somewhat unexpected instance of
this with (a copy of the Graphite project included in) LibreOffice, see the
commit message of
https://cgit.freedesktop.org/libreoffice/core/commit/?id=681b4a49d797996229513d3e842d2a431030730a
This revision was automatically updated to reflect the committed changes.
Closed by commit rL304459: [ubsan] Add a check for pointer overflow UB
(authored by vedantk).
Changed prior to commit:
https://reviews.llvm.org/D33305?vs=100475=101078#toc
Repository:
rL LLVM
vsk added a comment.
Thanks for the review! I've rebased the patch and plan on checking it in
tomorrow. At the moment I'm getting some additional test coverage (running
check-libcxx and testing more backends).
https://reviews.llvm.org/D33305
___
dtzWill added a comment.
LGTM!
I've built quite a bit with this (ground-up Linux distribution) which attests
to it being fairly robust (no crashing or new errors not experienced with
unpatched clang) and the bugs found so far are all true positives (few of which
were caught and reported last
vsk updated this revision to Diff 100475.
vsk edited the summary of this revision.
vsk added a comment.
Ping.
https://reviews.llvm.org/D33305
Files:
docs/UndefinedBehaviorSanitizer.rst
include/clang/Basic/Sanitizers.def
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CGExprScalar.cpp
vsk added a comment.
Thanks for the comments, responses inline --
Comment at: lib/CodeGen/CGExprScalar.cpp:3854
+ const Twine ) {
+ Value *GEPVal = Builder.CreateInBoundsGEP(Ptr, IdxList, Name);
+
filcab wrote:
>
filcab added inline comments.
Comment at: lib/CodeGen/CGExprScalar.cpp:3854
+ const Twine ) {
+ Value *GEPVal = Builder.CreateInBoundsGEP(Ptr, IdxList, Name);
+
You're creating the GEP first (possibly triggering
vsk created this revision.
Herald added a subscriber: krytarowski.
Check pointer arithmetic for overflow.
For some more background on this check, see:
https://wdtz.org/catching-pointer-overflow-bugs.html
https://reviews.llvm.org/D20322
Patch by Will Dietz and John Regehr!
This version of
12 matches
Mail list logo
