vabridgers updated this revision to Diff 419462.
vabridgers added a comment.
This revision is now accepted and ready to land.
make assert function void, update summary
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118050/new/
https://reviews.llvm.o
vabridgers planned changes to this revision.
vabridgers added a comment.
I need to make a few changes. Please hold off any comments until the next
patch, coming soon.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118050/new/
https://reviews.llvm.o
vabridgers added a comment.
I added #ifndef NDEBUG around the assert since I'm not sure if downstream
consumers that use different pointer sizes by address space will hit this
assert. Our downstream target causes this assert to fire, so I will be
continuing to find and fix these issues, submitt
vabridgers updated this revision to Diff 419438.
vabridgers added a comment.
add NDEBUG around assert
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118050/new/
https://reviews.llvm.org/D118050
Files:
clang/lib/StaticAnalyzer/Checkers/CStringChec
vabridgers updated this revision to Diff 419013.
vabridgers added a comment.
Herald added a project: All.
rebase, check ci
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118050/new/
https://reviews.llvm.org/D118050
Files:
clang/lib/StaticAnalyzer
vabridgers added inline comments.
Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:726-727
+ LhsLoc.getType(Ctx).isNull() ? 0 : Ctx.getTypeSize(LhsLoc.getType(Ctx));
+ if (RhsBitwidth && LhsBitwidth &&
+ (LhsLoc.getSubKind() == RhsLoc.getSubKind())) {
+
steakhal accepted this revision.
steakhal added a comment.
This revision is now accepted and ready to land.
Besides a couple of nits, it's ready for landing.
Thanks for the hard work @vabridgers!
Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:726-727
+ Lhs
vabridgers updated this revision to Diff 408104.
vabridgers added a comment.
Update after makeNull fixes
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118050/new/
https://reviews.llvm.org/D118050
Files:
clang/lib/StaticAnalyzer/Checkers/CStringC
vabridgers marked 7 inline comments as done.
vabridgers added a comment.
I think the comments have been addressed for now. Please let me know if I
missed something, or anything else needs to be done (besides back out the
change to SValBuilder.cpp when ready). Thanks for the comments. Best
Repo
vabridgers added a comment.
I think @steakhal will have a more comprehensive change to back out the
makeNull() calls to makeNullWithWidth() calls. I'll back out the change to
SValBuilder.cpp once that change is pushed. For now, this is my update. Thanks
Repository:
rG LLVM Github Monorepo
C
vabridgers updated this revision to Diff 404940.
vabridgers added a comment.
Simplify assertion per comments.
Add a "temp" fix in SValBuilder that permits the additional
test cases to pass without crash
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D
vabridgers added a comment.
I took a run at the improvements, and the rabbit hole keeps getting deeper :/
@steakhal, I'll contact you offline to work out the problems. Best
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118050/new/
https://review
vabridgers marked an inline comment as done.
vabridgers added a comment.
Thanks for the comments, I'll address. Best
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118050/new/
https://reviews.llvm.org/D118050
___
NoQ added inline comments.
Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:728
+const MemRegion *R = loc.castAs().getRegion();
+if (const auto *SR = dyn_cast(R)) {
+ QualType Ty = SR->getSymbol()->getType();
That's a pretty big "if".
steakhal added inline comments.
Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:718
+static uint64_t getLocBitwidth(ProgramStateRef State, Loc loc) {
+ uint64_t LocBitwidth = 0;
Why don't you simply call `loc->getType()` end delegate the heavy
vabridgers marked an inline comment as done.
vabridgers added inline comments.
Comment at: clang/test/Analysis/cstring-checker-addressspace.c:14
+// Copy from host to device memory.
+DEVICE void *memcpy(DEVICE void *dst, const void *src, unsigned long len);
+
ste
vabridgers added a comment.
I updated the test case after verifying I can reproduce the crash without the
fix. I also experimented with adding a bitwidth check for the lhs and rhs loc
parameters to evalBinOpLL, and verified this assert catches the negative
(crash) case.
I explored adding a 'me
vabridgers updated this revision to Diff 403276.
vabridgers added a comment.
- update test case
- add initial Loc bitwidth check for evalBinOpLL for review
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118050/new/
https://reviews.llvm.org/D118050
vabridgers added a comment.
Good thoughts, I will try those things and get back to you.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118050/new/
https://reviews.llvm.org/D118050
___
cfe-commits mailing
steakhal added a subscriber: NoQ.
steakhal added a comment.
I'm wondering if we should have an assertion within the
`SValBuilder::evalBinOpLL()` asserting that the pointers should have the same
bitwidths.
That's better than having nothing, waiting for a bugreport containing such a
constraint on
vabridgers created this revision.
vabridgers added reviewers: steakhal, martong.
Herald added subscribers: manas, ASDenysPetrov, dkrupp, donat.nagy, Szelethus,
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.
vabridgers requested review of this revision.
Herald added a
21 matches
Mail list logo
