baloghadamsoftware created this revision.
baloghadamsoftware added a reviewer: dcoughlin.
baloghadamsoftware added subscribers: cfe-commits, xazax.hun, o.gyorgy.
The strdup family was only partially handled in the original checker. As a
consequence it did not recognize leaks where a variable which received a result
of an strdup was later overwritten. (The unix.cstring checkers did not
recognize this leak either, but their task is also different.) This patch fixes
this issue by simulating the memory allocation of strdup functions. Some new
functions where taken from the unix.cstring checkers but they were also
simplified and adapted for the purpose of the checker. Some other tests had to
be fixed as well since they contained strdup caused memory leaks. (The leak was
not fixed but the warning is now expected.)
http://reviews.llvm.org/D20863
Files:
lib/StaticAnalyzer/Checkers/MallocChecker.cpp
test/Analysis/malloc.c
test/Analysis/pr22954.c
test/Analysis/unions.cpp
Index: test/Analysis/unions.cpp
===================================================================
--- test/Analysis/unions.cpp
+++ test/Analysis/unions.cpp
@@ -93,7 +93,7 @@
// FIXME: This is a leak of uu.s.
uu = vv;
- }
+ } // expected-warning{{leak}}
void testIndirectInvalidation() {
IntOrString uu;
Index: test/Analysis/pr22954.c
===================================================================
--- test/Analysis/pr22954.c
+++ test/Analysis/pr22954.c
@@ -585,7 +585,8 @@
m28[j].s3[k] = 1;
struct ll * l28 = (struct ll*)(&m28[1]);
l28->s1[l] = 2;
- char input[] = {'a', 'b', 'c', 'd'};
+ char input[] = {'a', 'b', 'c', 'd'}; // \
+ expected-warning{{Potential leak of memory pointed to by field 's4'}}
memcpy(l28->s1, input, 4);
clang_analyzer_eval(m28[0].s3[0] == 1); // expected-warning{{UNKNOWN}}
clang_analyzer_eval(m28[0].s3[1] == 1); // expected-warning{{UNKNOWN}}
Index: test/Analysis/malloc.c
===================================================================
--- test/Analysis/malloc.c
+++ test/Analysis/malloc.c
@@ -1106,54 +1106,70 @@
}
// Test various allocation/deallocation functions.
-void testStrdup(const char *s, unsigned validIndex) {
+void testStrdup(const char *s) {
char *s2 = strdup(s);
- s2[validIndex + 1] = 'b';
} // expected-warning {{Potential leak of memory pointed to by}}
-void testWinStrdup(const char *s, unsigned validIndex) {
+void testWinStrdup(const char *s) {
char *s2 = _strdup(s);
- s2[validIndex + 1] = 'b';
} // expected-warning {{Potential leak of memory pointed to by}}
-void testWcsdup(const wchar_t *s, unsigned validIndex) {
+void testWcsdup(const wchar_t *s) {
wchar_t *s2 = wcsdup(s);
- s2[validIndex + 1] = 'b';
} // expected-warning {{Potential leak of memory pointed to by}}
-void testWinWcsdup(const wchar_t *s, unsigned validIndex) {
+void testWinWcsdup(const wchar_t *s) {
wchar_t *s2 = _wcsdup(s);
- s2[validIndex + 1] = 'b';
} // expected-warning {{Potential leak of memory pointed to by}}
-int testStrndup(const char *s, unsigned validIndex, unsigned size) {
+void testStrndup(const char *s, unsigned size) {
char *s2 = strndup(s, size);
- s2 [validIndex + 1] = 'b';
- if (s2[validIndex] != 'a')
- return 0;
- else
- return 1;// expected-warning {{Potential leak of memory pointed to by}}
-}
+} // expected-warning {{Potential leak of memory pointed to by}}
+
+void testStrdupOverwritten(const char *s) {
+ char *s2 = strdup(s);
+ s2 = 0;
+} // expected-warning {{Potential leak of memory pointed to by}}
+
+void testWinStrdupOverwritten(const char *s) {
+ char *s2 = _strdup(s);
+ s2 = 0;
+} // expected-warning {{Potential leak of memory pointed to by}}
+
+void testWcsdupOverwritten(const wchar_t *s) {
+ wchar_t *s2 = wcsdup(s);
+ s2 = 0;
+} // expected-warning {{Potential leak of memory pointed to by}}
+
+void testWinWcsdupOverwritten(const wchar_t *s) {
+ wchar_t *s2 = _wcsdup(s);
+ s2 = 0;
+} // expected-warning {{Potential leak of memory pointed to by}}
+
+void testStrndupOverwritten(const char *s, unsigned size) {
+ char *s2 = strndup(s, size);
+ s2 = 0;
+} // expected-warning {{Potential leak of memory pointed to by}}
-void testStrdupContentIsDefined(const char *s, unsigned validIndex) {
+void testStrdupContentIsDefined(const char *s) {
char *s2 = strdup(s);
char result = s2[1];// no warning
free(s2);
}
-void testWinStrdupContentIsDefined(const char *s, unsigned validIndex) {
+void testWinStrdupContentIsDefined(const char *s) {
char *s2 = _strdup(s);
char result = s2[1];// no warning
free(s2);
}
-void testWcsdupContentIsDefined(const wchar_t *s, unsigned validIndex) {
+void testWcsdupContentIsDefined(const wchar_t *s) {
wchar_t *s2 = wcsdup(s);
wchar_t result = s2[1];// no warning
free(s2);
}
-void testWinWcsdupContentIsDefined(const wchar_t *s, unsigned validIndex) {
+void testWinWcsdupContentIsDefined(const wchar_t *s) {
wchar_t *s2 = _wcsdup(s);
wchar_t result = s2[1];// no warning
free(s2);
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -198,6 +198,8 @@
DefaultBool ChecksEnabled[CK_NumCheckKinds];
CheckName CheckNames[CK_NumCheckKinds];
+ static void *getTag() { static int tag; return &tag; }
+
void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
void checkPostStmt(const CallExpr *CE, CheckerContext &C) const;
void checkPostStmt(const CXXNewExpr *NE, CheckerContext &C) const;
@@ -320,6 +322,18 @@
static ProgramStateRef CallocMem(CheckerContext &C, const CallExpr *CE,
ProgramStateRef State);
+ static ProgramStateRef DuplicateString(CheckerContext &C, const CallExpr *CE,
+ ProgramStateRef State);
+ static ProgramStateRef DuplicateStringBounded(CheckerContext &C,
+ const CallExpr *CE,
+ ProgramStateRef State);
+ static SVal copyCString(CheckerContext &C, ProgramStateRef &State, SVal Buf);
+ static SVal getCStringLength(CheckerContext &C, ProgramStateRef &State,
+ const Expr *Ex, SVal Buf);
+ static SVal getCStringLengthForRegion(CheckerContext &C,
+ ProgramStateRef &State, const Expr *Ex,
+ const MemRegion *MR);
+
///\brief Check if the memory associated with this symbol was released.
bool isReleased(SymbolRef Sym, CheckerContext &C) const;
@@ -801,9 +815,15 @@
State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory);
} else if (FunI == II_strdup || FunI == II_win_strdup ||
FunI == II_wcsdup || FunI == II_win_wcsdup) {
- State = MallocUpdateRefState(C, CE, State);
+ if (CE->getNumArgs() < 1)
+ return;
+ State = DuplicateString(C, CE, State);
+ State = ProcessZeroAllocation(C, CE, 0, State);
} else if (FunI == II_strndup) {
- State = MallocUpdateRefState(C, CE, State);
+ if (CE->getNumArgs() < 2)
+ return;
+ State = DuplicateStringBounded(C, CE, State);
+ State = ProcessZeroAllocation(C, CE, 0, State);
} else if (FunI == II_alloca || FunI == II_win_alloca) {
State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State,
AF_Alloca);
@@ -2013,6 +2033,121 @@
return MallocMemAux(C, CE, TotalSize, zeroVal, State);
}
+ProgramStateRef MallocChecker::DuplicateString(CheckerContext &C,
+ const CallExpr *CE,
+ ProgramStateRef State) {
+ const auto *sourceEx = CE->getArg(0);
+ auto sourceVal = State->getSVal(sourceEx, C.getLocationContext());
+ return MallocMemAux(C, CE, getCStringLength(C, State, sourceEx, sourceVal),
+ copyCString(C, State, sourceVal), State);
+}
+
+ProgramStateRef MallocChecker::DuplicateStringBounded(CheckerContext &C,
+ const CallExpr *CE,
+ ProgramStateRef State) {
+ const auto *sourceEx = CE->getArg(0);
+ auto sourceVal = State->getSVal(sourceEx, C.getLocationContext());
+ auto strLen = getCStringLength(C, State, sourceEx, sourceVal);
+
+ SValBuilder &svalBuilder = C.getSValBuilder();
+ QualType cmpTy = svalBuilder.getConditionType();
+ QualType sizeTy = svalBuilder.getContext().getSizeType();
+
+ const auto *lenEx = CE->getArg(1);
+ auto lenVal = State->getSVal(lenEx, C.getLocationContext());
+ lenVal = svalBuilder.evalCast(lenVal, sizeTy, lenEx->getType());
+
+ Optional<NonLoc> strLenNL = strLen.getAs<NonLoc>();
+ Optional<NonLoc> lenValNL = lenVal.getAs<NonLoc>();
+
+ SVal destLength = UnknownVal();
+
+ if (strLenNL && lenValNL) {
+ ProgramStateRef stateLenVal, stateStrLen;
+
+ std::tie(stateLenVal, stateStrLen) = State->assume(
+ svalBuilder.evalBinOpNN(State, BO_GE, *strLenNL, *lenValNL, cmpTy)
+ .castAs<DefinedOrUnknownSVal>());
+
+ if (stateLenVal && !stateStrLen) {
+ State = stateLenVal;
+ destLength = lenVal;
+ } else if (!stateLenVal && stateStrLen) {
+ State = stateStrLen;
+ destLength = strLen;
+ }
+ }
+ return MallocMemAux(C, CE, destLength, copyCString(C, State, sourceVal),
+ State);
+}
+
+SVal MallocChecker::copyCString(CheckerContext &C, ProgramStateRef &State,
+ SVal Buf) {
+ const auto *MR = Buf.getAsRegion();
+ if (!MR)
+ return UnknownVal();
+
+ MR = MR->StripCasts();
+
+ if (const auto *TVR = MR->getAs<TypedValueRegion>()) {
+ auto &svalBuilder = C.getSValBuilder();
+ const auto store = State->getStore();
+ auto &storeMgr = State->getStateManager().getStoreManager();
+ return svalBuilder.makeLazyCompoundVal(StoreRef(store, storeMgr), TVR);
+ } else {
+ return UnknownVal();
+ }
+}
+
+SVal MallocChecker::getCStringLength(CheckerContext &C, ProgramStateRef &State,
+ const Expr *Ex, SVal Buf) {
+ const auto *MR = Buf.getAsRegion();
+ if (!MR)
+ return UnknownVal();
+
+ MR = MR->StripCasts();
+
+ switch (MR->getKind()) {
+ case MemRegion::StringRegionKind: {
+ auto &svalBuilder = C.getSValBuilder();
+ auto sizeTy = svalBuilder.getContext().getSizeType();
+ const auto *strLit = cast<StringRegion>(MR)->getStringLiteral();
+ return svalBuilder.makeIntVal(strLit->getByteLength(), sizeTy);
+ }
+ case MemRegion::SymbolicRegionKind:
+ case MemRegion::AllocaRegionKind:
+ case MemRegion::VarRegionKind:
+ case MemRegion::FieldRegionKind:
+ case MemRegion::ObjCIvarRegionKind:
+ return getCStringLengthForRegion(C, State, Ex, MR);
+ default:
+ return UnknownVal();
+ }
+}
+
+SVal MallocChecker::getCStringLengthForRegion(CheckerContext &C,
+ ProgramStateRef &State,
+ const Expr *Ex,
+ const MemRegion *MR) {
+ auto &svalBuilder = C.getSValBuilder();
+ auto sizeTy = svalBuilder.getContext().getSizeType();
+ auto strLength = svalBuilder.getMetadataSymbolVal(MallocChecker::getTag(), MR,
+ Ex, sizeTy, C.blockCount());
+ if (auto strLn = strLength.getAs<NonLoc>()) {
+ // In case of unbounded calls strlen etc bound the range to SIZE_MAX/4
+ auto &BVF = svalBuilder.getBasicValueFactory();
+ const auto &maxValInt = BVF.getMaxValue(sizeTy);
+ auto fourInt = APSIntType(maxValInt).getValue(4);
+ const auto *maxLengthInt = BVF.evalAPSInt(BO_Div, maxValInt, fourInt);
+ auto maxLength = svalBuilder.makeIntVal(*maxLengthInt);
+ auto evalLength =
+ svalBuilder.evalBinOpNN(State, BO_LE, *strLn, maxLength, sizeTy);
+ State = State->assume(evalLength.castAs<DefinedOrUnknownSVal>(), true);
+ }
+
+ return strLength;
+}
+
LeakInfo
MallocChecker::getAllocationSite(const ExplodedNode *N, SymbolRef Sym,
CheckerContext &C) const {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
