ddcc updated this revision to Diff 93591.
ddcc added a comment.
Rebase
https://reviews.llvm.org/D28955
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp
lib/StaticAnalyzer/Core/ProgramState.cpp
lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
lib/StaticAnalyzer/Core/Store.cpp
test/Analysis/dead-stores.m
test/Analysis/explain-svals.cpp
test/Analysis/malloc.c
test/Analysis/misc-ps-eager-assume.m
test/Analysis/std-c-library-functions.c
Index: test/Analysis/std-c-library-functions.c
===================================================================
--- test/Analysis/std-c-library-functions.c
+++ test/Analysis/std-c-library-functions.c
@@ -146,7 +146,7 @@
void test_isgraph_isprint(int x) {
char y = x;
if (isgraph(y))
- clang_analyzer_eval(isprint(x)); // expected-warning{{TRUE}}
+ clang_analyzer_eval(isprint(y)); // expected-warning{{TRUE}}
}
int isdigit(int);
Index: test/Analysis/misc-ps-eager-assume.m
===================================================================
--- test/Analysis/misc-ps-eager-assume.m
+++ test/Analysis/misc-ps-eager-assume.m
@@ -1,5 +1,4 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core -analyzer-store=region -verify -fblocks %s -analyzer-eagerly-assume
-// expected-no-diagnostics
// Delta-reduced header stuff (needed for test cases).
typedef signed char BOOL;
@@ -56,7 +55,7 @@
void handle_symbolic_cast_in_condition(void) {
NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];
- BOOL needsAnArray = [@"aString" isEqualToString:@"anotherString"];
+ BOOL needsAnArray = [@"aString" isEqualToString:@"anotherString"]; // expected-warning {{Assignment of a non-Boolean value}}
NSMutableArray* array = needsAnArray ? [[NSMutableArray alloc] init] : 0;
if(needsAnArray)
[array release];
Index: test/Analysis/malloc.c
===================================================================
--- test/Analysis/malloc.c
+++ test/Analysis/malloc.c
@@ -1656,13 +1656,13 @@
void testOffsetPassedToStrlen() {
char * string = malloc(sizeof(char)*10);
string += 1;
- int length = strlen(string); // expected-warning {{Potential leak of memory pointed to by 'string'}}
+ size_t length = strlen(string); // expected-warning {{Potential leak of memory pointed to by 'string'}}
}
void testOffsetPassedToStrlenThenFree() {
char * string = malloc(sizeof(char)*10);
string += 1;
- int length = strlen(string);
+ size_t length = strlen(string);
free(string); // expected-warning {{Argument to free() is offset by 1 byte from the start of memory allocated by malloc()}}
}
@@ -1705,7 +1705,7 @@
}
char *dupstrNoWarn(const char *s) {
- const int len = strlen(s);
+ const size_t len = strlen(s);
char *p = (char*) smallocNoWarn(len + 1);
strcpy(p, s); // no-warning
return p;
@@ -1721,7 +1721,7 @@
}
char *dupstrWarn(const char *s) {
- const int len = strlen(s);
+ const size_t len = strlen(s);
char *p = (char*) smallocWarn(len + 1);
strcpy(p, s); // expected-warning{{String copy function overflows destination buffer}}
return p;
Index: test/Analysis/explain-svals.cpp
===================================================================
--- test/Analysis/explain-svals.cpp
+++ test/Analysis/explain-svals.cpp
@@ -41,11 +41,11 @@
void test_2(char *ptr, int ext) {
clang_analyzer_explain((void *) "asdf"); // expected-warning-re{{{{^pointer to element of type 'char' with index 0 of string literal "asdf"$}}}}
- clang_analyzer_explain(strlen(ptr)); // expected-warning-re{{{{^metadata of type 'unsigned long' tied to pointee of argument 'ptr'$}}}}
+ clang_analyzer_explain(strlen(ptr)); // expected-warning-re{{{{^cast of type 'int' of metadata of type 'unsigned long' tied to pointee of argument 'ptr'$}}}}
clang_analyzer_explain(conjure()); // expected-warning-re{{{{^symbol of type 'int' conjured at statement 'conjure\(\)'$}}}}
clang_analyzer_explain(glob); // expected-warning-re{{{{^value derived from \(symbol of type 'int' conjured at statement 'conjure\(\)'\) for global variable 'glob'$}}}}
clang_analyzer_explain(glob_ptr); // expected-warning-re{{{{^value derived from \(symbol of type 'int' conjured at statement 'conjure\(\)'\) for global variable 'glob_ptr'$}}}}
- clang_analyzer_explain(clang_analyzer_getExtent(ptr)); // expected-warning-re{{{{^extent of pointee of argument 'ptr'$}}}}
+ clang_analyzer_explain(clang_analyzer_getExtent(ptr)); // expected-warning-re{{{{^cast of type 'int' of extent of pointee of argument 'ptr'$}}}}
int *x = new int[ext];
clang_analyzer_explain(x); // expected-warning-re{{{{^pointer to element of type 'int' with index 0 of heap segment that starts at symbol of type 'int \*' conjured at statement 'new int \[ext\]'$}}}}
// Sic! What gets computed is the extent of the element-region.
Index: test/Analysis/dead-stores.m
===================================================================
--- test/Analysis/dead-stores.m
+++ test/Analysis/dead-stores.m
@@ -1,5 +1,4 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core -analyzer-checker=deadcode.DeadStores,osx.cocoa.RetainCount -fblocks -verify -Wno-objc-root-class %s
-// expected-no-diagnostics
typedef signed char BOOL;
typedef unsigned int NSUInteger;
@@ -55,7 +54,7 @@
- (void) bar_rbar8527823
{
@synchronized(self) {
- BOOL isExec = baz_rdar8527823(); // no-warning
+ BOOL isExec = baz_rdar8527823(); // expected-warning {{Assignment of a non-Boolean value}}
if (isExec) foo_rdar8527823();
}
}
Index: lib/StaticAnalyzer/Core/Store.cpp
===================================================================
--- lib/StaticAnalyzer/Core/Store.cpp
+++ lib/StaticAnalyzer/Core/Store.cpp
@@ -439,9 +439,6 @@
// Pointer of any type can be cast and used as array base.
const ElementRegion *ElemR = dyn_cast<ElementRegion>(BaseRegion);
- // Convert the offset to the appropriate size and signedness.
- Offset = svalBuilder.convertToArrayIndex(Offset).castAs<NonLoc>();
-
if (!ElemR) {
//
// If the base region is not an ElementRegion, create one.
@@ -452,6 +449,9 @@
//
// Observe that 'p' binds to an AllocaRegion.
//
+
+ // Convert the offset to the appropriate size and signedness.
+ Offset = svalBuilder.convertToArrayIndex(Offset).castAs<NonLoc>();
return loc::MemRegionVal(MRMgr.getElementRegion(elementType, Offset,
BaseRegion, Ctx));
}
@@ -476,7 +476,9 @@
Ctx));
}
- const llvm::APSInt& OffI = Offset.castAs<nonloc::ConcreteInt>().getValue();
+ // FIXME: This isn't quite correct, but avoids casting the Offset symbol
+ llvm::APSInt OffI = APSIntType(BaseIdxI).convert(
+ Offset.castAs<nonloc::ConcreteInt>().getValue());
assert(BaseIdxI.isSigned());
// Compute the new index.
Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===================================================================
--- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -83,28 +83,19 @@
if (isLocType)
return LI->getLoc();
- // FIXME: Correctly support promotions/truncations.
- unsigned castSize = Context.getTypeSize(castTy);
- if (castSize == LI->getNumBits())
- return val;
- return makeLocAsInteger(LI->getLoc(), castSize);
+ return makeLocAsInteger(LI->getLoc(), Context.getTypeSize(castTy));
}
if (const SymExpr *se = val.getAsSymbolicExpression()) {
QualType T = Context.getCanonicalType(se->getType());
- // If types are the same or both are integers, ignore the cast.
- // FIXME: Remove this hack when we support symbolic truncation/extension.
- // HACK: If both castTy and T are integers, ignore the cast. This is
- // not a permanent solution. Eventually we want to precisely handle
- // extension/truncation of symbolic integers. This prevents us from losing
- // precision when we assign 'x = y' and 'y' is symbolic and x and y are
- // different integer types.
- if (haveSameType(T, castTy))
+ // If types are the same, ignore the cast.
+ if (haveSameType(T, castTy))
return val;
- if (!isLocType)
- return makeNonLoc(se, T, castTy);
- return UnknownVal();
+ if (isLocType)
+ return UnknownVal();
+
+ return makeNonLoc(se, T, castTy);
}
// If value is an unsupported constant, produce unknown.
Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -266,34 +266,15 @@
if (sym->getType()->isRealFloatingType()) {
if (const llvm::APFloat *Float = CM.getSymFloatVal(this, sym)) {
- // FIXME: Because we don't correctly model (yet) sign-extension
- // and truncation of symbolic values, we need to convert
- // the integer value to the correct signedness and bitwidth.
- //
- // This shows up in the following:
- //
- // char foo();
- // unsigned x = foo();
- // if (x == 54)
- // ...
- //
- // The symbolic value stored to 'x' is actually the conjured
- // symbol for the call to foo(); the type of that symbol is 'char',
- // not unsigned.
- const llvm::APFloat &NewV = getBasicVals().Convert(T, *Float);
-
assert(!V.getAs<Loc>() && "Loc::ConcreteFloat not supported!");
- return nonloc::ConcreteFloat(NewV);
+ return nonloc::ConcreteFloat(*Float);
}
} else {
if (const llvm::APSInt *Int = CM.getSymVal(this, sym)) {
- // FIXME: Likewise
- const llvm::APSInt &NewV = getBasicVals().Convert(T, *Int);
-
if (V.getAs<Loc>())
- return loc::ConcreteInt(NewV);
+ return loc::ConcreteInt(*Int);
else
- return nonloc::ConcreteInt(NewV);
+ return nonloc::ConcreteInt(*Int);
}
}
}
Index: lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp
+++ lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp
@@ -69,87 +69,26 @@
// Get the value of the right-hand side. We only care about values
// that are defined (UnknownVals and UndefinedVals are handled by other
// checkers).
- Optional<DefinedSVal> DV = val.getAs<DefinedSVal>();
- if (!DV)
+ Optional<NonLoc> NV = val.getAs<NonLoc>();
+ if (!NV)
return;
// Check if the assigned value meets our criteria for correctness. It must
// be a value that is either 0 or 1. One way to check this is to see if
// the value is possibly < 0 (for a negative value) or greater than 1.
ProgramStateRef state = C.getState();
SValBuilder &svalBuilder = C.getSValBuilder();
+ BasicValueFactory &BVF = svalBuilder.getBasicValueFactory();
ConstraintManager &CM = C.getConstraintManager();
- // First, ensure that the value is >= 0.
- DefinedSVal zeroVal = svalBuilder.makeIntVal(0, valTy);
- SVal greaterThanOrEqualToZeroVal =
- svalBuilder.evalBinOp(state, BO_GE, *DV, zeroVal,
- svalBuilder.getConditionType());
+ llvm::APSInt Zero = BVF.getValue(0, valTy);
+ llvm::APSInt One = BVF.getValue(1, valTy);
- Optional<DefinedSVal> greaterThanEqualToZero =
- greaterThanOrEqualToZeroVal.getAs<DefinedSVal>();
+ ProgramStateRef StIn, StOut;
+ std::tie(StIn, StOut) = CM.assumeInclusiveRangeDual(state, *NV, Zero, One);
- if (!greaterThanEqualToZero) {
- // The SValBuilder cannot construct a valid SVal for this condition.
- // This means we cannot properly reason about it.
- return;
- }
-
- ProgramStateRef stateLT, stateGE;
- std::tie(stateGE, stateLT) = CM.assumeDual(state, *greaterThanEqualToZero);
-
- // Is it possible for the value to be less than zero?
- if (stateLT) {
- // It is possible for the value to be less than zero. We only
- // want to emit a warning, however, if that value is fully constrained.
- // If it it possible for the value to be >= 0, then essentially the
- // value is underconstrained and there is nothing left to be done.
- if (!stateGE)
- emitReport(stateLT, C);
-
- // In either case, we are done.
- return;
- }
-
- // If we reach here, it must be the case that the value is constrained
- // to only be >= 0.
- assert(stateGE == state);
-
- // At this point we know that the value is >= 0.
- // Now check to ensure that the value is <= 1.
- DefinedSVal OneVal = svalBuilder.makeIntVal(1, valTy);
- SVal lessThanEqToOneVal =
- svalBuilder.evalBinOp(state, BO_LE, *DV, OneVal,
- svalBuilder.getConditionType());
-
- Optional<DefinedSVal> lessThanEqToOne =
- lessThanEqToOneVal.getAs<DefinedSVal>();
-
- if (!lessThanEqToOne) {
- // The SValBuilder cannot construct a valid SVal for this condition.
- // This means we cannot properly reason about it.
- return;
- }
-
- ProgramStateRef stateGT, stateLE;
- std::tie(stateLE, stateGT) = CM.assumeDual(state, *lessThanEqToOne);
-
- // Is it possible for the value to be greater than one?
- if (stateGT) {
- // It is possible for the value to be greater than one. We only
- // want to emit a warning, however, if that value is fully constrained.
- // If it is possible for the value to be <= 1, then essentially the
- // value is underconstrained and there is nothing left to be done.
- if (!stateLE)
- emitReport(stateGT, C);
-
- // In either case, we are done.
- return;
- }
-
- // If we reach here, it must be the case that the value is constrained
- // to only be <= 1.
- assert(stateLE == state);
+ if (StOut)
+ emitReport(StOut, C);
}
void ento::registerBoolAssignmentChecker(CheckerManager &mgr) {
Index: include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
===================================================================
--- include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -76,11 +76,7 @@
}
bool haveSameType(QualType Ty1, QualType Ty2) {
- // FIXME: Remove the second disjunct when we support symbolic
- // truncation/extension.
- return (Context.getCanonicalType(Ty1) == Context.getCanonicalType(Ty2) ||
- (Ty1->isIntegralOrEnumerationType() &&
- Ty2->isIntegralOrEnumerationType()));
+ return (Context.getCanonicalType(Ty1) == Context.getCanonicalType(Ty2));
}
SVal evalCast(SVal val, QualType castTy, QualType originalType);
Index: include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
===================================================================
--- include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
@@ -234,7 +234,8 @@
}
inline const llvm::APSInt& getTruthValue(bool b, QualType T) {
- return getValue(b ? 1 : 0, Ctx.getTypeSize(T), false);
+ return getValue(b ? 1 : 0, Ctx.getTypeSize(T),
+ !T->isSignedIntegerOrEnumerationType());
}
inline const llvm::APSInt& getTruthValue(bool b) {
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
