zaks.anna accepted this revision.
zaks.anna added a comment.
This revision is now accepted and ready to land.
Thi has been committed in r290505.
https://reviews.llvm.org/D22862
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
dcoughlin accepted this revision.
dcoughlin added a comment.
Overall, this looks good to me. Thanks for tackling this!
One request, though. Could you move the tests into already existing test files?
We're trying to avoid test files that only test a single issue. This makes it
easier for people
ayartsev updated this revision to Diff 78810.
ayartsev added a comment.
The updated patch implements Devin's solution. Please review.
https://reviews.llvm.org/D22862
Files:
lib/StaticAnalyzer/Core/ExprEngineC.cpp
lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
NoQ added a comment.
> @dcoughlin, @NoQ, could you, please, tell, how you get dumps of symbolic
> expressions and constraints like "(conj_$6{void *}) != 0U"? Tried different
> debug.* checkers and clang_analyzer_explain() but failed.
That's `SVal.dump()`, `SymbolRef->dump()`,
ayartsev added a comment.
@zaks.anna, sorry for the noise about the "misc-ps-region-store.m" test, my
mistake.
In https://reviews.llvm.org/D22862#508674, @NoQ wrote:
> Hmm. The test in `unwanted-programstate-data-propagation.c` passes on my
> machine even without the patch, and the return
NoQ added a comment.
Hmm. The test in `unwanted-programstate-data-propagation.c` passes on my
machine even without the patch, and the return value on the respective path is
correctly represented as `(conj_$6{void *}) != 0U`, which comes from the
`evalCast()` call in `VisitLogicalExpr()` and is
dcoughlin added a comment.
As PR15623 shows, returning the existing cast is not correct. But rather than
replace it with an unknown, here is a proposal for how to address this without
regressing in precision.
Instead of using `assumeDual()` in `ExprEngine::VisitLogicalExpr()` on the
`RHSVal`
ayartsev added inline comments.
Comment at: test/Analysis/misc-ps-region-store.m:332
@@ -330,3 +331,3 @@
if (p < q) {
// If we reach here, 'p' cannot be null. If 'p' is null, then 'n' must
// be '0', meaning that this branch is not feasible.
NoQ added inline comments.
Comment at: test/Analysis/misc-ps-region-store.m:332
@@ -330,3 +331,3 @@
if (p < q) {
// If we reach here, 'p' cannot be null. If 'p' is null, then 'n' must
// be '0', meaning that this branch is not feasible.
ayartsev
zaks.anna added a comment.
I am not sure it's the right way of fixing this problem and it introduces a
regression. The bug is probably specific to "&&".
+ Devin as we might have seen something similar.
Comment at: test/Analysis/misc-ps-region-store.m:332
@@ -330,3 +331,3 @@
ayartsev created this revision.
ayartsev added reviewers: zaks.anna, krememek.
ayartsev added a subscriber: cfe-commits.
The attached patch eliminates unneeded checker data propagation from one of the
operands of a logical operation to the operation result. The result of a
logical operation is
11 matches
Mail list logo
