NoQ created this revision.
NoQ added reviewers: pgousseau, zaks.anna, dcoughlin, xazax.hun.
NoQ added a subscriber: cfe-commits.
Sorry for being a bit slow, i should have had a look at the review earlier; i
noticed some stuff after the recent patch by Pierre Gousseau was committed.
1. There's an off-by-one in the comparison `evalBinOp` in `evalIntegralCast`,
the patch attached to this revision fixes it, and tests it via bool-assignment
checker.
2. There's also a FIXME test for the same checker, which is a regression after
this patch (used to throw a warning, now it doesn't); seems not as bad as the
crash, so it can probably be addressed later.
3. I noticed that it's still possible to trigger the crash with the following
test, not sure what to do with it:
```
void f1(long foo)
{
unsigned index = -1;
if (index < foo - 1) index = foo;
if (index + 1 == 0) {}
}
```
So my best idea is to push this quick fix for off-by-one and then think where
to go further. Probably we need to teach `RangeConstraintManager` to work with
`SymbolCast`'s correctly (eg. understand how their range is related to a parent
symbol's range). Eventually, once it gets better, we'd probably want to change
the following in `evalIntegralCast`:
```
std::tie(IsNotTruncated, IsTruncated) = state->assume(CompVal);
- if (!IsNotTruncated && IsTruncated) {
+ if (IsTruncated) {
```
but right now it breaks tests (not only bool-assignment, but also other
checkers that start failing can probably be shown to loose warnings after this
patch, but i didn't have a look at this yet, so not sure, just hand-waving).
Anyway, generally, overally, i think r257464 is a step in the very right
direction if we want to work with casts better, and i was actually very happy
to see it went that way :)
http://reviews.llvm.org/D16178
Files:
lib/StaticAnalyzer/Core/SValBuilder.cpp
test/Analysis/bool-assignment.c
Index: test/Analysis/bool-assignment.c
===
--- test/Analysis/bool-assignment.c
+++ test/Analysis/bool-assignment.c
@@ -42,6 +42,15 @@
BOOL x = y; // expected-warning {{Assignment of a non-Boolean value}}
return;
}
+ if (y > 200 && y < 250) {
+// FIXME: Currently we are loosing this warning due to a SymbolCast in RHS.
+BOOL x = y; // no-warning
+return;
+ }
+ if (y >= 127 && y < 150) {
+BOOL x = y; // expected-warning{{Assignment of a non-Boolean value}}
+return;
+ }
if (y > 1) {
BOOL x = y; // expected-warning {{Assignment of a non-Boolean value}}
return;
Index: lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -451,7 +451,7 @@
NonLoc FromVal = val.castAs();
QualType CmpTy = getConditionType();
NonLoc CompVal =
- evalBinOpNN(state, BO_LT, FromVal, ToTypeMaxVal, CmpTy).castAs();
+ evalBinOpNN(state, BO_LE, FromVal, ToTypeMaxVal, CmpTy).castAs();
ProgramStateRef IsNotTruncated, IsTruncated;
std::tie(IsNotTruncated, IsTruncated) = state->assume(CompVal);
if (!IsNotTruncated && IsTruncated) {
Index: test/Analysis/bool-assignment.c
===
--- test/Analysis/bool-assignment.c
+++ test/Analysis/bool-assignment.c
@@ -42,6 +42,15 @@
BOOL x = y; // expected-warning {{Assignment of a non-Boolean value}}
return;
}
+ if (y > 200 && y < 250) {
+// FIXME: Currently we are loosing this warning due to a SymbolCast in RHS.
+BOOL x = y; // no-warning
+return;
+ }
+ if (y >= 127 && y < 150) {
+BOOL x = y; // expected-warning{{Assignment of a non-Boolean value}}
+return;
+ }
if (y > 1) {
BOOL x = y; // expected-warning {{Assignment of a non-Boolean value}}
return;
Index: lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -451,7 +451,7 @@
NonLoc FromVal = val.castAs();
QualType CmpTy = getConditionType();
NonLoc CompVal =
- evalBinOpNN(state, BO_LT, FromVal, ToTypeMaxVal, CmpTy).castAs();
+ evalBinOpNN(state, BO_LE, FromVal, ToTypeMaxVal, CmpTy).castAs();
ProgramStateRef IsNotTruncated, IsTruncated;
std::tie(IsNotTruncated, IsTruncated) = state->assume(CompVal);
if (!IsNotTruncated && IsTruncated) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
