summaryrefslogtreecommitdiffstats
path: root/lib/StaticAnalyzer
diff options
context:
space:
mode:
Diffstat (limited to 'lib/StaticAnalyzer')
-rw-r--r--lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp2
-rw-r--r--lib/StaticAnalyzer/Checkers/CStringChecker.cpp756
-rw-r--r--lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp2
-rw-r--r--lib/StaticAnalyzer/Checkers/Checkers.td8
-rw-r--r--lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp6
-rw-r--r--lib/StaticAnalyzer/Checkers/IteratorsChecker.cpp27
-rw-r--r--lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp79
-rw-r--r--lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp20
-rw-r--r--lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp110
-rw-r--r--lib/StaticAnalyzer/Core/CFRefCount.cpp142
-rw-r--r--lib/StaticAnalyzer/Core/Environment.cpp3
-rw-r--r--lib/StaticAnalyzer/Core/ExprEngine.cpp78
-rw-r--r--lib/StaticAnalyzer/Core/RegionStore.cpp7
-rw-r--r--lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp36
14 files changed, 901 insertions, 375 deletions
diff --git a/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp b/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
index 235b400..9fc8163 100644
--- a/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
+++ b/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
@@ -606,7 +606,7 @@ void VariadicMethodTypeChecker::checkPreObjCMessage(ObjCMessage msg,
continue;
// Ignore CF references, which can be toll-free bridged.
- if (cocoa::isCFObjectRef(ArgTy))
+ if (coreFoundation::isCFObjectRef(ArgTy))
continue;
// Generate only one error node to use for all bug reports.
diff --git a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index f2f5c1e..c5dac5d 100644
--- a/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -30,8 +30,11 @@ class CStringChecker : public Checker< eval::Call,
check::DeadSymbols,
check::RegionChanges
> {
- mutable llvm::OwningPtr<BugType> BT_Null, BT_Bounds, BT_BoundsWrite,
- BT_Overlap, BT_NotCString;
+ mutable llvm::OwningPtr<BugType> BT_Null, BT_Bounds,
+ BT_Overlap, BT_NotCString,
+ BT_AdditionOverflow;
+ mutable const char *CurrentFunctionDescription;
+
public:
static void *getTag() { static int tag; return &tag; }
@@ -91,9 +94,11 @@ public:
const MemRegion *MR, SVal strLength);
static SVal getCStringLengthForRegion(CheckerContext &C,
const GRState *&state,
- const Expr *Ex, const MemRegion *MR);
+ const Expr *Ex, const MemRegion *MR,
+ bool hypothetical);
SVal getCStringLength(CheckerContext &C, const GRState *&state,
- const Expr *Ex, SVal Buf) const;
+ const Expr *Ex, SVal Buf,
+ bool hypothetical = false) const;
const StringLiteral *getCStringLiteral(CheckerContext &C,
const GRState *&state,
@@ -112,17 +117,29 @@ public:
const Expr *S, SVal l) const;
const GRState *CheckLocation(CheckerContext &C, const GRState *state,
const Expr *S, SVal l,
- bool IsDestination = false) const;
+ const char *message = NULL) const;
const GRState *CheckBufferAccess(CheckerContext &C, const GRState *state,
const Expr *Size,
const Expr *FirstBuf,
- const Expr *SecondBuf = NULL,
- bool FirstIsDestination = false) const;
+ const Expr *SecondBuf,
+ const char *firstMessage = NULL,
+ const char *secondMessage = NULL,
+ bool WarnAboutSize = false) const;
+ const GRState *CheckBufferAccess(CheckerContext &C, const GRState *state,
+ const Expr *Size, const Expr *Buf,
+ const char *message = NULL,
+ bool WarnAboutSize = false) const {
+ // This is a convenience override.
+ return CheckBufferAccess(C, state, Size, Buf, NULL, message, NULL,
+ WarnAboutSize);
+ }
const GRState *CheckOverlap(CheckerContext &C, const GRState *state,
const Expr *Size, const Expr *First,
const Expr *Second) const;
void emitOverlapBug(CheckerContext &C, const GRState *state,
const Stmt *First, const Stmt *Second) const;
+ const GRState *checkAdditionOverflow(CheckerContext &C, const GRState *state,
+ NonLoc left, NonLoc right) const;
};
class CStringLength {
@@ -176,10 +193,14 @@ const GRState *CStringChecker::checkNonNull(CheckerContext &C,
BT_Null.reset(new BuiltinBug("API",
"Null pointer argument in call to byte string function"));
+ llvm::SmallString<80> buf;
+ llvm::raw_svector_ostream os(buf);
+ assert(CurrentFunctionDescription);
+ os << "Null pointer argument in call to " << CurrentFunctionDescription;
+
// Generate a report for this bug.
BuiltinBug *BT = static_cast<BuiltinBug*>(BT_Null.get());
- EnhancedBugReport *report = new EnhancedBugReport(*BT,
- BT->getDescription(), N);
+ EnhancedBugReport *report = new EnhancedBugReport(*BT, os.str(), N);
report->addRange(S->getSourceRange());
report->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue, S);
@@ -196,7 +217,7 @@ const GRState *CStringChecker::checkNonNull(CheckerContext &C,
const GRState *CStringChecker::CheckLocation(CheckerContext &C,
const GRState *state,
const Expr *S, SVal l,
- bool IsDestination) const {
+ const char *warningMsg) const {
// If a previous check has failed, propagate the failure.
if (!state)
return NULL;
@@ -216,7 +237,8 @@ const GRState *CStringChecker::CheckLocation(CheckerContext &C,
// Get the size of the array.
const SubRegion *superReg = cast<SubRegion>(ER->getSuperRegion());
SValBuilder &svalBuilder = C.getSValBuilder();
- SVal Extent = svalBuilder.convertToArrayIndex(superReg->getExtent(svalBuilder));
+ SVal Extent =
+ svalBuilder.convertToArrayIndex(superReg->getExtent(svalBuilder));
DefinedOrUnknownSVal Size = cast<DefinedOrUnknownSVal>(Extent);
// Get the index of the accessed element.
@@ -229,28 +251,32 @@ const GRState *CStringChecker::CheckLocation(CheckerContext &C,
if (!N)
return NULL;
- BuiltinBug *BT;
- if (IsDestination) {
- if (!BT_BoundsWrite) {
- BT_BoundsWrite.reset(new BuiltinBug("Out-of-bound array access",
- "Byte string function overflows destination buffer"));
- }
- BT = static_cast<BuiltinBug*>(BT_BoundsWrite.get());
+ if (!BT_Bounds) {
+ BT_Bounds.reset(new BuiltinBug("Out-of-bound array access",
+ "Byte string function accesses out-of-bound array element"));
+ }
+ BuiltinBug *BT = static_cast<BuiltinBug*>(BT_Bounds.get());
+
+ // Generate a report for this bug.
+ RangedBugReport *report;
+ if (warningMsg) {
+ report = new RangedBugReport(*BT, warningMsg, N);
} else {
- if (!BT_Bounds) {
- BT_Bounds.reset(new BuiltinBug("Out-of-bound array access",
- "Byte string function accesses out-of-bound array element"));
- }
- BT = static_cast<BuiltinBug*>(BT_Bounds.get());
+ assert(CurrentFunctionDescription);
+ assert(CurrentFunctionDescription[0] != '\0');
+
+ llvm::SmallString<80> buf;
+ llvm::raw_svector_ostream os(buf);
+ os << (char)toupper(CurrentFunctionDescription[0])
+ << &CurrentFunctionDescription[1]
+ << " accesses out-of-bound array element";
+ report = new RangedBugReport(*BT, os.str(), N);
}
// FIXME: It would be nice to eventually make this diagnostic more clear,
// e.g., by referencing the original declaration or by saying *why* this
// reference is outside the range.
- // Generate a report for this bug.
- RangedBugReport *report = new RangedBugReport(*BT, BT->getDescription(), N);
-
report->addRange(S->getSourceRange());
C.EmitReport(report);
return NULL;
@@ -266,13 +292,15 @@ const GRState *CStringChecker::CheckBufferAccess(CheckerContext &C,
const Expr *Size,
const Expr *FirstBuf,
const Expr *SecondBuf,
- bool FirstIsDestination) const {
+ const char *firstMessage,
+ const char *secondMessage,
+ bool WarnAboutSize) const {
// If a previous check has failed, propagate the failure.
if (!state)
return NULL;
SValBuilder &svalBuilder = C.getSValBuilder();
- ASTContext &Ctx = C.getASTContext();
+ ASTContext &Ctx = svalBuilder.getContext();
QualType sizeTy = Size->getType();
QualType PtrTy = Ctx.getPointerType(Ctx.CharTy);
@@ -284,6 +312,8 @@ const GRState *CStringChecker::CheckBufferAccess(CheckerContext &C,
return NULL;
// Get the access length and make sure it is known.
+ // FIXME: This assumes the caller has already checked that the access length
+ // is positive. And that it's unsigned.
SVal LengthVal = state->getSVal(Size);
NonLoc *Length = dyn_cast<NonLoc>(&LengthVal);
if (!Length)
@@ -297,9 +327,11 @@ const GRState *CStringChecker::CheckBufferAccess(CheckerContext &C,
// Check that the first buffer is sufficiently long.
SVal BufStart = svalBuilder.evalCast(BufVal, PtrTy, FirstBuf->getType());
if (Loc *BufLoc = dyn_cast<Loc>(&BufStart)) {
+ const Expr *warningExpr = (WarnAboutSize ? Size : FirstBuf);
+
SVal BufEnd = svalBuilder.evalBinOpLN(state, BO_Add, *BufLoc,
LastOffset, PtrTy);
- state = CheckLocation(C, state, FirstBuf, BufEnd, FirstIsDestination);
+ state = CheckLocation(C, state, warningExpr, BufEnd, firstMessage);
// If the buffer isn't large enough, abort.
if (!state)
@@ -315,9 +347,11 @@ const GRState *CStringChecker::CheckBufferAccess(CheckerContext &C,
BufStart = svalBuilder.evalCast(BufVal, PtrTy, SecondBuf->getType());
if (Loc *BufLoc = dyn_cast<Loc>(&BufStart)) {
+ const Expr *warningExpr = (WarnAboutSize ? Size : SecondBuf);
+
SVal BufEnd = svalBuilder.evalBinOpLN(state, BO_Add, *BufLoc,
LastOffset, PtrTy);
- state = CheckLocation(C, state, SecondBuf, BufEnd);
+ state = CheckLocation(C, state, warningExpr, BufEnd, secondMessage);
}
}
@@ -368,8 +402,7 @@ const GRState *CStringChecker::CheckOverlap(CheckerContext &C,
state = stateFalse;
// Which value comes first?
- ASTContext &Ctx = svalBuilder.getContext();
- QualType cmpTy = Ctx.IntTy;
+ QualType cmpTy = svalBuilder.getConditionType();
SVal reverse = svalBuilder.evalBinOpLL(state, BO_GT,
*firstLoc, *secondLoc, cmpTy);
DefinedOrUnknownSVal *reverseTest = dyn_cast<DefinedOrUnknownSVal>(&reverse);
@@ -402,8 +435,10 @@ const GRState *CStringChecker::CheckOverlap(CheckerContext &C,
// Convert the first buffer's start address to char*.
// Bail out if the cast fails.
+ ASTContext &Ctx = svalBuilder.getContext();
QualType CharPtrTy = Ctx.getPointerType(Ctx.CharTy);
- SVal FirstStart = svalBuilder.evalCast(*firstLoc, CharPtrTy, First->getType());
+ SVal FirstStart = svalBuilder.evalCast(*firstLoc, CharPtrTy,
+ First->getType());
Loc *FirstStartLoc = dyn_cast<Loc>(&FirstStart);
if (!FirstStartLoc)
return state;
@@ -454,12 +489,78 @@ void CStringChecker::emitOverlapBug(CheckerContext &C, const GRState *state,
C.EmitReport(report);
}
+const GRState *CStringChecker::checkAdditionOverflow(CheckerContext &C,
+ const GRState *state,
+ NonLoc left,
+ NonLoc right) const {
+ // If a previous check has failed, propagate the failure.
+ if (!state)
+ return NULL;
+
+ SValBuilder &svalBuilder = C.getSValBuilder();
+ BasicValueFactory &BVF = svalBuilder.getBasicValueFactory();
+
+ QualType sizeTy = svalBuilder.getContext().getSizeType();
+ const llvm::APSInt &maxValInt = BVF.getMaxValue(sizeTy);
+ NonLoc maxVal = svalBuilder.makeIntVal(maxValInt);
+
+ SVal maxMinusRight = svalBuilder.evalBinOpNN(state, BO_Sub, maxVal, right,
+ sizeTy);
+
+ if (maxMinusRight.isUnknownOrUndef()) {
+ // Try switching the operands. (The order of these two assignments is
+ // important!)
+ maxMinusRight = svalBuilder.evalBinOpNN(state, BO_Sub, maxVal, left,
+ sizeTy);
+ left = right;
+ }
+
+ if (NonLoc *maxMinusRightNL = dyn_cast<NonLoc>(&maxMinusRight)) {
+ QualType cmpTy = svalBuilder.getConditionType();
+ // If left > max - right, we have an overflow.
+ SVal willOverflow = svalBuilder.evalBinOpNN(state, BO_GT, left,
+ *maxMinusRightNL, cmpTy);
+
+ const GRState *stateOverflow, *stateOkay;
+ llvm::tie(stateOverflow, stateOkay) =
+ state->assume(cast<DefinedOrUnknownSVal>(willOverflow));
+
+ if (stateOverflow && !stateOkay) {
+ // We have an overflow. Emit a bug report.
+ ExplodedNode *N = C.generateSink(stateOverflow);
+ if (!N)
+ return NULL;
+
+ if (!BT_AdditionOverflow)
+ BT_AdditionOverflow.reset(new BuiltinBug("API",
+ "Sum of expressions causes overflow"));
+
+ // This isn't a great error message, but this should never occur in real
+ // code anyway -- you'd have to create a buffer longer than a size_t can
+ // represent, which is sort of a contradiction.
+ const char *warning =
+ "This expression will create a string whose length is too big to "
+ "be represented as a size_t";
+
+ // Generate a report for this bug.
+ BugReport *report = new BugReport(*BT_AdditionOverflow, warning, N);
+ C.EmitReport(report);
+
+ return NULL;
+ }
+
+ // From now on, assume an overflow didn't occur.
+ assert(stateOkay);
+ state = stateOkay;
+ }
+
+ return state;
+}
+
const GRState *CStringChecker::setCStringLength(const GRState *state,
const MemRegion *MR,
SVal strLength) {
assert(!strLength.isUndef() && "Attempt to set an undefined string length");
- if (strLength.isUnknown())
- return state;
MR = MR->StripCasts();
@@ -474,7 +575,8 @@ const GRState *CStringChecker::setCStringLength(const GRState *state,
case MemRegion::VarRegionKind:
case MemRegion::FieldRegionKind:
case MemRegion::ObjCIvarRegionKind:
- return state->set<CStringLength>(MR, strLength);
+ // These are the types we can currently track string lengths for.
+ break;
case MemRegion::ElementRegionKind:
// FIXME: Handle element regions by upper-bounding the parent region's
@@ -488,16 +590,24 @@ const GRState *CStringChecker::setCStringLength(const GRState *state,
// warning for things like strcpy((char[]){'a', 0}, "b");
return state;
}
+
+ if (strLength.isUnknown())
+ return state->remove<CStringLength>(MR);
+
+ return state->set<CStringLength>(MR, strLength);
}
SVal CStringChecker::getCStringLengthForRegion(CheckerContext &C,
const GRState *&state,
const Expr *Ex,
- const MemRegion *MR) {
- // If there's a recorded length, go ahead and return it.
- const SVal *Recorded = state->get<CStringLength>(MR);
- if (Recorded)
- return *Recorded;
+ const MemRegion *MR,
+ bool hypothetical) {
+ if (!hypothetical) {
+ // If there's a recorded length, go ahead and return it.
+ const SVal *Recorded = state->get<CStringLength>(MR);
+ if (Recorded)
+ return *Recorded;
+ }
// Otherwise, get a new symbol and update the state.
unsigned Count = C.getNodeBuilder().getCurrentBlockCount();
@@ -505,12 +615,16 @@ SVal CStringChecker::getCStringLengthForRegion(CheckerContext &C,
QualType sizeTy = svalBuilder.getContext().getSizeType();
SVal strLength = svalBuilder.getMetadataSymbolVal(CStringChecker::getTag(),
MR, Ex, sizeTy, Count);
- state = state->set<CStringLength>(MR, strLength);
+
+ if (!hypothetical)
+ state = state->set<CStringLength>(MR, strLength);
+
return strLength;
}
SVal CStringChecker::getCStringLength(CheckerContext &C, const GRState *&state,
- const Expr *Ex, SVal Buf) const {
+ const Expr *Ex, SVal Buf,
+ bool hypothetical) const {
const MemRegion *MR = Buf.getAsRegion();
if (!MR) {
// If we can't get a region, see if it's something we /know/ isn't a
@@ -524,8 +638,9 @@ SVal CStringChecker::getCStringLength(CheckerContext &C, const GRState *&state,
llvm::SmallString<120> buf;
llvm::raw_svector_ostream os(buf);
- os << "Argument to byte string function is the address of the label '"
- << Label->getLabel()->getName()
+ assert(CurrentFunctionDescription);
+ os << "Argument to " << CurrentFunctionDescription
+ << " is the address of the label '" << Label->getLabel()->getName()
<< "', which is not a null-terminated string";
// Generate a report for this bug.
@@ -561,7 +676,7 @@ SVal CStringChecker::getCStringLength(CheckerContext &C, const GRState *&state,
case MemRegion::VarRegionKind:
case MemRegion::FieldRegionKind:
case MemRegion::ObjCIvarRegionKind:
- return getCStringLengthForRegion(C, state, Ex, MR);
+ return getCStringLengthForRegion(C, state, Ex, MR, hypothetical);
case MemRegion::CompoundLiteralRegionKind:
// FIXME: Can we track this? Is it necessary?
return UnknownVal();
@@ -581,7 +696,8 @@ SVal CStringChecker::getCStringLength(CheckerContext &C, const GRState *&state,
llvm::SmallString<120> buf;
llvm::raw_svector_ostream os(buf);
- os << "Argument to byte string function is ";
+ assert(CurrentFunctionDescription);
+ os << "Argument to " << CurrentFunctionDescription << " is ";
if (SummarizeRegion(os, C.getASTContext(), MR))
os << ", which is not a null-terminated string";
@@ -700,12 +816,15 @@ void CStringChecker::evalCopyCommon(CheckerContext &C,
const Expr *Size, const Expr *Dest,
const Expr *Source, bool Restricted,
bool IsMempcpy) const {
+ CurrentFunctionDescription = "memory copy function";
+
// See if the size argument is zero.
SVal sizeVal = state->getSVal(Size);
QualType sizeTy = Size->getType();
const GRState *stateZeroSize, *stateNonZeroSize;
- llvm::tie(stateZeroSize, stateNonZeroSize) = assumeZero(C, state, sizeVal, sizeTy);
+ llvm::tie(stateZeroSize, stateNonZeroSize) =
+ assumeZero(C, state, sizeVal, sizeTy);
// Get the value of the Dest.
SVal destVal = state->getSVal(Dest);
@@ -737,8 +856,10 @@ void CStringChecker::evalCopyCommon(CheckerContext &C,
return;
// Ensure the accesses are valid and that the buffers do not overlap.
+ const char * const writeWarning =
+ "Memory copy function overflows destination buffer";
state = CheckBufferAccess(C, state, Size, Dest, Source,
- /* FirstIsDst = */ true);
+ writeWarning, /* sourceWarning = */ NULL);
if (Restricted)
state = CheckOverlap(C, state, Size, Dest, Source);
@@ -824,6 +945,8 @@ void CStringChecker::evalBcopy(CheckerContext &C, const CallExpr *CE) const {
void CStringChecker::evalMemcmp(CheckerContext &C, const CallExpr *CE) const {
// int memcmp(const void *s1, const void *s2, size_t n);
+ CurrentFunctionDescription = "memory comparison function";
+
const Expr *Left = CE->getArg(0);
const Expr *Right = CE->getArg(1);
const Expr *Size = CE->getArg(2);
@@ -861,7 +984,7 @@ void CStringChecker::evalMemcmp(CheckerContext &C, const CallExpr *CE) const {
const GRState *StSameBuf, *StNotSameBuf;
llvm::tie(StSameBuf, StNotSameBuf) = state->assume(SameBuf);
- // If the two arguments might be the same buffer, we know the result is zero,
+ // If the two arguments might be the same buffer, we know the result is 0,
// and we only need to check one size.
if (StSameBuf) {
state = StSameBuf;
@@ -902,58 +1025,126 @@ void CStringChecker::evalstrnLength(CheckerContext &C,
void CStringChecker::evalstrLengthCommon(CheckerContext &C, const CallExpr *CE,
bool IsStrnlen) const {
+ CurrentFunctionDescription = "string length function";
const GRState *state = C.getState();
+
+ if (IsStrnlen) {
+ const Expr *maxlenExpr = CE->getArg(1);
+ SVal maxlenVal = state->getSVal(maxlenExpr);
+
+ const GRState *stateZeroSize, *stateNonZeroSize;
+ llvm::tie(stateZeroSize, stateNonZeroSize) =
+ assumeZero(C, state, maxlenVal, maxlenExpr->getType());
+
+ // If the size can be zero, the result will be 0 in that case, and we don't
+ // have to check the string itself.
+ if (stateZeroSize) {
+ SVal zero = C.getSValBuilder().makeZeroVal(CE->getType());
+ stateZeroSize = stateZeroSize->BindExpr(CE, zero);
+ C.addTransition(stateZeroSize);
+ }
+
+ // If the size is GUARANTEED to be zero, we're done!
+ if (!stateNonZeroSize)
+ return;
+
+ // Otherwise, record the assumption that the size is nonzero.
+ state = stateNonZeroSize;
+ }
+
+ // Check that the string argument is non-null.
const Expr *Arg = CE->getArg(0);
SVal ArgVal = state->getSVal(Arg);
- // Check that the argument is non-null.
state = checkNonNull(C, state, Arg, ArgVal);
- if (state) {
- SVal strLength = getCStringLength(C, state, Arg, ArgVal);
+ if (!state)
+ return;
- // If the argument isn't a valid C string, there's no valid state to
- // transition to.
- if (strLength.isUndef())
- return;
+ SVal strLength = getCStringLength(C, state, Arg, ArgVal);
- // If the check is for strnlen() then bind the return value to no more than
- // the maxlen value.
- if (IsStrnlen) {
- const Expr *maxlenExpr = CE->getArg(1);
- SVal maxlenVal = state->getSVal(maxlenExpr);
-
- NonLoc *strLengthNL = dyn_cast<NonLoc>(&strLength);
- NonLoc *maxlenValNL = dyn_cast<NonLoc>(&maxlenVal);
+ // If the argument isn't a valid C string, there's no valid state to
+ // transition to.
+ if (strLength.isUndef())
+ return;
- QualType cmpTy = C.getSValBuilder().getContext().IntTy;
- const GRState *stateTrue, *stateFalse;
-
- // Check if the strLength is greater than or equal to the maxlen
- llvm::tie(stateTrue, stateFalse) =
+ DefinedOrUnknownSVal result = UnknownVal();
+
+ // If the check is for strnlen() then bind the return value to no more than
+ // the maxlen value.
+ if (IsStrnlen) {
+ QualType cmpTy = C.getSValBuilder().getConditionType();
+
+ // It's a little unfortunate to be getting this again,
+ // but it's not that expensive...
+ const Expr *maxlenExpr = CE->getArg(1);
+ SVal maxlenVal = state->getSVal(maxlenExpr);
+
+ NonLoc *strLengthNL = dyn_cast<NonLoc>(&strLength);
+ NonLoc *maxlenValNL = dyn_cast<NonLoc>(&maxlenVal);
+
+ if (strLengthNL && maxlenValNL) {
+ const GRState *stateStringTooLong, *stateStringNotTooLong;
+
+ // Check if the strLength is greater than the maxlen.
+ llvm::tie(stateStringTooLong, stateStringNotTooLong) =
state->assume(cast<DefinedOrUnknownSVal>
- (C.getSValBuilder().evalBinOpNN(state, BO_GE,
- *strLengthNL, *maxlenValNL,
+ (C.getSValBuilder().evalBinOpNN(state, BO_GT,
+ *strLengthNL,
+ *maxlenValNL,
cmpTy)));
- // If the strLength is greater than or equal to the maxlen, set strLength
- // to maxlen
- if (stateTrue && !stateFalse) {
- strLength = maxlenVal;
+ if (stateStringTooLong && !stateStringNotTooLong) {
+ // If the string is longer than maxlen, return maxlen.
+ result = *maxlenValNL;
+ } else if (stateStringNotTooLong && !stateStringTooLong) {
+ // If the string is shorter than maxlen, return its length.
+ result = *strLengthNL;
}
}
- // If getCStringLength couldn't figure out the length, conjure a return
- // value, so it can be used in constraints, at least.
- if (strLength.isUnknown()) {
+ if (result.isUnknown()) {
+ // If we don't have enough information for a comparison, there's
+ // no guarantee the full string length will actually be returned.
+ // All we know is the return value is the min of the string length
+ // and the limit. This is better than nothing.
unsigned Count = C.getNodeBuilder().getCurrentBlockCount();
- strLength = C.getSValBuilder().getConjuredSymbolVal(NULL, CE, Count);
+ result = C.getSValBuilder().getConjuredSymbolVal(NULL, CE, Count);
+ NonLoc *resultNL = cast<NonLoc>(&result);
+
+ if (strLengthNL) {
+ state = state->assume(cast<DefinedOrUnknownSVal>
+ (C.getSValBuilder().evalBinOpNN(state, BO_LE,
+ *resultNL,
+ *strLengthNL,
+ cmpTy)), true);
+ }
+
+ if (maxlenValNL) {
+ state = state->assume(cast<DefinedOrUnknownSVal>
+ (C.getSValBuilder().evalBinOpNN(state, BO_LE,
+ *resultNL,
+ *maxlenValNL,
+ cmpTy)), true);
+ }
}
- // Bind the return value.
- state = state->BindExpr(CE, strLength);
- C.addTransition(state);
+ } else {
+ // This is a plain strlen(), not strnlen().
+ result = cast<DefinedOrUnknownSVal>(strLength);
+
+ // If we don't know the length of the string, conjure a return
+ // value, so it can be used in constraints, at least.
+ if (result.isUnknown()) {
+ unsigned Count = C.getNodeBuilder().getCurrentBlockCount();
+ result = C.getSValBuilder().getConjuredSymbolVal(NULL, CE, Count);
+ }
}
+
+ // Bind the return value.
+ assert(!result.isUnknown() && "Should have conjured a value by now");
+ state = state->BindExpr(CE, result);
+ C.addTransition(state);
}
void CStringChecker::evalStrcpy(CheckerContext &C, const CallExpr *CE) const {
@@ -999,6 +1190,7 @@ void CStringChecker::evalStrncat(CheckerContext &C, const CallExpr *CE) const {
void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallExpr *CE,
bool returnEnd, bool isBounded,
bool isAppending) const {
+ CurrentFunctionDescription = "string copy function";
const GRState *state = C.getState();
// Check that the destination is non-null.
@@ -1023,76 +1215,240 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallExpr *CE,
if (strLength.isUndef())
return;
+ SValBuilder &svalBuilder = C.getSValBuilder();
+ QualType cmpTy = svalBuilder.getConditionType();
+ QualType sizeTy = svalBuilder.getContext().getSizeType();
+
+ // These two values allow checking two kinds of errors:
+ // - actual overflows caused by a source that doesn't fit in the destination
+ // - potential overflows caused by a bound that could exceed the destination
+ SVal amountCopied = UnknownVal();
+ SVal maxLastElementIndex = UnknownVal();
+ const char *boundWarning = NULL;
+
// If the function is strncpy, strncat, etc... it is bounded.
if (isBounded) {
// Get the max number of characters to copy.
const Expr *lenExpr = CE->getArg(2);
SVal lenVal = state->getSVal(lenExpr);
- // Cast the length to a NonLoc SVal. If it is not a NonLoc then give up.
- NonLoc *strLengthNL = dyn_cast<NonLoc>(&strLength);
- if (!strLengthNL)
- return;
+ // Protect against misdeclared strncpy().
+ lenVal = svalBuilder.evalCast(lenVal, sizeTy, lenExpr->getType());
- // Cast the max length to a NonLoc SVal. If it is not a NonLoc then give up.
+ NonLoc *strLengthNL = dyn_cast<NonLoc>(&strLength);
NonLoc *lenValNL = dyn_cast<NonLoc>(&lenVal);
- if (!lenValNL)
- return;
- QualType cmpTy = C.getSValBuilder().getContext().IntTy;
- const GRState *stateTrue, *stateFalse;
-
- // Check if the max number to copy is less than the length of the src.
- llvm::tie(stateTrue, stateFalse) =
- state->assume(cast<DefinedOrUnknownSVal>
- (C.getSValBuilder().evalBinOpNN(state, BO_GT,
- *strLengthNL, *lenValNL,
- cmpTy)));
-
- if (stateTrue) {
- // Max number to copy is less than the length of the src, so the actual
- // strLength copied is the max number arg.
- strLength = lenVal;
- }
+ // If we know both values, we might be able to figure out how much
+ // we're copying.
+ if (strLengthNL && lenValNL) {
+ const GRState *stateSourceTooLong, *stateSourceNotTooLong;
+
+ // Check if the max number to copy is less than the length of the src.
+ // If the bound is equal to the source length, strncpy won't null-
+ // terminate the result!
+ llvm::tie(stateSourceTooLong, stateSourceNotTooLong) =
+ state->assume(cast<DefinedOrUnknownSVal>
+ (svalBuilder.evalBinOpNN(state, BO_GE, *strLengthNL,
+ *lenValNL, cmpTy)));
+
+ if (stateSourceTooLong && !stateSourceNotTooLong) {
+ // Max number to copy is less than the length of the src, so the actual
+ // strLength copied is the max number arg.
+ state = stateSourceTooLong;
+ amountCopied = lenVal;
+
+ } else if (!stateSourceTooLong && stateSourceNotTooLong) {
+ // The source buffer entirely fits in the bound.
+ state = stateSourceNotTooLong;
+ amountCopied = strLength;
+ }
+ }
+
+ // We still want to know if the bound is known to be too large.
+ if (lenValNL) {
+ if (isAppending) {
+ // For strncat, the check is strlen(dst) + lenVal < sizeof(dst)
+
+ // Get the string length of the destination. If the destination is
+ // memory that can't have a string length, we shouldn't be copying
+ // into it anyway.
+ SVal dstStrLength = getCStringLength(C, state, Dst, DstVal);
+ if (dstStrLength.isUndef())
+ return;
+
+ if (NonLoc *dstStrLengthNL = dyn_cast<NonLoc>(&dstStrLength)) {
+ maxLastElementIndex = svalBuilder.evalBinOpNN(state, BO_Add,
+ *lenValNL,
+ *dstStrLengthNL,
+ sizeTy);
+ boundWarning = "Size argument is greater than the free space in the "
+ "destination buffer";
+ }
+
+ } else {
+ // For strncpy, this is just checking that lenVal <= sizeof(dst)
+ // (Yes, strncpy and strncat differ in how they treat termination.
+ // strncat ALWAYS terminates, but strncpy doesn't.)
+ NonLoc one = cast<NonLoc>(svalBuilder.makeIntVal(1, sizeTy));
+ maxLastElementIndex = svalBuilder.evalBinOpNN(state, BO_Sub, *lenValNL,
+ one, sizeTy);
+ boundWarning = "Size argument is greater than the length of the "
+ "destination buffer";
+ }
+ }
+
+ // If we couldn't pin down the copy length, at least bound it.
+ // FIXME: We should actually run this code path for append as well, but
+ // right now it creates problems with constraints (since we can end up
+ // trying to pass constraints from symbol to symbol).
+ if (amountCopied.isUnknown() && !isAppending) {
+ // Try to get a "hypothetical" string length symbol, which we can later
+ // set as a real value if that turns out to be the case.
+ amountCopied = getCStringLength(C, state, lenExpr, srcVal, true);
+ assert(!amountCopied.isUndef());
+
+ if (NonLoc *amountCopiedNL = dyn_cast<NonLoc>(&amountCopied)) {
+ if (lenValNL) {
+ // amountCopied <= lenVal
+ SVal copiedLessThanBound = svalBuilder.evalBinOpNN(state, BO_LE,
+ *amountCopiedNL,
+ *lenValNL,
+ cmpTy);
+ state = state->assume(cast<DefinedOrUnknownSVal>(copiedLessThanBound),
+ true);
+ if (!state)
+ return;
+ }
+
+ if (strLengthNL) {
+ // amountCopied <= strlen(source)
+ SVal copiedLessThanSrc = svalBuilder.evalBinOpNN(state, BO_LE,
+ *amountCopiedNL,
+ *strLengthNL,
+ cmpTy);
+ state = state->assume(cast<DefinedOrUnknownSVal>(copiedLessThanSrc),
+ true);
+ if (!state)
+ return;
+ }
+ }
+ }
+
+ } else {
+ // The function isn't bounded. The amount copied should match the length
+ // of the source buffer.
+ amountCopied = strLength;
}
+ assert(state);
+
+ // This represents the number of characters copied into the destination
+ // buffer. (It may not actually be the strlen if the destination buffer
+ // is not terminated.)
+ SVal finalStrLength = UnknownVal();
+
// If this is an appending function (strcat, strncat...) then set the
// string length to strlen(src) + strlen(dst) since the buffer will
// ultimately contain both.
if (isAppending) {
- // Get the string length of the destination, or give up.
+ // Get the string length of the destination. If the destination is memory
+ // that can't have a string length, we shouldn't be copying into it anyway.
SVal dstStrLength = getCStringLength(C, state, Dst, DstVal);
if (dstStrLength.isUndef())
return;
- NonLoc *srcStrLengthNL = dyn_cast<NonLoc>(&strLength);
+ NonLoc *srcStrLengthNL = dyn_cast<NonLoc>(&amountCopied);
NonLoc *dstStrLengthNL = dyn_cast<NonLoc>(&dstStrLength);
- // If src or dst cast to NonLoc is NULL, give up.
- if ((!srcStrLengthNL) || (!dstStrLengthNL))
- return;
+ // If we know both string lengths, we might know the final string length.
+ if (srcStrLengthNL && dstStrLengthNL) {
+ // Make sure the two lengths together don't overflow a size_t.
+ state = checkAdditionOverflow(C, state, *srcStrLengthNL, *dstStrLengthNL);
+ if (!state)
+ return;
+
+ finalStrLength = svalBuilder.evalBinOpNN(state, BO_Add, *srcStrLengthNL,
+ *dstStrLengthNL, sizeTy);
+ }
- QualType addTy = C.getSValBuilder().getContext().getSizeType();
+ // If we couldn't get a single value for the final string length,
+ // we can at least bound it by the individual lengths.
+ if (finalStrLength.isUnknown()) {
+ // Try to get a "hypothetical" string length symbol, which we can later
+ // set as a real value if that turns out to be the case.
+ finalStrLength = getCStringLength(C, state, CE, DstVal, true);
+ assert(!finalStrLength.isUndef());
+
+ if (NonLoc *finalStrLengthNL = dyn_cast<NonLoc>(&finalStrLength)) {
+ if (srcStrLengthNL) {
+ // finalStrLength >= srcStrLength
+ SVal sourceInResult = svalBuilder.evalBinOpNN(state, BO_GE,
+ *finalStrLengthNL,
+ *srcStrLengthNL,
+ cmpTy);
+ state = state->assume(cast<DefinedOrUnknownSVal>(sourceInResult),
+ true);
+ if (!state)
+ return;
+ }
+
+ if (dstStrLengthNL) {
+ // finalStrLength >= dstStrLength
+ SVal destInResult = svalBuilder.evalBinOpNN(state, BO_GE,
+ *finalStrLengthNL,
+ *dstStrLengthNL,
+ cmpTy);
+ state = state->assume(cast<DefinedOrUnknownSVal>(destInResult),
+ true);
+ if (!state)
+ return;
+ }
+ }
+ }
- strLength = C.getSValBuilder().evalBinOpNN(state, BO_Add,
- *srcStrLengthNL, *dstStrLengthNL,
- addTy);
+ } else {
+ // Otherwise, this is a copy-over function (strcpy, strncpy, ...), and
+ // the final string length will match the input string length.
+ finalStrLength = amountCopied;
}
+ // The final result of the function will either be a pointer past the last
+ // copied element, or a pointer to the start of the destination buffer.
SVal Result = (returnEnd ? UnknownVal() : DstVal);
+ assert(state);
+
// If the destination is a MemRegion, try to check for a buffer overflow and
// record the new string length.
if (loc::MemRegionVal *dstRegVal = dyn_cast<loc::MemRegionVal>(&DstVal)) {
- // If the length is known, we can check for an overflow.
- if (NonLoc *knownStrLength = dyn_cast<NonLoc>(&strLength)) {
- SVal lastElement =
- C.getSValBuilder().evalBinOpLN(state, BO_Add, *dstRegVal,
- *knownStrLength, Dst->getType());
+ QualType ptrTy = Dst->getType();
+
+ // If we have an exact value on a bounded copy, use that to check for
+ // overflows, rather than our estimate about how much is actually copied.
+ if (boundWarning) {
+ if (NonLoc *maxLastNL = dyn_cast<NonLoc>(&maxLastElementIndex)) {
+ SVal maxLastElement = svalBuilder.evalBinOpLN(state, BO_Add, *dstRegVal,
+ *maxLastNL, ptrTy);
+ state = CheckLocation(C, state, CE->getArg(2), maxLastElement,
+ boundWarning);
+ if (!state)
+ return;
+ }
+ }
- state = CheckLocation(C, state, Dst, lastElement, /* IsDst = */ true);
- if (!state)
- return;
+ // Then, if the final length is known...
+ if (NonLoc *knownStrLength = dyn_cast<NonLoc>(&finalStrLength)) {
+ SVal lastElement = svalBuilder.evalBinOpLN(state, BO_Add, *dstRegVal,
+ *knownStrLength, ptrTy);
+
+ // ...and we haven't checked the bound, we'll check the actual copy.
+ if (!boundWarning) {
+ const char * const warningMsg =
+ "String copy function overflows destination buffer";
+ state = CheckLocation(C, state, Dst, lastElement, warningMsg);
+ if (!state)
+ return;
+ }
// If this is a stpcpy-style copy, the last element is the return value.
if (returnEnd)
@@ -1107,16 +1463,25 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallExpr *CE,
// string, but that's still an improvement over blank invalidation.
state = InvalidateBuffer(C, state, Dst, *dstRegVal);
- // Set the C string length of the destination.
- state = setCStringLength(state, dstRegVal->getRegion(), strLength);
+ // Set the C string length of the destination, if we know it.
+ if (isBounded && !isAppending) {
+ // strncpy is annoying in that it doesn't guarantee to null-terminate
+ // the result string. If the original string didn't fit entirely inside
+ // the bound (including the null-terminator), we don't know how long the
+ // result is.
+ if (amountCopied != strLength)
+ finalStrLength = UnknownVal();
+ }
+ state = setCStringLength(state, dstRegVal->getRegion(), finalStrLength);
}
+ assert(state);
+
// If this is a stpcpy-style copy, but we were unable to check for a buffer
// overflow, we still need a result. Conjure a return value.
if (returnEnd && Result.isUnknown()) {
- SValBuilder &svalBuilder = C.getSValBuilder();
unsigned Count = C.getNodeBuilder().getCurrentBlockCount();
- strLength = svalBuilder.getConjuredSymbolVal(NULL, CE, Count);
+ Result = svalBuilder.getConjuredSymbolVal(NULL, CE, Count);
}
// Set the return value.
@@ -1125,29 +1490,30 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallExpr *CE,
}
void CStringChecker::evalStrcmp(CheckerContext &C, const CallExpr *CE) const {
- //int strcmp(const char *restrict s1, const char *restrict s2);
+ //int strcmp(const char *s1, const char *s2);
evalStrcmpCommon(C, CE, /* isBounded = */ false, /* ignoreCase = */ false);
}
void CStringChecker::evalStrncmp(CheckerContext &C, const CallExpr *CE) const {
- //int strncmp(const char *restrict s1, const char *restrict s2, size_t n);
+ //int strncmp(const char *s1, const char *s2, size_t n);
evalStrcmpCommon(C, CE, /* isBounded = */ true, /* ignoreCase = */ false);
}
void CStringChecker::evalStrcasecmp(CheckerContext &C,
const CallExpr *CE) const {
- //int strcasecmp(const char *restrict s1, const char *restrict s2);
+ //int strcasecmp(const char *s1, const char *s2);
evalStrcmpCommon(C, CE, /* isBounded = */ false, /* ignoreCase = */ true);
}
void CStringChecker::evalStrncasecmp(CheckerContext &C,
const CallExpr *CE) const {
- //int strncasecmp(const char *restrict s1, const char *restrict s2, size_t n);
+ //int strncasecmp(const char *s1, const char *s2, size_t n);
evalStrcmpCommon(C, CE, /* isBounded = */ true, /* ignoreCase = */ true);
}
void CStringChecker::evalStrcmpCommon(CheckerContext &C, const CallExpr *CE,
bool isBounded, bool ignoreCase) const {
+ CurrentFunctionDescription = "string comparison function";
const GRState *state = C.getState();
// Check that the first string is non-null
@@ -1174,52 +1540,96 @@ void CStringChecker::evalStrcmpCommon(CheckerContext &C, const CallExpr *CE,
if (s2Length.isUndef())
return;
- // Get the string literal of the first string.
- const StringLiteral *s1StrLiteral = getCStringLiteral(C, state, s1, s1Val);
- if (!s1StrLiteral)
- return;
- llvm::StringRef s1StrRef = s1StrLiteral->getString();
+ // If we know the two buffers are the same, we know the result is 0.
+ // First, get the two buffers' addresses. Another checker will have already
+ // made sure they're not undefined.
+ DefinedOrUnknownSVal LV = cast<DefinedOrUnknownSVal>(s1Val);
+ DefinedOrUnknownSVal RV = cast<DefinedOrUnknownSVal>(s2Val);
+
+ // See if they are the same.
+ SValBuilder &svalBuilder = C.getSValBuilder();
+ DefinedOrUnknownSVal SameBuf = svalBuilder.evalEQ(state, LV, RV);
+ const GRState *StSameBuf, *StNotSameBuf;
+ llvm::tie(StSameBuf, StNotSameBuf) = state->assume(SameBuf);
+
+ // If the two arguments might be the same buffer, we know the result is 0,
+ // and we only need to check one size.
+ if (StSameBuf) {
+ StSameBuf = StSameBuf->BindExpr(CE, svalBuilder.makeZeroVal(CE->getType()));
+ C.addTransition(StSameBuf);
+
+ // If the two arguments are GUARANTEED to be the same, we're done!
+ if (!StNotSameBuf)
+ return;
+ }
+
+ assert(StNotSameBuf);
+ state = StNotSameBuf;
+
+ // At this point we can go about comparing the two buffers.
+ // For now, we only do this if they're both known string literals.
- // Get the string literal of the second string.
+ // Attempt to extract string literals from both expressions.
+ const StringLiteral *s1StrLiteral = getCStringLiteral(C, state, s1, s1Val);
const StringLiteral *s2StrLiteral = getCStringLiteral(C, state, s2, s2Val);
- if (!s2StrLiteral)
- return;
- llvm::StringRef s2StrRef = s2StrLiteral->getString();
+ bool canComputeResult = false;
+
+ if (s1StrLiteral && s2StrLiteral) {
+ llvm::StringRef s1StrRef = s1StrLiteral->getString();
+ llvm::StringRef s2StrRef = s2StrLiteral->getString();
+
+ if (isBounded) {
+ // Get the max number of characters to compare.
+ const Expr *lenExpr = CE->getArg(2);
+ SVal lenVal = state->getSVal(lenExpr);
+
+ // If the length is known, we can get the right substrings.
+ if (const llvm::APSInt *len = svalBuilder.getKnownValue(state, lenVal)) {
+ // Create substrings of each to compare the prefix.
+ s1StrRef = s1StrRef.substr(0, (size_t)len->getZExtValue());
+ s2StrRef = s2StrRef.substr(0, (size_t)len->getZExtValue());
+ canComputeResult = true;
+ }
+ } else {
+ // This is a normal, unbounded strcmp.
+ canComputeResult = true;
+ }
- int result;
- if (isBounded) {
- // Get the max number of characters to compare.
- const Expr *lenExpr = CE->getArg(2);
- SVal lenVal = state->getSVal(lenExpr);
+ if (canComputeResult) {
+ // Real strcmp stops at null characters.
+ size_t s1Term = s1StrRef.find('\0');
+ if (s1Term != llvm::StringRef::npos)
+ s1StrRef = s1StrRef.substr(0, s1Term);
- // Dynamically cast the length to a ConcreteInt. If it is not a ConcreteInt
- // then give up, otherwise get the value and use it as the bounds.
- nonloc::ConcreteInt *CI = dyn_cast<nonloc::ConcreteInt>(&lenVal);
- if (!CI)
- return;
- llvm::APSInt lenInt(CI->getValue());
+ size_t s2Term = s2StrRef.find('\0');
+ if (s2Term != llvm::StringRef::npos)
+ s2StrRef = s2StrRef.substr(0, s2Term);
+
+ // Use StringRef's comparison methods to compute the actual result.
+ int result;
- // Create substrings of each to compare the prefix.
- s1StrRef = s1StrRef.substr(0, (size_t)lenInt.getLimitedValue());
- s2StrRef = s2StrRef.substr(0, (size_t)lenInt.getLimitedValue());
+ if (ignoreCase) {
+ // Compare string 1 to string 2 the same way strcasecmp() does.
+ result = s1StrRef.compare_lower(s2StrRef);
+ } else {
+ // Compare string 1 to string 2 the same way strcmp() does.
+ result = s1StrRef.compare(s2StrRef);
+ }
+
+ // Build the SVal of the comparison and bind the return value.
+ SVal resultVal = svalBuilder.makeIntVal(result, CE->getType());
+ state = state->BindExpr(CE, resultVal);
+ }
}
- if (ignoreCase) {
- // Compare string 1 to string 2 the same way strcasecmp() does.
- result = s1StrRef.compare_lower(s2StrRef);
- } else {
- // Compare string 1 to string 2 the same way strcmp() does.
- result = s1StrRef.compare(s2StrRef);
+ if (!canComputeResult) {
+ // Conjure a symbolic value. It's the best we can do.
+ unsigned Count = C.getNodeBuilder().getCurrentBlockCount();
+ SVal resultVal = svalBuilder.getConjuredSymbolVal(NULL, CE, Count);
+ state = state->BindExpr(CE, resultVal);
}
-
- // Build the SVal of the comparison to bind the return value.
- SValBuilder &svalBuilder = C.getSValBuilder();
- QualType intTy = svalBuilder.getContext().IntTy;
- SVal resultVal = svalBuilder.makeIntVal(result, intTy);
- // Bind the return value of the expression.
- // Set the return value.
- state = state->BindExpr(CE, resultVal);
+ // Record this as a possible path.
C.addTransition(state);
}
@@ -1251,7 +1661,7 @@ bool CStringChecker::evalCall(const CallExpr *CE, CheckerContext &C) const {
.Cases("memcmp", "bcmp", &CStringChecker::evalMemcmp)
.Cases("memmove", "__memmove_chk", &CStringChecker::evalMemmove)
.Cases("strcpy", "__strcpy_chk", &CStringChecker::evalStrcpy)
- //.Cases("strncpy", "__strncpy_chk", &CStringChecker::evalStrncpy)
+ .Cases("strncpy", "__strncpy_chk", &CStringChecker::evalStrncpy)
.Cases("stpcpy", "__stpcpy_chk", &CStringChecker::evalStpcpy)
.Cases("strcat", "__strcat_chk", &CStringChecker::evalStrcat)
.Cases("strncat", "__strncat_chk", &CStringChecker::evalStrncat)
@@ -1268,6 +1678,10 @@ bool CStringChecker::evalCall(const CallExpr *CE, CheckerContext &C) const {
if (!evalFunction)
return false;
+ // Make sure each function sets its own description.
+ // (But don't bother in a release build.)
+ assert(!(CurrentFunctionDescription = NULL));
+
// Check and evaluate the call.
(this->*evalFunction)(C, CE);
return true;
@@ -1373,8 +1787,10 @@ void CStringChecker::checkLiveSymbols(const GRState *state,
for (CStringLength::EntryMap::iterator I = Entries.begin(), E = Entries.end();
I != E; ++I) {
SVal Len = I.getData();
- if (SymbolRef Sym = Len.getAsSymbol())
- SR.markInUse(Sym);
+
+ for (SVal::symbol_iterator si = Len.symbol_begin(), se = Len.symbol_end();
+ si != se; ++si)
+ SR.markInUse(*si);
}
}
diff --git a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
index dfe0a0e..6c3dfac 100644
--- a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
@@ -282,7 +282,7 @@ void CallAndMessageChecker::emitNilReceiverBug(CheckerContext &C,
static bool supportsNilWithFloatRet(const llvm::Triple &triple) {
return triple.getVendor() == llvm::Triple::Apple &&
- (triple.getDarwinMajorNumber() >= 9 ||
+ (!triple.isMacOSXVersionLT(10,5) ||
triple.getArch() == llvm::Triple::arm ||
triple.getArch() == llvm::Triple::thumb);
}
diff --git a/lib/StaticAnalyzer/Checkers/Checkers.td b/lib/StaticAnalyzer/Checkers/Checkers.td
index 1a71fc4..2c196b5 100644
--- a/lib/StaticAnalyzer/Checkers/Checkers.td
+++ b/lib/StaticAnalyzer/Checkers/Checkers.td
@@ -168,10 +168,6 @@ def ReturnUndefChecker : Checker<"UndefReturn">,
let ParentPackage = CplusplusExperimental in {
-def CStringChecker : Checker<"CString">,
- HelpText<"Check calls to functions in <string.h>">,
- DescFile<"CStringChecker.cpp">;
-
def IteratorsChecker : Checker<"Iterators">,
HelpText<"Check improper uses of STL vector iterators">,
DescFile<"IteratorsChecker.cpp">;
@@ -244,6 +240,10 @@ def ChrootChecker : Checker<"Chroot">,
HelpText<"Check improper use of chroot">,
DescFile<"ChrootChecker.cpp">;
+def CStringChecker : Checker<"CString">,
+ HelpText<"Check calls to functions in <string.h>">,
+ DescFile<"CStringChecker.cpp">;
+
def MallocChecker : Checker<"Malloc">,
HelpText<"Check for potential memory leaks, double free, and use-after-free problems">,
DescFile<"MallocChecker.cpp">;
diff --git a/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp b/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
index bc1d823..ec2a88a 100644
--- a/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
@@ -254,13 +254,13 @@ public:
return;
if (Expr* E = V->getInit()) {
+ while (ExprWithCleanups *exprClean = dyn_cast<ExprWithCleanups>(E))
+ E = exprClean->getSubExpr();
+
// Don't warn on C++ objects (yet) until we can show that their
// constructors/destructors don't have side effects.
if (isa<CXXConstructExpr>(E))
return;
-
- if (isa<ExprWithCleanups>(E))
- return;
// A dead initialization is a variable that is dead after it
// is initialized. We don't flag warnings for those variables
diff --git a/lib/StaticAnalyzer/Checkers/IteratorsChecker.cpp b/lib/StaticAnalyzer/Checkers/IteratorsChecker.cpp
index e4e5f54..de6da4f 100644
--- a/lib/StaticAnalyzer/Checkers/IteratorsChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/IteratorsChecker.cpp
@@ -237,8 +237,11 @@ const GRState *IteratorsChecker::invalidateIterators(const GRState *state,
const GRState *IteratorsChecker::handleAssign(const GRState *state,
const Expr *lexp, const Expr *rexp, const LocationContext *LC) const {
// Skip the cast if present.
- if (isa<ImplicitCastExpr>(lexp))
- lexp = dyn_cast<ImplicitCastExpr>(lexp)->getSubExpr();
+ if (const MaterializeTemporaryExpr *M
+ = dyn_cast<MaterializeTemporaryExpr>(lexp))
+ lexp = M->GetTemporaryExpr();
+ if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(lexp))
+ lexp = ICE->getSubExpr();
SVal sv = state->getSVal(lexp);
const MemRegion *MR = sv.getAsRegion();
if (!MR)
@@ -260,8 +263,11 @@ const GRState *IteratorsChecker::handleAssign(const GRState *state,
const MemRegion *MR, const Expr *rexp, const LocationContext *LC) const {
// Assume unknown until we find something definite.
state = state->set<IteratorState>(MR, RefState::getUnknown());
- if (isa<ImplicitCastExpr>(rexp))
- rexp = dyn_cast<ImplicitCastExpr>(rexp)->getSubExpr();
+ if (const MaterializeTemporaryExpr *M
+ = dyn_cast<MaterializeTemporaryExpr>(rexp))
+ rexp = M->GetTemporaryExpr();
+ if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(rexp))
+ rexp = ICE->getSubExpr();
// Need to handle three cases: MemberCall, copy, copy with addition.
if (const CallExpr *CE = dyn_cast<CallExpr>(rexp)) {
// Handle MemberCall.
@@ -347,8 +353,10 @@ const DeclRefExpr *IteratorsChecker::getDeclRefExpr(const Expr *E) const {
E = CE->getArg(0);
}
}
- if (isa<ImplicitCastExpr>(E))
- E = dyn_cast<ImplicitCastExpr>(E)->getSubExpr();
+ if (const MaterializeTemporaryExpr *M = dyn_cast<MaterializeTemporaryExpr>(E))
+ E = M->GetTemporaryExpr();
+ if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E))
+ E = ICE->getSubExpr();
// If it isn't one of our types, don't do anything.
if (getTemplateKind(E->getType()) != VectorIteratorKind)
return NULL;
@@ -520,8 +528,11 @@ void IteratorsChecker::checkPreStmt(const DeclStmt *DS,
if (const CXXConstructExpr *CE = dyn_cast<CXXConstructExpr>(InitEx)) {
if (CE->getNumArgs() == 1) {
const Expr *E = CE->getArg(0);
- if (isa<ImplicitCastExpr>(E))
- InitEx = dyn_cast<ImplicitCastExpr>(E)->getSubExpr();
+ if (const MaterializeTemporaryExpr *M
+ = dyn_cast<MaterializeTemporaryExpr>(E))
+ E = M->GetTemporaryExpr();
+ if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E))
+ InitEx = ICE->getSubExpr();
state = handleAssign(state, MR, InitEx,
C.getPredecessor()->getLocationContext());
}
diff --git a/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
index 12ce866..f8d076b 100644
--- a/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
@@ -31,22 +31,17 @@ using namespace ento;
namespace {
class MacOSXAPIChecker : public Checker< check::PreStmt<CallExpr> > {
- enum SubChecks {
- DispatchOnce = 0,
- DispatchOnceF,
- NumChecks
- };
-
- mutable BugType *BTypes[NumChecks];
+ mutable llvm::OwningPtr<BugType> BT_dispatchOnce;
public:
- MacOSXAPIChecker() { memset(BTypes, 0, sizeof(*BTypes) * NumChecks); }
- ~MacOSXAPIChecker() {
- for (unsigned i=0; i != NumChecks; ++i)
- delete BTypes[i];
- }
-
void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
+
+ void CheckDispatchOnce(CheckerContext &C, const CallExpr *CE,
+ const IdentifierInfo *FI) const;
+
+ typedef void (MacOSXAPIChecker::*SubChecker)(CheckerContext &,
+ const CallExpr *,
+ const IdentifierInfo *) const;
};
} //end anonymous namespace
@@ -54,16 +49,8 @@ public:
// dispatch_once and dispatch_once_f
//===----------------------------------------------------------------------===//
-static void CheckDispatchOnce(CheckerContext &C, const CallExpr *CE,
- BugType *&BT, const IdentifierInfo *FI) {
-
- if (!BT) {
- llvm::SmallString<128> S;
- llvm::raw_svector_ostream os(S);
- os << "Improper use of '" << FI->getName() << '\'';
- BT = new BugType(os.str(), "Mac OS X API");
- }
-
+void MacOSXAPIChecker::CheckDispatchOnce(CheckerContext &C, const CallExpr *CE,
+ const IdentifierInfo *FI) const {
if (CE->getNumArgs() < 1)
return;
@@ -78,6 +65,10 @@ static void CheckDispatchOnce(CheckerContext &C, const CallExpr *CE,
if (!N)
return;
+ if (!BT_dispatchOnce)
+ BT_dispatchOnce.reset(new BugType("Improper use of 'dispatch_once'",
+ "Mac OS X API"));
+
llvm::SmallString<256> S;
llvm::raw_svector_ostream os(S);
os << "Call to '" << FI->getName() << "' uses";
@@ -90,7 +81,7 @@ static void CheckDispatchOnce(CheckerContext &C, const CallExpr *CE,
if (isa<VarRegion>(R) && isa<StackLocalsSpaceRegion>(R->getMemorySpace()))
os << " Perhaps you intended to declare the variable as 'static'?";
- EnhancedBugReport *report = new EnhancedBugReport(*BT, os.str(), N);
+ RangedBugReport *report = new RangedBugReport(*BT_dispatchOnce, os.str(), N);
report->addRange(CE->getArg(0)->getSourceRange());
C.EmitReport(report);
}
@@ -99,47 +90,29 @@ static void CheckDispatchOnce(CheckerContext &C, const CallExpr *CE,
// Central dispatch function.
//===----------------------------------------------------------------------===//
-typedef void (*SubChecker)(CheckerContext &C, const CallExpr *CE, BugType *&BT,
- const IdentifierInfo *FI);
-namespace {
- class SubCheck {
- SubChecker SC;
- BugType **BT;
- public:
- SubCheck(SubChecker sc, BugType *& bt) : SC(sc), BT(&bt) {}
- SubCheck() : SC(NULL), BT(NULL) {}
-
- void run(CheckerContext &C, const CallExpr *CE,
- const IdentifierInfo *FI) const {
- if (SC)
- SC(C, CE, *BT, FI);
- }
- };
-} // end anonymous namespace
-
void MacOSXAPIChecker::checkPreStmt(const CallExpr *CE,
CheckerContext &C) const {
- // FIXME: Mostly copy and paste from UnixAPIChecker. Should refactor.
+ // FIXME: This sort of logic is common to several checkers, including
+ // UnixAPIChecker, PthreadLockChecker, and CStringChecker. Should refactor.
const GRState *state = C.getState();
const Expr *Callee = CE->getCallee();
- const FunctionTextRegion *Fn =
- dyn_cast_or_null<FunctionTextRegion>(state->getSVal(Callee).getAsRegion());
+ const FunctionDecl *Fn = state->getSVal(Callee).getAsFunctionDecl();
if (!Fn)
return;
- const IdentifierInfo *FI = Fn->getDecl()->getIdentifier();
+ const IdentifierInfo *FI = Fn->getIdentifier();
if (!FI)
return;
- const SubCheck &SC =
- llvm::StringSwitch<SubCheck>(FI->getName())
- .Case("dispatch_once", SubCheck(CheckDispatchOnce, BTypes[DispatchOnce]))
- .Case("dispatch_once_f", SubCheck(CheckDispatchOnce,
- BTypes[DispatchOnceF]))
- .Default(SubCheck());
+ SubChecker SC =
+ llvm::StringSwitch<SubChecker>(FI->getName())
+ .Cases("dispatch_once", "dispatch_once_f",
+ &MacOSXAPIChecker::CheckDispatchOnce)
+ .Default(NULL);
- SC.run(C, CE, FI);
+ if (SC)
+ (this->*SC)(C, CE, FI);
}
//===----------------------------------------------------------------------===//
diff --git a/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp b/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index 07de870..73ce359 100644
--- a/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -121,6 +121,11 @@ void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt *RS,
return;
if (R->hasStackStorage()) {
+ // Automatic reference counting automatically copies blocks.
+ if (C.getASTContext().getLangOptions().ObjCAutoRefCount &&
+ isa<BlockDataRegion>(R))
+ return;
+
EmitStackError(C, R, RetE);
return;
}
@@ -135,12 +140,13 @@ void StackAddrEscapeChecker::checkEndPath(EndOfFunctionNodeBuilder &B,
// a memory region in the stack space.
class CallBack : public StoreManager::BindingsHandler {
private:
+ ExprEngine &Eng;
const StackFrameContext *CurSFC;
public:
llvm::SmallVector<std::pair<const MemRegion*, const MemRegion*>, 10> V;
- CallBack(const LocationContext *LCtx)
- : CurSFC(LCtx->getCurrentStackFrame()) {}
+ CallBack(ExprEngine &Eng, const LocationContext *LCtx)
+ : Eng(Eng), CurSFC(LCtx->getCurrentStackFrame()) {}
bool HandleBinding(StoreManager &SMgr, Store store,
const MemRegion *region, SVal val) {
@@ -151,7 +157,13 @@ void StackAddrEscapeChecker::checkEndPath(EndOfFunctionNodeBuilder &B,
const MemRegion *vR = val.getAsRegion();
if (!vR)
return true;
-
+
+ // Under automated retain release, it is okay to assign a block
+ // directly to a global variable.
+ if (Eng.getContext().getLangOptions().ObjCAutoRefCount &&
+ isa<BlockDataRegion>(vR))
+ return true;
+
if (const StackSpaceRegion *SSR =
dyn_cast<StackSpaceRegion>(vR->getMemorySpace())) {
// If the global variable holds a location in the current stack frame,
@@ -164,7 +176,7 @@ void StackAddrEscapeChecker::checkEndPath(EndOfFunctionNodeBuilder &B,
}
};
- CallBack cb(B.getPredecessor()->getLocationContext());
+ CallBack cb(Eng, B.getPredecessor()->getLocationContext());
state->getStateManager().getStoreManager().iterBindings(state->getStore(),cb);
if (cb.V.empty())
diff --git a/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
index 48d7c36..0ecc391 100644
--- a/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
@@ -28,26 +28,18 @@ using llvm::Optional;
namespace {
class UnixAPIChecker : public Checker< check::PreStmt<CallExpr> > {
- enum SubChecks {
- OpenFn = 0,
- PthreadOnceFn = 1,
- MallocZero = 2,
- NumChecks
- };
-
- mutable BugType *BTypes[NumChecks];
-
-public:
+ mutable llvm::OwningPtr<BugType> BT_open, BT_pthreadOnce, BT_mallocZero;
mutable Optional<uint64_t> Val_O_CREAT;
public:
- UnixAPIChecker() { memset(BTypes, 0, sizeof(*BTypes) * NumChecks); }
- ~UnixAPIChecker() {
- for (unsigned i=0; i != NumChecks; ++i)
- delete BTypes[i];
- }
-
void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
+
+ void CheckOpen(CheckerContext &C, const CallExpr *CE) const;
+ void CheckPthreadOnce(CheckerContext &C, const CallExpr *CE) const;
+ void CheckMallocZero(CheckerContext &C, const CallExpr *CE) const;
+
+ typedef void (UnixAPIChecker::*SubChecker)(CheckerContext &,
+ const CallExpr *) const;
};
} //end anonymous namespace
@@ -55,23 +47,23 @@ public:
// Utility functions.
//===----------------------------------------------------------------------===//
-static inline void LazyInitialize(BugType *&BT, const char *name) {
+static inline void LazyInitialize(llvm::OwningPtr<BugType> &BT,
+ const char *name) {
if (BT)
return;
- BT = new BugType(name, "Unix API");
+ BT.reset(new BugType(name, "Unix API"));
}
//===----------------------------------------------------------------------===//
// "open" (man 2 open)
//===----------------------------------------------------------------------===//
-static void CheckOpen(CheckerContext &C, const UnixAPIChecker &UC,
- const CallExpr *CE, BugType *&BT) {
+void UnixAPIChecker::CheckOpen(CheckerContext &C, const CallExpr *CE) const {
// The definition of O_CREAT is platform specific. We need a better way
// of querying this information from the checking environment.
- if (!UC.Val_O_CREAT.hasValue()) {
+ if (!Val_O_CREAT.hasValue()) {
if (C.getASTContext().Target.getTriple().getVendor() == llvm::Triple::Apple)
- UC.Val_O_CREAT = 0x0200;
+ Val_O_CREAT = 0x0200;
else {
// FIXME: We need a more general way of getting the O_CREAT value.
// We could possibly grovel through the preprocessor state, but
@@ -80,8 +72,6 @@ static void CheckOpen(CheckerContext &C, const UnixAPIChecker &UC,
}
}
- LazyInitialize(BT, "Improper use of 'open'");
-
// Look at the 'oflags' argument for the O_CREAT flag.
const GRState *state = C.getState();
@@ -101,7 +91,7 @@ static void CheckOpen(CheckerContext &C, const UnixAPIChecker &UC,
}
NonLoc oflags = cast<NonLoc>(V);
NonLoc ocreateFlag =
- cast<NonLoc>(C.getSValBuilder().makeIntVal(UC.Val_O_CREAT.getValue(),
+ cast<NonLoc>(C.getSValBuilder().makeIntVal(Val_O_CREAT.getValue(),
oflagsEx->getType()));
SVal maskedFlagsUC = C.getSValBuilder().evalBinOpNN(state, BO_And,
oflags, ocreateFlag,
@@ -124,8 +114,10 @@ static void CheckOpen(CheckerContext &C, const UnixAPIChecker &UC,
if (!N)
return;
- EnhancedBugReport *report =
- new EnhancedBugReport(*BT,
+ LazyInitialize(BT_open, "Improper use of 'open'");
+
+ RangedBugReport *report =
+ new RangedBugReport(*BT_open,
"Call to 'open' requires a third argument when "
"the 'O_CREAT' flag is set", N);
report->addRange(oflagsEx->getSourceRange());
@@ -137,14 +129,12 @@ static void CheckOpen(CheckerContext &C, const UnixAPIChecker &UC,
// pthread_once
//===----------------------------------------------------------------------===//
-static void CheckPthreadOnce(CheckerContext &C, const UnixAPIChecker &,
- const CallExpr *CE, BugType *&BT) {
+void UnixAPIChecker::CheckPthreadOnce(CheckerContext &C,
+ const CallExpr *CE) const {
// This is similar to 'CheckDispatchOnce' in the MacOSXAPIChecker.
// They can possibly be refactored.
- LazyInitialize(BT, "Improper use of 'pthread_once'");
-
if (CE->getNumArgs() < 1)
return;
@@ -171,7 +161,9 @@ static void CheckPthreadOnce(CheckerContext &C, const UnixAPIChecker &,
if (isa<VarRegion>(R) && isa<StackLocalsSpaceRegion>(R->getMemorySpace()))
os << " Perhaps you intended to declare the variable as 'static'?";
- EnhancedBugReport *report = new EnhancedBugReport(*BT, os.str(), N);
+ LazyInitialize(BT_pthreadOnce, "Improper use of 'pthread_once'");
+
+ RangedBugReport *report = new RangedBugReport(*BT_pthreadOnce, os.str(), N);
report->addRange(CE->getArg(0)->getSourceRange());
C.EmitReport(report);
}
@@ -182,8 +174,8 @@ static void CheckPthreadOnce(CheckerContext &C, const UnixAPIChecker &,
// FIXME: Eventually this should be rolled into the MallocChecker, but this
// check is more basic and is valuable for widespread use.
-static void CheckMallocZero(CheckerContext &C, const UnixAPIChecker &UC,
- const CallExpr *CE, BugType *&BT) {
+void UnixAPIChecker::CheckMallocZero(CheckerContext &C,
+ const CallExpr *CE) const {
// Sanity check that malloc takes one argument.
if (CE->getNumArgs() != 1)
@@ -208,11 +200,11 @@ static void CheckMallocZero(CheckerContext &C, const UnixAPIChecker &UC,
// FIXME: Add reference to CERT advisory, and/or C99 standard in bug
// output.
- LazyInitialize(BT, "Undefined allocation of 0 bytes");
+ LazyInitialize(BT_mallocZero, "Undefined allocation of 0 bytes");
EnhancedBugReport *report =
- new EnhancedBugReport(*BT, "Call to 'malloc' has an allocation size"
- " of 0 bytes", N);
+ new EnhancedBugReport(*BT_mallocZero, "Call to 'malloc' has an allocation"
+ " size of 0 bytes", N);
report->addRange(CE->getArg(0)->getSourceRange());
report->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue,
CE->getArg(0));
@@ -230,51 +222,29 @@ static void CheckMallocZero(CheckerContext &C, const UnixAPIChecker &UC,
// Central dispatch function.
//===----------------------------------------------------------------------===//
-typedef void (*SubChecker)(CheckerContext &C, const UnixAPIChecker &UC,
- const CallExpr *CE, BugType *&BT);
-namespace {
- class SubCheck {
- SubChecker SC;
- const UnixAPIChecker *UC;
- BugType **BT;
- public:
- SubCheck(SubChecker sc, const UnixAPIChecker *uc, BugType *& bt)
- : SC(sc), UC(uc), BT(&bt) {}
- SubCheck() : SC(NULL), UC(NULL), BT(NULL) {}
-
- void run(CheckerContext &C, const CallExpr *CE) const {
- if (SC)
- SC(C, *UC, CE, *BT);
- }
- };
-} // end anonymous namespace
-
void UnixAPIChecker::checkPreStmt(const CallExpr *CE, CheckerContext &C) const {
// Get the callee. All the functions we care about are C functions
// with simple identifiers.
const GRState *state = C.getState();
const Expr *Callee = CE->getCallee();
- const FunctionTextRegion *Fn =
- dyn_cast_or_null<FunctionTextRegion>(state->getSVal(Callee).getAsRegion());
+ const FunctionDecl *Fn = state->getSVal(Callee).getAsFunctionDecl();
if (!Fn)
return;
- const IdentifierInfo *FI = Fn->getDecl()->getIdentifier();
+ const IdentifierInfo *FI = Fn->getIdentifier();
if (!FI)
return;
- const SubCheck &SC =
- llvm::StringSwitch<SubCheck>(FI->getName())
- .Case("open",
- SubCheck(CheckOpen, this, BTypes[OpenFn]))
- .Case("pthread_once",
- SubCheck(CheckPthreadOnce, this, BTypes[PthreadOnceFn]))
- .Case("malloc",
- SubCheck(CheckMallocZero, this, BTypes[MallocZero]))
- .Default(SubCheck());
-
- SC.run(C, CE);
+ SubChecker SC =
+ llvm::StringSwitch<SubChecker>(FI->getName())
+ .Case("open", &UnixAPIChecker::CheckOpen)
+ .Case("pthread_once", &UnixAPIChecker::CheckPthreadOnce)
+ .Case("malloc", &UnixAPIChecker::CheckMallocZero)
+ .Default(NULL);
+
+ if (SC)
+ (this->*SC)(C, CE);
}
//===----------------------------------------------------------------------===//
diff --git a/lib/StaticAnalyzer/Core/CFRefCount.cpp b/lib/StaticAnalyzer/Core/CFRefCount.cpp
index 0512e2f..bf53029 100644
--- a/lib/StaticAnalyzer/Core/CFRefCount.cpp
+++ b/lib/StaticAnalyzer/Core/CFRefCount.cpp
@@ -126,6 +126,7 @@ public:
/// ArgEffect is used to summarize a function/method call's effect on a
/// particular argument.
enum ArgEffect { Autorelease, Dealloc, DecRef, DecRefMsg, DoNothing,
+ DecRefBridgedTransfered,
DoNothingByRef, IncRefMsg, IncRef, MakeCollectable, MayEscape,
NewAutoreleasePool, SelfOwn, StopTracking };
@@ -148,7 +149,8 @@ namespace {
class RetEffect {
public:
enum Kind { NoRet, Alias, OwnedSymbol, OwnedAllocatedSymbol,
- NotOwnedSymbol, GCNotOwnedSymbol, ReceiverAlias,
+ NotOwnedSymbol, GCNotOwnedSymbol, ARCNotOwnedSymbol,
+ ReceiverAlias,
OwnedWhenTrackedReceiver };
enum ObjKind { CF, ObjC, AnyObj };
@@ -195,7 +197,9 @@ public:
static RetEffect MakeGCNotOwned() {
return RetEffect(GCNotOwnedSymbol, ObjC);
}
-
+ static RetEffect MakeARCNotOwned() {
+ return RetEffect(ARCNotOwnedSymbol, ObjC);
+ }
static RetEffect MakeNoRet() {
return RetEffect(NoRet);
}
@@ -636,6 +640,9 @@ class RetainSummaryManager {
/// GCEnabled - Records whether or not the analyzed code runs in GC mode.
const bool GCEnabled;
+ /// Records whether or not the analyzed code runs in ARC mode.
+ const bool ARCEnabled;
+
/// FuncSummaries - A map from FunctionDecls to summaries.
FuncSummariesTy FuncSummaries;
@@ -788,14 +795,20 @@ private:
public:
- RetainSummaryManager(ASTContext& ctx, bool gcenabled)
+ RetainSummaryManager(ASTContext& ctx, bool gcenabled, bool usesARC)
: Ctx(ctx),
CFDictionaryCreateII(&ctx.Idents.get("CFDictionaryCreate")),
- GCEnabled(gcenabled), AF(BPAlloc), ScratchArgs(AF.getEmptyMap()),
- ObjCAllocRetE(gcenabled ? RetEffect::MakeGCNotOwned()
- : RetEffect::MakeOwned(RetEffect::ObjC, true)),
- ObjCInitRetE(gcenabled ? RetEffect::MakeGCNotOwned()
- : RetEffect::MakeOwnedWhenTrackedReceiver()),
+ GCEnabled(gcenabled),
+ ARCEnabled(usesARC),
+ AF(BPAlloc), ScratchArgs(AF.getEmptyMap()),
+ ObjCAllocRetE(gcenabled
+ ? RetEffect::MakeGCNotOwned()
+ : (usesARC ? RetEffect::MakeARCNotOwned()
+ : RetEffect::MakeOwned(RetEffect::ObjC, true))),
+ ObjCInitRetE(gcenabled
+ ? RetEffect::MakeGCNotOwned()
+ : (usesARC ? RetEffect::MakeARCNotOwned()
+ : RetEffect::MakeOwnedWhenTrackedReceiver())),
DefaultSummary(AF.getEmptyMap() /* per-argument effects (none) */,
RetEffect::MakeNoRet() /* return effect */,
MayEscape, /* default argument effect */
@@ -871,6 +884,10 @@ public:
bool isGCEnabled() const { return GCEnabled; }
+ bool isARCEnabled() const { return ARCEnabled; }
+
+ bool isARCorGCEnabled() const { return GCEnabled || ARCEnabled; }
+
RetainSummary *copySummary(RetainSummary *OldSumm) {
RetainSummary *Summ = (RetainSummary*) BPAlloc.Allocate<RetainSummary>();
new (Summ) RetainSummary(*OldSumm);
@@ -1118,8 +1135,7 @@ RetainSummary* RetainSummaryManager::getSummary(const FunctionDecl* FD) {
RetainSummary*
RetainSummaryManager::getCFCreateGetRuleSummary(const FunctionDecl* FD,
StringRef FName) {
- if (FName.find("Create") != StringRef::npos ||
- FName.find("Copy") != StringRef::npos)
+ if (coreFoundation::followsCreateRule(FName))
return getCFSummaryCreateRule(FD);
return getCFSummaryGetRule(FD);
@@ -1189,7 +1205,8 @@ RetainSummaryManager::getInitMethodSummary(QualType RetTy) {
assert(ScratchArgs.isEmpty());
// 'init' methods conceptually return a newly allocated object and claim
// the receiver.
- if (cocoa::isCocoaObjectRef(RetTy) || cocoa::isCFObjectRef(RetTy))
+ if (cocoa::isCocoaObjectRef(RetTy) ||
+ coreFoundation::isCFObjectRef(RetTy))
return getPersistentSummary(ObjCInitRetE, DecRefMsg);
return getDefaultSummary();
@@ -1332,15 +1349,15 @@ RetainSummaryManager::getCommonMethodSummary(const ObjCMethodDecl* MD,
if (cocoa::isCocoaObjectRef(RetTy)) {
// EXPERIMENTAL: assume the Cocoa conventions for all objects returned
// by instance methods.
- RetEffect E = cocoa::followsFundamentalRule(S)
+ RetEffect E = cocoa::followsFundamentalRule(S, MD)
? ObjCAllocRetE : RetEffect::MakeNotOwned(RetEffect::ObjC);
return getPersistentSummary(E, ReceiverEff, MayEscape);
}
// Look for methods that return an owned core foundation object.
- if (cocoa::isCFObjectRef(RetTy)) {
- RetEffect E = cocoa::followsFundamentalRule(S)
+ if (coreFoundation::isCFObjectRef(RetTy)) {
+ RetEffect E = cocoa::followsFundamentalRule(S, MD)
? RetEffect::MakeOwned(RetEffect::CF, true)
: RetEffect::MakeNotOwned(RetEffect::CF);
@@ -1411,7 +1428,7 @@ RetainSummaryManager::getInstanceMethodSummary(Selector S,
assert(ScratchArgs.isEmpty());
// "initXXX": pass-through for receiver.
- if (cocoa::deriveNamingConvention(S) == cocoa::InitRule)
+ if (cocoa::deriveNamingConvention(S, MD) == cocoa::InitRule)
Summ = getInitMethodSummary(RetTy);
else
Summ = getCommonMethodSummary(MD, S, RetTy);
@@ -1654,7 +1671,6 @@ public:
const char* nl, const char* sep);
};
-private:
typedef llvm::DenseMap<const ExplodedNode*, const RetainSummary*>
SummaryLogTy;
@@ -1691,7 +1707,7 @@ private:
public:
CFRefCount(ASTContext& Ctx, bool gcenabled, const LangOptions& lopts)
- : Summaries(Ctx, gcenabled),
+ : Summaries(Ctx, gcenabled, (bool)lopts.ObjCAutoRefCount),
LOpts(lopts), useAfterRelease(0), releaseNotOwned(0),
deallocGC(0), deallocNotOwned(0),
leakWithinFunction(0), leakAtReturn(0), overAutorelease(0),
@@ -1706,6 +1722,8 @@ public:
}
bool isGCEnabled() const { return Summaries.isGCEnabled(); }
+ bool isARCorGCEnabled() const { return Summaries.isARCorGCEnabled(); }
+
const LangOptions& getLangOptions() const { return LOpts; }
const RetainSummary *getSummaryOfNode(const ExplodedNode *N) const {
@@ -1907,7 +1925,7 @@ namespace {
CFRefBug(tf, "Method should return an owned object") {}
const char *getDescription() const {
- return "Object with +0 retain counts returned to caller where a +1 "
+ return "Object with a +0 retain count returned to caller where a +1 "
"(owning) retain count is expected";
}
};
@@ -2094,7 +2112,7 @@ PathDiagnosticPiece* CFRefReport::VisitNode(const ExplodedNode* N,
if (static_cast<CFRefBug&>(getBugType()).getTF().isGCEnabled()) {
assert(CurrV.getObjKind() == RetEffect::CF);
- os << " "
+ os << ". "
"Core Foundation objects are not automatically garbage collected.";
}
}
@@ -2416,14 +2434,14 @@ CFRefLeakReport::getEndPath(BugReporterContext& BRC,
if (RV->getKind() == RefVal::ErrorLeakReturned) {
// FIXME: Per comments in rdar://6320065, "create" only applies to CF
- // ojbects. Only "copy", "alloc", "retain" and "new" transfer ownership
+ // objects. Only "copy", "alloc", "retain" and "new" transfer ownership
// to the caller for NS objects.
const Decl *D = &EndN->getCodeDecl();
if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D)) {
os << " is returned from a method whose name ('"
<< MD->getSelector().getAsString()
<< "') does not start with 'copy', 'mutableCopy', 'alloc' or 'new'."
- " This violates the naming convention rules "
+ " This violates the naming convention rules"
" given in the Memory Management Guide for Cocoa";
}
else {
@@ -2431,7 +2449,7 @@ CFRefLeakReport::getEndPath(BugReporterContext& BRC,
os << " is return from a function whose name ('"
<< FD->getNameAsString()
<< "') does not contain 'Copy' or 'Create'. This violates the naming"
- " convention rules given the Memory Management Guide for Core "
+ " convention rules given the Memory Management Guide for Core"
" Foundation";
}
}
@@ -2777,6 +2795,7 @@ void CFRefCount::evalSummary(ExplodedNodeSet& Dst,
}
case RetEffect::GCNotOwnedSymbol:
+ case RetEffect::ARCNotOwnedSymbol:
case RetEffect::NotOwnedSymbol: {
unsigned Count = Builder.getCurrentBlockCount();
SValBuilder &svalBuilder = Eng.getSValBuilder();
@@ -3103,8 +3122,8 @@ const GRState * CFRefCount::Update(const GRState * state, SymbolRef sym,
// In GC mode [... release] and [... retain] do nothing.
switch (E) {
default: break;
- case IncRefMsg: E = isGCEnabled() ? DoNothing : IncRef; break;
- case DecRefMsg: E = isGCEnabled() ? DoNothing : DecRef; break;
+ case IncRefMsg: E = isARCorGCEnabled() ? DoNothing : IncRef; break;
+ case DecRefMsg: E = isARCorGCEnabled() ? DoNothing : DecRef; break;
case MakeCollectable: E = isGCEnabled() ? DecRef : DoNothing; break;
case NewAutoreleasePool: E = isGCEnabled() ? DoNothing :
NewAutoreleasePool; break;
@@ -3118,9 +3137,13 @@ const GRState * CFRefCount::Update(const GRState * state, SymbolRef sym,
}
switch (E) {
- default:
- assert (false && "Unhandled CFRef transition.");
-
+ case DecRefMsg:
+ case IncRefMsg:
+ case MakeCollectable:
+ assert(false &&
+ "DecRefMsg/IncRefMsg/MakeCollectable already transformed");
+ return state;
+
case Dealloc:
// Any use of -dealloc in GC is *bad*.
if (isGCEnabled()) {
@@ -3193,6 +3216,7 @@ const GRState * CFRefCount::Update(const GRState * state, SymbolRef sym,
V = V ^ RefVal::NotOwned;
// Fall-through.
case DecRef:
+ case DecRefBridgedTransfered:
switch (V.getKind()) {
default:
// case 'RefVal::Released' handled above.
@@ -3200,7 +3224,9 @@ const GRState * CFRefCount::Update(const GRState * state, SymbolRef sym,
case RefVal::Owned:
assert(V.getCount() > 0);
- if (V.getCount() == 1) V = V ^ RefVal::Released;
+ if (V.getCount() == 1)
+ V = V ^ (E == DecRefBridgedTransfered ?
+ RefVal::NotOwned : RefVal::Released);
V = V - 1;
break;
@@ -3280,15 +3306,10 @@ CFRefCount::HandleAutoreleaseCounts(const GRState * state,
std::string sbuf;
llvm::raw_string_ostream os(sbuf);
- os << "Object over-autoreleased: object was sent -autorelease";
+ os << "Object over-autoreleased: object was sent -autorelease ";
if (V.getAutoreleaseCount() > 1)
- os << V.getAutoreleaseCount() << " times";
- os << " but the object has ";
- if (V.getCount() == 0)
- os << "zero (locally visible)";
- else
- os << "+" << V.getCount();
- os << " retain counts";
+ os << V.getAutoreleaseCount() << " times ";
+ os << "but the object has a +" << V.getCount() << " retain count";
CFRefReport *report =
new CFRefReport(*static_cast<CFRefBug*>(overAutorelease),
@@ -3468,7 +3489,9 @@ void CFRefCount::ProcessNonLeakError(ExplodedNodeSet& Dst,
namespace {
class RetainReleaseChecker
- : public Checker< check::PostStmt<BlockExpr>, check::RegionChanges > {
+ : public Checker< check::PostStmt<BlockExpr>,
+ check::PostStmt<CastExpr>,
+ check::RegionChanges > {
public:
bool wantsRegionUpdate;
@@ -3476,6 +3499,9 @@ public:
void checkPostStmt(const BlockExpr *BE, CheckerContext &C) const;
+
+ void checkPostStmt(const CastExpr *CE, CheckerContext &C) const;
+
const GRState *checkRegionChanges(const GRState *state,
const StoreManager::InvalidatedSymbols *invalidated,
const MemRegion * const *begin,
@@ -3545,6 +3571,48 @@ void RetainReleaseChecker::checkPostStmt(const BlockExpr *BE,
C.addTransition(state);
}
+void RetainReleaseChecker::checkPostStmt(const CastExpr *CE,
+ CheckerContext &C) const {
+ const ObjCBridgedCastExpr *BE = dyn_cast<ObjCBridgedCastExpr>(CE);
+ if (!BE)
+ return;
+
+ ArgEffect AE = IncRef;
+
+ switch (BE->getBridgeKind()) {
+ case clang::OBC_Bridge:
+ // Do nothing.
+ return;
+ case clang::OBC_BridgeRetained:
+ AE = IncRef;
+ break;
+ case clang::OBC_BridgeTransfer:
+ AE = DecRefBridgedTransfered;
+ break;
+ }
+
+ const GRState *state = C.getState();
+ SymbolRef Sym = state->getSVal(CE).getAsLocSymbol();
+ if (!Sym)
+ return;
+ const RefVal* T = state->get<RefBindings>(Sym);
+ if (!T)
+ return;
+
+ // This is gross. Once the checker and CFRefCount are unified,
+ // this will go away.
+ CFRefCount &cf = static_cast<CFRefCount&>(C.getEngine().getTF());
+ RefVal::Kind hasErr = (RefVal::Kind) 0;
+ state = cf.Update(state, Sym, *T, AE, hasErr);
+
+ if (hasErr) {
+
+ return;
+ }
+
+ C.generateNode(state);
+}
+
//===----------------------------------------------------------------------===//
// Transfer function creation for external clients.
//===----------------------------------------------------------------------===//
diff --git a/lib/StaticAnalyzer/Core/Environment.cpp b/lib/StaticAnalyzer/Core/Environment.cpp
index 48f126b..3961c7b 100644
--- a/lib/StaticAnalyzer/Core/Environment.cpp
+++ b/lib/StaticAnalyzer/Core/Environment.cpp
@@ -83,6 +83,9 @@ SVal Environment::getSVal(const Stmt *E, SValBuilder& svalBuilder,
case Stmt::CXXBindTemporaryExprClass:
E = cast<CXXBindTemporaryExpr>(E)->getSubExpr();
continue;
+ case Stmt::MaterializeTemporaryExprClass:
+ E = cast<MaterializeTemporaryExpr>(E)->GetTemporaryExpr();
+ continue;
// Handle all other Stmt* using a lookup.
default:
break;
diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp
index aed39eb..ffe5f0b 100644
--- a/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -442,7 +442,8 @@ void ExprEngine::Visit(const Stmt* S, ExplodedNode* Pred,
}
switch (S->getStmtClass()) {
- // C++ stuff we don't support yet.
+ // C++ and ARC stuff we don't support yet.
+ case Expr::ObjCIndirectCopyRestoreExprClass:
case Stmt::CXXBindTemporaryExprClass:
case Stmt::CXXCatchStmtClass:
case Stmt::CXXDependentScopeMemberExprClass:
@@ -478,6 +479,7 @@ void ExprEngine::Visit(const Stmt* S, ExplodedNode* Pred,
// We don't handle default arguments either yet, but we can fake it
// for now by just skipping them.
+ case Stmt::SubstNonTypeTemplateParmExprClass:
case Stmt::CXXDefaultArgExprClass: {
Dst.Add(Pred);
break;
@@ -508,7 +510,10 @@ void ExprEngine::Visit(const Stmt* S, ExplodedNode* Pred,
break;
case Stmt::GNUNullExprClass: {
- MakeNode(Dst, S, Pred, GetState(Pred)->BindExpr(S, svalBuilder.makeNull()));
+ // GNU __null is a pointer-width integer, not an actual pointer.
+ const GRState *state = GetState(Pred);
+ state = state->BindExpr(S, svalBuilder.makeIntValWithPtrWidth(0, false));
+ MakeNode(Dst, S, Pred, state);
break;
}
@@ -520,14 +525,27 @@ void ExprEngine::Visit(const Stmt* S, ExplodedNode* Pred,
VisitObjCPropertyRefExpr(cast<ObjCPropertyRefExpr>(S), Pred, Dst);
break;
+ case Stmt::ImplicitValueInitExprClass: {
+ const GRState *state = GetState(Pred);
+ QualType ty = cast<ImplicitValueInitExpr>(S)->getType();
+ SVal val = svalBuilder.makeZeroVal(ty);
+ MakeNode(Dst, S, Pred, state->BindExpr(S, val));
+ break;
+ }
+
+ case Stmt::ExprWithCleanupsClass: {
+ Visit(cast<ExprWithCleanups>(S)->getSubExpr(), Pred, Dst);
+ break;
+ }
+
// Cases not handled yet; but will handle some day.
case Stmt::DesignatedInitExprClass:
case Stmt::ExtVectorElementExprClass:
case Stmt::ImaginaryLiteralClass:
- case Stmt::ImplicitValueInitExprClass:
case Stmt::ObjCAtCatchStmtClass:
case Stmt::ObjCAtFinallyStmtClass:
case Stmt::ObjCAtTryStmtClass:
+ case Stmt::ObjCAutoreleasePoolStmtClass:
case Stmt::ObjCEncodeExprClass:
case Stmt::ObjCIsaExprClass:
case Stmt::ObjCProtocolExprClass:
@@ -548,7 +566,6 @@ void ExprEngine::Visit(const Stmt* S, ExplodedNode* Pred,
case Stmt::IntegerLiteralClass:
case Stmt::CharacterLiteralClass:
case Stmt::CXXBoolLiteralExprClass:
- case Stmt::ExprWithCleanupsClass:
case Stmt::FloatingLiteralClass:
case Stmt::SizeOfPackExprClass:
case Stmt::CXXNullPtrLiteralExprClass:
@@ -668,12 +685,35 @@ void ExprEngine::Visit(const Stmt* S, ExplodedNode* Pred,
case Stmt::CXXDynamicCastExprClass:
case Stmt::CXXReinterpretCastExprClass:
case Stmt::CXXConstCastExprClass:
- case Stmt::CXXFunctionalCastExprClass: {
+ case Stmt::CXXFunctionalCastExprClass:
+ case Stmt::ObjCBridgedCastExprClass: {
const CastExpr* C = cast<CastExpr>(S);
- VisitCast(C, C->getSubExpr(), Pred, Dst);
+ // Handle the previsit checks.
+ ExplodedNodeSet dstPrevisit;
+ getCheckerManager().runCheckersForPreStmt(dstPrevisit, Pred, C, *this);
+
+ // Handle the expression itself.
+ ExplodedNodeSet dstExpr;
+ for (ExplodedNodeSet::iterator i = dstPrevisit.begin(),
+ e = dstPrevisit.end(); i != e ; ++i) {
+ VisitCast(C, C->getSubExpr(), *i, dstExpr);
+ }
+
+ // Handle the postvisit checks.
+ getCheckerManager().runCheckersForPostStmt(Dst, dstExpr, C, *this);
break;
}
+ case Expr::MaterializeTemporaryExprClass: {
+ const MaterializeTemporaryExpr *Materialize
+ = cast<MaterializeTemporaryExpr>(S);
+ if (!Materialize->getType()->isRecordType())
+ CreateCXXTemporaryObject(Materialize->GetTemporaryExpr(), Pred, Dst);
+ else
+ Visit(Materialize->GetTemporaryExpr(), Pred, Dst);
+ break;
+ }
+
case Stmt::InitListExprClass:
VisitInitListExpr(cast<InitListExpr>(S), Pred, Dst);
break;
@@ -2148,10 +2188,19 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex,
Pred = *I;
switch (CastE->getCastKind()) {
+ case CK_LValueToRValue:
+ assert(false && "LValueToRValue casts handled earlier.");
+ case CK_GetObjCProperty:
+ assert(false && "GetObjCProperty casts handled earlier.");
case CK_ToVoid:
Dst.Add(Pred);
continue;
- case CK_LValueToRValue:
+ // The analyzer doesn't do anything special with these casts,
+ // since it understands retain/release semantics already.
+ case CK_ObjCProduceObject:
+ case CK_ObjCConsumeObject:
+ case CK_ObjCReclaimReturnedObject: // Fall-through.
+ // True no-ops.
case CK_NoOp:
case CK_FunctionToPointerDecay: {
// Copy the SVal of Ex to CastE.
@@ -2161,7 +2210,6 @@ void ExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex,
MakeNode(Dst, CastE, Pred, state);
continue;
}
- case CK_GetObjCProperty:
case CK_Dependent:
case CK_ArrayToPointerDecay:
case CK_BitCast:
@@ -2273,17 +2321,9 @@ void ExprEngine::VisitDeclStmt(const DeclStmt *DS, ExplodedNode *Pred,
// time a function is called those values may not be current.
ExplodedNodeSet Tmp;
- if (InitEx) {
- if (VD->getType()->isReferenceType() && !InitEx->isLValue()) {
- // If the initializer is C++ record type, it should already has a
- // temp object.
- if (!InitEx->getType()->isRecordType())
- CreateCXXTemporaryObject(InitEx, Pred, Tmp);
- else
- Tmp.Add(Pred);
- } else
- Visit(InitEx, Pred, Tmp);
- } else
+ if (InitEx)
+ Visit(InitEx, Pred, Tmp);
+ else
Tmp.Add(Pred);
ExplodedNodeSet Tmp2;
diff --git a/lib/StaticAnalyzer/Core/RegionStore.cpp b/lib/StaticAnalyzer/Core/RegionStore.cpp
index d0d8f60..23dd641 100644
--- a/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1377,7 +1377,12 @@ StoreRef RegionStoreManager::setImplicitDefaultValue(Store store,
V = svalBuilder.makeZeroVal(Ctx.IntTy);
}
else {
- return StoreRef(store, *this);
+ // We can't represent values of this type, but we still need to set a value
+ // to record that the region has been initialized.
+ // If this assertion ever fires, a new case should be added above -- we
+ // should know how to default-initialize any value we can symbolicate.
+ assert(!SymbolManager::canSymbolicate(T) && "This type is representable");
+ V = UnknownVal();
}
return StoreRef(addBinding(B, R, BindingKey::Default,
diff --git a/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp b/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
index 197442b..80c18a3 100644
--- a/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ b/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -415,6 +415,24 @@ SVal SimpleSValBuilder::evalBinOpNN(const GRState *state,
case nonloc::ConcreteIntKind: {
const nonloc::ConcreteInt& lhsInt = cast<nonloc::ConcreteInt>(lhs);
+ // Is the RHS a symbol we can simplify?
+ // FIXME: This was mostly copy/pasted from the LHS-is-a-symbol case.
+ if (const nonloc::SymbolVal *srhs = dyn_cast<nonloc::SymbolVal>(&rhs)) {
+ SymbolRef RSym = srhs->getSymbol();
+ if (RSym->getType(Context)->isIntegerType()) {
+ if (const llvm::APSInt *Constant = state->getSymVal(RSym)) {
+ // The symbol evaluates to a constant.
+ const llvm::APSInt *rhs_I;
+ if (BinaryOperator::isRelationalOp(op))
+ rhs_I = &BasicVals.Convert(lhsInt.getValue(), *Constant);
+ else
+ rhs_I = &BasicVals.Convert(resultTy, *Constant);
+
+ rhs = nonloc::ConcreteInt(*rhs_I);
+ }
+ }
+ }
+
if (isa<nonloc::ConcreteInt>(rhs)) {
return lhsInt.evalBinOp(*this, op, cast<nonloc::ConcreteInt>(rhs));
} else {
@@ -461,13 +479,22 @@ SVal SimpleSValBuilder::evalBinOpNN(const GRState *state,
case nonloc::SymbolValKind: {
nonloc::SymbolVal *slhs = cast<nonloc::SymbolVal>(&lhs);
SymbolRef Sym = slhs->getSymbol();
+ QualType lhsType = Sym->getType(Context);
+
+ // The conversion type is usually the result type, but not in the case
+ // of relational expressions.
+ QualType conversionType = resultTy;
+ if (BinaryOperator::isRelationalOp(op))
+ conversionType = lhsType;
+
// Does the symbol simplify to a constant? If so, "fold" the constant
// by setting 'lhs' to a ConcreteInt and try again.
- if (Sym->getType(Context)->isIntegerType())
+ if (lhsType->isIntegerType())
if (const llvm::APSInt *Constant = state->getSymVal(Sym)) {
// The symbol evaluates to a constant. If necessary, promote the
// folded constant (LHS) to the result type.
- const llvm::APSInt &lhs_I = BasicVals.Convert(resultTy, *Constant);
+ const llvm::APSInt &lhs_I = BasicVals.Convert(conversionType,
+ *Constant);
lhs = nonloc::ConcreteInt(lhs_I);
// Also promote the RHS (if necessary).
@@ -479,7 +506,7 @@ SVal SimpleSValBuilder::evalBinOpNN(const GRState *state,
// Other operators: do an implicit conversion. This shouldn't be
// necessary once we support truncation/extension of symbolic values.
if (nonloc::ConcreteInt *rhs_I = dyn_cast<nonloc::ConcreteInt>(&rhs)){
- rhs = nonloc::ConcreteInt(BasicVals.Convert(resultTy,
+ rhs = nonloc::ConcreteInt(BasicVals.Convert(conversionType,
rhs_I->getValue()));
}
@@ -492,7 +519,8 @@ SVal SimpleSValBuilder::evalBinOpNN(const GRState *state,
if (RSym->getType(Context)->isIntegerType()) {
if (const llvm::APSInt *Constant = state->getSymVal(RSym)) {
// The symbol evaluates to a constant.
- const llvm::APSInt &rhs_I = BasicVals.Convert(resultTy, *Constant);
+ const llvm::APSInt &rhs_I = BasicVals.Convert(conversionType,
+ *Constant);
rhs = nonloc::ConcreteInt(rhs_I);
}
}
OpenPOWER on IntegriCloud