Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp @@ -73,7 +73,38 @@ typedef void (CStringChecker::*FnCheck)(CheckerContext &, const CallExpr *) const; + CallDescriptionMap Callbacks = { + {{CDF_MaybeBuiltin, "memcpy", 3}, &CStringChecker::evalMemcpy}, + {{CDF_MaybeBuiltin, "mempcpy", 3}, &CStringChecker::evalMempcpy}, + {{CDF_MaybeBuiltin, "memcmp", 3}, &CStringChecker::evalMemcmp}, + {{CDF_MaybeBuiltin, "memmove", 3}, &CStringChecker::evalMemmove}, + {{CDF_MaybeBuiltin, "memset", 3}, &CStringChecker::evalMemset}, + {{CDF_MaybeBuiltin, "explicit_memset", 3}, &CStringChecker::evalMemset}, + {{CDF_MaybeBuiltin, "strcpy", 2}, &CStringChecker::evalStrcpy}, + {{CDF_MaybeBuiltin, "strncpy", 3}, &CStringChecker::evalStrncpy}, + {{CDF_MaybeBuiltin, "stpcpy", 2}, &CStringChecker::evalStpcpy}, + {{CDF_MaybeBuiltin, "strlcpy", 3}, &CStringChecker::evalStrlcpy}, + {{CDF_MaybeBuiltin, "strcat", 2}, &CStringChecker::evalStrcat}, + {{CDF_MaybeBuiltin, "strncat", 3}, &CStringChecker::evalStrncat}, + {{CDF_MaybeBuiltin, "strlcat", 3}, &CStringChecker::evalStrlcat}, + {{CDF_MaybeBuiltin, "strlen", 1}, &CStringChecker::evalstrLength}, + {{CDF_MaybeBuiltin, "strnlen", 2}, &CStringChecker::evalstrnLength}, + {{CDF_MaybeBuiltin, "strcmp", 2}, &CStringChecker::evalStrcmp}, + {{CDF_MaybeBuiltin, "strncmp", 3}, &CStringChecker::evalStrncmp}, + {{CDF_MaybeBuiltin, "strcasecmp", 2}, &CStringChecker::evalStrcasecmp}, + {{CDF_MaybeBuiltin, "strncasecmp", 3}, &CStringChecker::evalStrncasecmp}, + {{CDF_MaybeBuiltin, "strsep", 2}, &CStringChecker::evalStrsep}, + {{CDF_MaybeBuiltin, "bcopy", 3}, &CStringChecker::evalBcopy}, + {{CDF_MaybeBuiltin, "bcmp", 3}, &CStringChecker::evalMemcmp}, + {{CDF_MaybeBuiltin, "bzero", 2}, &CStringChecker::evalBzero}, + {{CDF_MaybeBuiltin, "explicit_bzero", 2}, &CStringChecker::evalBzero}, + }; + + // These require a bit of special handling. + CallDescription StdCopy{{"std", "copy"}, 3}, + StdCopyBackward{{"std", "copy_backward"}, 3}; + FnCheck identifyCall(const CallEvent &Call, CheckerContext &C) const; void evalMemcpy(CheckerContext &C, const CallExpr *CE) const; void evalMempcpy(CheckerContext &C, const CallExpr *CE) const; void evalMemmove(CheckerContext &C, const CallExpr *CE) const; @@ -1201,9 +1232,6 @@ void CStringChecker::evalMemcpy(CheckerContext &C, const CallExpr *CE) const { - if (CE->getNumArgs() < 3) - return; - // void *memcpy(void *restrict dst, const void *restrict src, size_t n); // The return value is the address of the destination buffer. const Expr *Dest = CE->getArg(0); @@ -1213,9 +1241,6 @@ } void CStringChecker::evalMempcpy(CheckerContext &C, const CallExpr *CE) const { - if (CE->getNumArgs() < 3) - return; - // void *mempcpy(void *restrict dst, const void *restrict src, size_t n); // The return value is a pointer to the byte following the last written byte. const Expr *Dest = CE->getArg(0); @@ -1225,9 +1250,6 @@ } void CStringChecker::evalMemmove(CheckerContext &C, const CallExpr *CE) const { - if (CE->getNumArgs() < 3) - return; - // void *memmove(void *dst, const void *src, size_t n); // The return value is the address of the destination buffer. const Expr *Dest = CE->getArg(0); @@ -1237,18 +1259,12 @@ } void CStringChecker::evalBcopy(CheckerContext &C, const CallExpr *CE) const { - if (CE->getNumArgs() < 3) - return; - // void bcopy(const void *src, void *dst, size_t n); evalCopyCommon(C, CE, C.getState(), CE->getArg(2), CE->getArg(1), CE->getArg(0)); } void CStringChecker::evalMemcmp(CheckerContext &C, const CallExpr *CE) const { - if (CE->getNumArgs() < 3) - return; - // int memcmp(const void *s1, const void *s2, size_t n); CurrentFunctionDescription = "memory comparison function"; @@ -1323,18 +1339,12 @@ void CStringChecker::evalstrLength(CheckerContext &C, const CallExpr *CE) const { - if (CE->getNumArgs() < 1) - return; - // size_t strlen(const char *s); evalstrLengthCommon(C, CE, /* IsStrnlen = */ false); } void CStringChecker::evalstrnLength(CheckerContext &C, const CallExpr *CE) const { - if (CE->getNumArgs() < 2) - return; - // size_t strnlen(const char *s, size_t maxlen); evalstrLengthCommon(C, CE, /* IsStrnlen = */ true); } @@ -1459,9 +1469,6 @@ } void CStringChecker::evalStrcpy(CheckerContext &C, const CallExpr *CE) const { - if (CE->getNumArgs() < 2) - return; - // char *strcpy(char *restrict dst, const char *restrict src); evalStrcpyCommon(C, CE, /* returnEnd = */ false, @@ -1470,9 +1477,6 @@ } void CStringChecker::evalStrncpy(CheckerContext &C, const CallExpr *CE) const { - if (CE->getNumArgs() < 3) - return; - // char *strncpy(char *restrict dst, const char *restrict src, size_t n); evalStrcpyCommon(C, CE, /* returnEnd = */ false, @@ -1481,9 +1485,6 @@ } void CStringChecker::evalStpcpy(CheckerContext &C, const CallExpr *CE) const { - if (CE->getNumArgs() < 2) - return; - // char *stpcpy(char *restrict dst, const char *restrict src); evalStrcpyCommon(C, CE, /* returnEnd = */ true, @@ -1492,9 +1493,6 @@ } 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, @@ -1504,9 +1502,6 @@ } void CStringChecker::evalStrcat(CheckerContext &C, const CallExpr *CE) const { - if (CE->getNumArgs() < 2) - return; - //char *strcat(char *restrict s1, const char *restrict s2); evalStrcpyCommon(C, CE, /* returnEnd = */ false, @@ -1515,9 +1510,6 @@ } void CStringChecker::evalStrncat(CheckerContext &C, const CallExpr *CE) const { - if (CE->getNumArgs() < 3) - return; - //char *strncat(char *restrict s1, const char *restrict s2, size_t n); evalStrcpyCommon(C, CE, /* returnEnd = */ false, @@ -1526,9 +1518,6 @@ } void CStringChecker::evalStrlcat(CheckerContext &C, const CallExpr *CE) const { - if (CE->getNumArgs() < 3) - return; - // FIXME: strlcat() uses a different rule for bound checking, i.e. 'n' means // a different thing as compared to strncat(). This currently causes // false positives in the alpha string bound checker. @@ -1885,35 +1874,23 @@ } void CStringChecker::evalStrcmp(CheckerContext &C, const CallExpr *CE) const { - if (CE->getNumArgs() < 2) - return; - //int strcmp(const char *s1, const char *s2); evalStrcmpCommon(C, CE, /* isBounded = */ false, /* ignoreCase = */ false); } void CStringChecker::evalStrncmp(CheckerContext &C, const CallExpr *CE) const { - if (CE->getNumArgs() < 3) - return; - //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 { - if (CE->getNumArgs() < 2) - return; - //int strcasecmp(const char *s1, const char *s2); evalStrcmpCommon(C, CE, /* isBounded = */ false, /* ignoreCase = */ true); } void CStringChecker::evalStrncasecmp(CheckerContext &C, const CallExpr *CE) const { - if (CE->getNumArgs() < 3) - return; - //int strncasecmp(const char *s1, const char *s2, size_t n); evalStrcmpCommon(C, CE, /* isBounded = */ true, /* ignoreCase = */ true); } @@ -2047,9 +2024,6 @@ void CStringChecker::evalStrsep(CheckerContext &C, const CallExpr *CE) const { //char *strsep(char **stringp, const char *delim); - if (CE->getNumArgs() < 2) - return; - // Sanity: does the search string parameter match the return type? const Expr *SearchStrPtr = CE->getArg(0); QualType CharPtrTy = SearchStrPtr->getType()->getPointeeType(); @@ -2118,7 +2092,7 @@ void CStringChecker::evalStdCopyCommon(CheckerContext &C, const CallExpr *CE) const { - if (CE->getNumArgs() < 3) + if (!CE->getArg(2)->getType()->isPointerType()) return; ProgramStateRef State = C.getState(); @@ -2145,9 +2119,6 @@ } void CStringChecker::evalMemset(CheckerContext &C, const CallExpr *CE) const { - if (CE->getNumArgs() != 3) - return; - CurrentFunctionDescription = "memory set function"; const Expr *Mem = CE->getArg(0); @@ -2196,9 +2167,6 @@ } void CStringChecker::evalBzero(CheckerContext &C, const CallExpr *CE) const { - if (CE->getNumArgs() != 2) - return; - CurrentFunctionDescription = "memory clearance function"; const Expr *Mem = CE->getArg(0); @@ -2241,110 +2209,53 @@ C.addTransition(State); } -static bool isCPPStdLibraryFunction(const FunctionDecl *FD, StringRef Name) { - IdentifierInfo *II = FD->getIdentifier(); - if (!II) - return false; - - if (!AnalysisDeclContext::isInStdNamespace(FD)) - return false; - - if (II->getName().equals(Name)) - return true; - - return false; -} //===----------------------------------------------------------------------===// // The driver method, and other Checker callbacks. //===----------------------------------------------------------------------===// -static CStringChecker::FnCheck identifyCall(const CallExpr *CE, - CheckerContext &C) { - const FunctionDecl *FDecl = C.getCalleeDecl(CE); - if (!FDecl) +CStringChecker::FnCheck CStringChecker::identifyCall(const CallEvent &Call, + CheckerContext &C) const { + const auto *CE = dyn_cast_or_null(Call.getOriginExpr()); + if (!CE) + return nullptr; + + const FunctionDecl *FD = dyn_cast_or_null(Call.getDecl()); + if (!FD) return nullptr; + if (Call.isCalled(StdCopy)) { + return &CStringChecker::evalStdCopy; + } else if (Call.isCalled(StdCopyBackward)) { + return &CStringChecker::evalStdCopyBackward; + } + // Pro-actively check that argument types are safe to do arithmetic upon. // We do not want to crash if someone accidentally passes a structure - // into, say, a C++ overload of any of these functions. - if (isCPPStdLibraryFunction(FDecl, "copy")) { - if (CE->getNumArgs() < 3 || !CE->getArg(2)->getType()->isPointerType()) - return nullptr; - return &CStringChecker::evalStdCopy; - } else if (isCPPStdLibraryFunction(FDecl, "copy_backward")) { - if (CE->getNumArgs() < 3 || !CE->getArg(2)->getType()->isPointerType()) + // into, say, a C++ overload of any of these functions. We could not check + // that for std::copy because they may have arguments of other types. + for (auto I : CE->arguments()) { + QualType T = I->getType(); + if (!T->isIntegralOrEnumerationType() && !T->isPointerType()) return nullptr; - return &CStringChecker::evalStdCopyBackward; - } else { - // An umbrella check for all C library functions. - for (auto I: CE->arguments()) { - QualType T = I->getType(); - if (!T->isIntegralOrEnumerationType() && !T->isPointerType()) - return nullptr; - } } - // FIXME: Poorly-factored string switches are slow. - if (C.isCLibraryFunction(FDecl, "memcpy")) - return &CStringChecker::evalMemcpy; - else if (C.isCLibraryFunction(FDecl, "mempcpy")) - return &CStringChecker::evalMempcpy; - else if (C.isCLibraryFunction(FDecl, "memcmp")) - return &CStringChecker::evalMemcmp; - else if (C.isCLibraryFunction(FDecl, "memmove")) - return &CStringChecker::evalMemmove; - else if (C.isCLibraryFunction(FDecl, "memset") || - C.isCLibraryFunction(FDecl, "explicit_memset")) - return &CStringChecker::evalMemset; - else if (C.isCLibraryFunction(FDecl, "strcpy")) - return &CStringChecker::evalStrcpy; - else if (C.isCLibraryFunction(FDecl, "strncpy")) - return &CStringChecker::evalStrncpy; - else if (C.isCLibraryFunction(FDecl, "stpcpy")) - return &CStringChecker::evalStpcpy; - else if (C.isCLibraryFunction(FDecl, "strlcpy")) - return &CStringChecker::evalStrlcpy; - else if (C.isCLibraryFunction(FDecl, "strcat")) - return &CStringChecker::evalStrcat; - else if (C.isCLibraryFunction(FDecl, "strncat")) - return &CStringChecker::evalStrncat; - else if (C.isCLibraryFunction(FDecl, "strlcat")) - return &CStringChecker::evalStrlcat; - else if (C.isCLibraryFunction(FDecl, "strlen")) - return &CStringChecker::evalstrLength; - else if (C.isCLibraryFunction(FDecl, "strnlen")) - return &CStringChecker::evalstrnLength; - else if (C.isCLibraryFunction(FDecl, "strcmp")) - return &CStringChecker::evalStrcmp; - else if (C.isCLibraryFunction(FDecl, "strncmp")) - return &CStringChecker::evalStrncmp; - else if (C.isCLibraryFunction(FDecl, "strcasecmp")) - return &CStringChecker::evalStrcasecmp; - else if (C.isCLibraryFunction(FDecl, "strncasecmp")) - return &CStringChecker::evalStrncasecmp; - else if (C.isCLibraryFunction(FDecl, "strsep")) - return &CStringChecker::evalStrsep; - else if (C.isCLibraryFunction(FDecl, "bcopy")) - return &CStringChecker::evalBcopy; - else if (C.isCLibraryFunction(FDecl, "bcmp")) - return &CStringChecker::evalMemcmp; - else if (C.isCLibraryFunction(FDecl, "bzero") || - C.isCLibraryFunction(FDecl, "explicit_bzero")) - return &CStringChecker::evalBzero; + const FnCheck *Callback = Callbacks.lookup(Call); + if (Callback) + return *Callback; return nullptr; } bool CStringChecker::evalCall(const CallEvent &Call, CheckerContext &C) const { - const auto *CE = dyn_cast_or_null(Call.getOriginExpr()); - FnCheck evalFunction = identifyCall(CE, C); + FnCheck Callback = identifyCall(Call, C); // If the callee isn't a string function, let another checker handle it. - if (!evalFunction) + if (!Callback) return false; // Check and evaluate the call. - (this->*evalFunction)(C, CE); + const auto *CE = cast(Call.getOriginExpr()); + (this->*Callback)(C, CE); // If the evaluate call resulted in no change, chain to the next eval call // handler. Index: clang/test/Analysis/string.c =================================================================== --- clang/test/Analysis/string.c +++ clang/test/Analysis/string.c @@ -1598,3 +1598,9 @@ memset(&x, 0, sizeof(short)); clang_analyzer_eval(x == 0); // expected-warning{{TRUE}} } + +void test_memset_chk() { + int x; + __builtin___memset_chk(&x, 0, sizeof(x), __builtin_object_size(&x, 0)); + clang_analyzer_eval(x == 0); // expected-warning{{TRUE}} +}