r332303 - [analyzer] Re-apply r331096 "CStringChecker: Add support for BSD strlcpy()...".
Artem Dergachev via cfe-commits
cfe-commits at lists.llvm.org
Mon May 14 15:32:24 PDT 2018
Author: dergachev
Date: Mon May 14 15:32:24 2018
New Revision: 332303
URL: http://llvm.org/viewvc/llvm-project?rev=332303&view=rev
Log:
[analyzer] Re-apply r331096 "CStringChecker: Add support for BSD strlcpy()...".
Fixed after revert in r331401.
Patch by David Carlier!
Differential Revision: https://reviews.llvm.org/D45177
Added:
cfe/trunk/test/Analysis/bsd-string.c
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp?rev=332303&r1=332302&r2=332303&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp Mon May 14 15:32:24 2018
@@ -97,14 +97,17 @@ public:
void evalStrcpy(CheckerContext &C, const CallExpr *CE) const;
void evalStrncpy(CheckerContext &C, const CallExpr *CE) const;
void evalStpcpy(CheckerContext &C, const CallExpr *CE) const;
+ void evalStrlcpy(CheckerContext &C, const CallExpr *CE) const;
void evalStrcpyCommon(CheckerContext &C,
const CallExpr *CE,
bool returnEnd,
bool isBounded,
- bool isAppending) const;
+ bool isAppending,
+ bool returnPtr = true) const;
void evalStrcat(CheckerContext &C, const CallExpr *CE) const;
void evalStrncat(CheckerContext &C, const CallExpr *CE) const;
+ void evalStrlcat(CheckerContext &C, const CallExpr *CE) const;
void evalStrcmp(CheckerContext &C, const CallExpr *CE) const;
void evalStrncmp(CheckerContext &C, const CallExpr *CE) const;
@@ -1393,6 +1396,18 @@ void CStringChecker::evalStpcpy(CheckerC
/* isAppending = */ false);
}
+void CStringChecker::evalStrlcpy(CheckerContext &C, const CallExpr *CE) const {
+ if (CE->getNumArgs() < 3)
+ return;
+
+ // char *strlcpy(char *dst, const char *src, size_t n);
+ evalStrcpyCommon(C, CE,
+ /* returnEnd = */ true,
+ /* isBounded = */ true,
+ /* isAppending = */ false,
+ /* returnPtr = */ false);
+}
+
void CStringChecker::evalStrcat(CheckerContext &C, const CallExpr *CE) const {
if (CE->getNumArgs() < 2)
return;
@@ -1415,9 +1430,21 @@ void CStringChecker::evalStrncat(Checker
/* isAppending = */ true);
}
+void CStringChecker::evalStrlcat(CheckerContext &C, const CallExpr *CE) const {
+ if (CE->getNumArgs() < 3)
+ return;
+
+ //char *strlcat(char *s1, const char *s2, size_t n);
+ evalStrcpyCommon(C, CE,
+ /* returnEnd = */ false,
+ /* isBounded = */ true,
+ /* isAppending = */ true,
+ /* returnPtr = */ false);
+}
+
void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallExpr *CE,
bool returnEnd, bool isBounded,
- bool isAppending) const {
+ bool isAppending, bool returnPtr) const {
CurrentFunctionDescription = "string copy function";
ProgramStateRef state = C.getState();
const LocationContext *LCtx = C.getLocationContext();
@@ -1455,6 +1482,11 @@ void CStringChecker::evalStrcpyCommon(Ch
SVal maxLastElementIndex = UnknownVal();
const char *boundWarning = nullptr;
+ state = CheckOverlap(C, state, isBounded ? CE->getArg(2) : CE->getArg(1), Dst, srcExpr);
+
+ if (!state)
+ return;
+
// If the function is strncpy, strncat, etc... it is bounded.
if (isBounded) {
// Get the max number of characters to copy.
@@ -1658,16 +1690,22 @@ void CStringChecker::evalStrcpyCommon(Ch
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);
+ SVal Result;
+
+ if (returnPtr) {
+ // 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.
+ Result = (returnEnd ? UnknownVal() : DstVal);
+ } else {
+ Result = finalStrLength;
+ }
assert(state);
// If the destination is a MemRegion, try to check for a buffer overflow and
// record the new string length.
if (Optional<loc::MemRegionVal> dstRegVal =
- DstVal.getAs<loc::MemRegionVal>()) {
+ DstVal.getAs<loc::MemRegionVal>()) {
QualType ptrTy = Dst->getType();
// If we have an exact value on a bounded copy, use that to check for
@@ -1675,9 +1713,9 @@ void CStringChecker::evalStrcpyCommon(Ch
if (boundWarning) {
if (Optional<NonLoc> maxLastNL = maxLastElementIndex.getAs<NonLoc>()) {
SVal maxLastElement = svalBuilder.evalBinOpLN(state, BO_Add, *dstRegVal,
- *maxLastNL, ptrTy);
+ *maxLastNL, ptrTy);
state = CheckLocation(C, state, CE->getArg(2), maxLastElement,
- boundWarning);
+ boundWarning);
if (!state)
return;
}
@@ -1686,7 +1724,7 @@ void CStringChecker::evalStrcpyCommon(Ch
// Then, if the final length is known...
if (Optional<NonLoc> knownStrLength = finalStrLength.getAs<NonLoc>()) {
SVal lastElement = svalBuilder.evalBinOpLN(state, BO_Add, *dstRegVal,
- *knownStrLength, ptrTy);
+ *knownStrLength, ptrTy);
// ...and we haven't checked the bound, we'll check the actual copy.
if (!boundWarning) {
@@ -1698,7 +1736,7 @@ void CStringChecker::evalStrcpyCommon(Ch
}
// If this is a stpcpy-style copy, the last element is the return value.
- if (returnEnd)
+ if (returnPtr && returnEnd)
Result = lastElement;
}
@@ -1710,12 +1748,12 @@ void CStringChecker::evalStrcpyCommon(Ch
// This would probably remove any existing bindings past the end of the
// string, but that's still an improvement over blank invalidation.
state = InvalidateBuffer(C, state, Dst, *dstRegVal,
- /*IsSourceBuffer*/false, nullptr);
+ /*IsSourceBuffer*/false, nullptr);
// Invalidate the source (const-invalidation without const-pointer-escaping
// the address of the top-level region).
state = InvalidateBuffer(C, state, srcExpr, srcVal, /*IsSourceBuffer*/true,
- nullptr);
+ nullptr);
// Set the C string length of the destination, if we know it.
if (isBounded && !isAppending) {
@@ -1731,12 +1769,13 @@ void CStringChecker::evalStrcpyCommon(Ch
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()) {
- Result = svalBuilder.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount());
+ if (returnPtr) {
+ // 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()) {
+ Result = svalBuilder.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount());
+ }
}
-
// Set the return value.
state = state->BindExpr(CE, LCtx, Result);
C.addTransition(state);
@@ -1759,7 +1798,7 @@ void CStringChecker::evalStrncmp(Checker
}
void CStringChecker::evalStrcasecmp(CheckerContext &C,
- const CallExpr *CE) const {
+ const CallExpr *CE) const {
if (CE->getNumArgs() < 2)
return;
@@ -1768,7 +1807,7 @@ void CStringChecker::evalStrcasecmp(Chec
}
void CStringChecker::evalStrncasecmp(CheckerContext &C,
- const CallExpr *CE) const {
+ const CallExpr *CE) const {
if (CE->getNumArgs() < 3)
return;
@@ -1777,7 +1816,7 @@ void CStringChecker::evalStrncasecmp(Che
}
void CStringChecker::evalStrcmpCommon(CheckerContext &C, const CallExpr *CE,
- bool isBounded, bool ignoreCase) const {
+ bool isBounded, bool ignoreCase) const {
CurrentFunctionDescription = "string comparison function";
ProgramStateRef state = C.getState();
const LocationContext *LCtx = C.getLocationContext();
@@ -1822,7 +1861,7 @@ void CStringChecker::evalStrcmpCommon(Ch
// and we only need to check one size.
if (StSameBuf) {
StSameBuf = StSameBuf->BindExpr(CE, LCtx,
- svalBuilder.makeZeroVal(CE->getType()));
+ svalBuilder.makeZeroVal(CE->getType()));
C.addTransition(StSameBuf);
// If the two arguments are GUARANTEED to be the same, we're done!
@@ -1841,7 +1880,7 @@ void CStringChecker::evalStrcmpCommon(Ch
const StringLiteral *s2StrLiteral = getCStringLiteral(C, state, s2, s2Val);
bool canComputeResult = false;
SVal resultVal = svalBuilder.conjureSymbolVal(nullptr, CE, LCtx,
- C.blockCount());
+ C.blockCount());
if (s1StrLiteral && s2StrLiteral) {
StringRef s1StrRef = s1StrLiteral->getString();
@@ -1876,7 +1915,7 @@ void CStringChecker::evalStrcmpCommon(Ch
// Use StringRef's comparison methods to compute the actual result.
int compareRes = ignoreCase ? s1StrRef.compare_lower(s2StrRef)
- : s1StrRef.compare(s2StrRef);
+ : s1StrRef.compare(s2StrRef);
// The strcmp function returns an integer greater than, equal to, or less
// than zero, [c11, p7.24.4.2].
@@ -1890,7 +1929,7 @@ void CStringChecker::evalStrcmpCommon(Ch
BinaryOperatorKind op = (compareRes == 1) ? BO_GT : BO_LT;
SVal compareWithZero =
svalBuilder.evalBinOp(state, op, resultVal, zeroVal,
- svalBuilder.getConditionType());
+ svalBuilder.getConditionType());
DefinedSVal compareWithZeroVal = compareWithZero.castAs<DefinedSVal>();
state = state->assume(compareWithZeroVal, true);
}
@@ -1942,17 +1981,17 @@ void CStringChecker::evalStrsep(CheckerC
// Invalidate the search string, representing the change of one delimiter
// character to NUL.
State = InvalidateBuffer(C, State, SearchStrPtr, Result,
- /*IsSourceBuffer*/false, nullptr);
+ /*IsSourceBuffer*/false, nullptr);
// Overwrite the search string pointer. The new value is either an address
// further along in the same string, or NULL if there are no more tokens.
State = State->bindLoc(*SearchStrLoc,
- SVB.conjureSymbolVal(getTag(),
- CE,
- LCtx,
- CharPtrTy,
- C.blockCount()),
- LCtx);
+ SVB.conjureSymbolVal(getTag(),
+ CE,
+ LCtx,
+ CharPtrTy,
+ C.blockCount()),
+ LCtx);
} else {
assert(SearchStrVal.isUnknown());
// Conjure a symbolic value. It's the best we can do.
@@ -1970,12 +2009,12 @@ void CStringChecker::evalStdCopy(Checker
}
void CStringChecker::evalStdCopyBackward(CheckerContext &C,
- const CallExpr *CE) const {
+ const CallExpr *CE) const {
evalStdCopyCommon(C, CE);
}
void CStringChecker::evalStdCopyCommon(CheckerContext &C,
- const CallExpr *CE) const {
+ const CallExpr *CE) const {
if (CE->getNumArgs() < 3)
return;
@@ -1992,7 +2031,7 @@ void CStringChecker::evalStdCopyCommon(C
const Expr *Dst = CE->getArg(2);
SVal DstVal = State->getSVal(Dst, LCtx);
State = InvalidateBuffer(C, State, Dst, DstVal, /*IsSource=*/false,
- /*Size=*/nullptr);
+ /*Size=*/nullptr);
SValBuilder &SVB = C.getSValBuilder();
@@ -2042,7 +2081,7 @@ void CStringChecker::evalMemset(CheckerC
if (!State)
return;
State = InvalidateBuffer(C, State, Mem, C.getSVal(Mem),
- /*IsSourceBuffer*/false, Size);
+ /*IsSourceBuffer*/false, Size);
if (!State)
return;
@@ -2091,10 +2130,14 @@ bool CStringChecker::evalCall(const Call
evalFunction = &CStringChecker::evalStrncpy;
else if (C.isCLibraryFunction(FDecl, "stpcpy"))
evalFunction = &CStringChecker::evalStpcpy;
+ else if (C.isCLibraryFunction(FDecl, "strlcpy"))
+ evalFunction = &CStringChecker::evalStrlcpy;
else if (C.isCLibraryFunction(FDecl, "strcat"))
evalFunction = &CStringChecker::evalStrcat;
else if (C.isCLibraryFunction(FDecl, "strncat"))
evalFunction = &CStringChecker::evalStrncat;
+ else if (C.isCLibraryFunction(FDecl, "strlcat"))
+ evalFunction = &CStringChecker::evalStrlcat;
else if (C.isCLibraryFunction(FDecl, "strlen"))
evalFunction = &CStringChecker::evalstrLength;
else if (C.isCLibraryFunction(FDecl, "strnlen"))
@@ -2161,7 +2204,7 @@ void CStringChecker::checkPreStmt(const
SVal StrVal = C.getSVal(Init);
assert(StrVal.isValid() && "Initializer string is unknown or undefined");
DefinedOrUnknownSVal strLength =
- getCStringLength(C, state, Init, StrVal).castAs<DefinedOrUnknownSVal>();
+ getCStringLength(C, state, Init, StrVal).castAs<DefinedOrUnknownSVal>();
state = state->set<CStringLength>(MR, strLength);
}
@@ -2171,11 +2214,11 @@ void CStringChecker::checkPreStmt(const
ProgramStateRef
CStringChecker::checkRegionChanges(ProgramStateRef state,
- const InvalidatedSymbols *,
- ArrayRef<const MemRegion *> ExplicitRegions,
- ArrayRef<const MemRegion *> Regions,
- const LocationContext *LCtx,
- const CallEvent *Call) const {
+ const InvalidatedSymbols *,
+ ArrayRef<const MemRegion *> ExplicitRegions,
+ ArrayRef<const MemRegion *> Regions,
+ const LocationContext *LCtx,
+ const CallEvent *Call) const {
CStringLengthTy Entries = state->get<CStringLength>();
if (Entries.isEmpty())
return state;
@@ -2185,7 +2228,7 @@ CStringChecker::checkRegionChanges(Progr
// First build sets for the changed regions and their super-regions.
for (ArrayRef<const MemRegion *>::iterator
- I = Regions.begin(), E = Regions.end(); I != E; ++I) {
+ I = Regions.begin(), E = Regions.end(); I != E; ++I) {
const MemRegion *MR = *I;
Invalidated.insert(MR);
@@ -2200,7 +2243,7 @@ CStringChecker::checkRegionChanges(Progr
// Then loop over the entries in the current state.
for (CStringLengthTy::iterator I = Entries.begin(),
- E = Entries.end(); I != E; ++I) {
+ E = Entries.end(); I != E; ++I) {
const MemRegion *MR = I.getKey();
// Is this entry for a super-region of a changed region?
@@ -2224,22 +2267,22 @@ CStringChecker::checkRegionChanges(Progr
}
void CStringChecker::checkLiveSymbols(ProgramStateRef state,
- SymbolReaper &SR) const {
+ SymbolReaper &SR) const {
// Mark all symbols in our string length map as valid.
CStringLengthTy Entries = state->get<CStringLength>();
for (CStringLengthTy::iterator I = Entries.begin(), E = Entries.end();
- I != E; ++I) {
+ I != E; ++I) {
SVal Len = I.getData();
for (SymExpr::symbol_iterator si = Len.symbol_begin(),
- se = Len.symbol_end(); si != se; ++si)
+ se = Len.symbol_end(); si != se; ++si)
SR.markInUse(*si);
}
}
void CStringChecker::checkDeadSymbols(SymbolReaper &SR,
- CheckerContext &C) const {
+ CheckerContext &C) const {
if (!SR.hasDeadSymbols())
return;
@@ -2250,7 +2293,7 @@ void CStringChecker::checkDeadSymbols(Sy
CStringLengthTy::Factory &F = state->get_context<CStringLength>();
for (CStringLengthTy::iterator I = Entries.begin(), E = Entries.end();
- I != E; ++I) {
+ I != E; ++I) {
SVal Len = I.getData();
if (SymbolRef Sym = Len.getAsSymbol()) {
if (SR.isDead(Sym))
@@ -2269,11 +2312,11 @@ void CStringChecker::checkDeadSymbols(Sy
checker->Filter.CheckName##name = mgr.getCurrentCheckName(); \
}
-REGISTER_CHECKER(CStringNullArg)
-REGISTER_CHECKER(CStringOutOfBounds)
-REGISTER_CHECKER(CStringBufferOverlap)
+ REGISTER_CHECKER(CStringNullArg)
+ REGISTER_CHECKER(CStringOutOfBounds)
+ REGISTER_CHECKER(CStringBufferOverlap)
REGISTER_CHECKER(CStringNotNullTerm)
-void ento::registerCStringCheckerBasic(CheckerManager &Mgr) {
- registerCStringNullArg(Mgr);
-}
+ void ento::registerCStringCheckerBasic(CheckerManager &Mgr) {
+ registerCStringNullArg(Mgr);
+ }
Added: cfe/trunk/test/Analysis/bsd-string.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/bsd-string.c?rev=332303&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/bsd-string.c (added)
+++ cfe/trunk/test/Analysis/bsd-string.c Mon May 14 15:32:24 2018
@@ -0,0 +1,40 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.cstring,alpha.unix.cstring,debug.ExprInspection -analyzer-store=region -verify %s
+
+#define NULL ((void *)0)
+
+typedef __typeof(sizeof(int)) size_t;
+size_t strlcpy(char *dst, const char *src, size_t n);
+size_t strlcat(char *dst, const char *src, size_t n);
+void clang_analyzer_eval(int);
+
+void f1() {
+ char overlap[] = "123456789";
+ strlcpy(overlap, overlap + 1, 3); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void f2() {
+ char buf[5];
+ strlcpy(buf, "abcd", sizeof(buf)); // expected-no-warning
+ strlcat(buf, "efgh", sizeof(buf)); // expected-warning{{Size argument is greater than the free space in the destination buffer}}
+}
+
+void f3() {
+ char dst[2];
+ const char *src = "abdef";
+ strlcpy(dst, src, 5); // expected-warning{{Size argument is greater than the length of the destination buffer}}
+}
+
+void f4() {
+ strlcpy(NULL, "abcdef", 6); // expected-warning{{Null pointer argument in call to string copy function}}
+}
+
+void f5() {
+ strlcat(NULL, "abcdef", 6); // expected-warning{{Null pointer argument in call to string copy function}}
+}
+
+void f6() {
+ char buf[8];
+ strlcpy(buf, "abc", 3);
+ size_t len = strlcat(buf, "defg", 4);
+ clang_analyzer_eval(len == 7); // expected-warning{{TRUE}}
+}
More information about the cfe-commits
mailing list