Author: dergachev Date: Mon Jul 1 16:02:10 2019 New Revision: 364868 URL: http://llvm.org/viewvc/llvm-project?rev=364868&view=rev Log: [analyzer] CStringChecker: Modernize to use CallDescriptions.
This patch uses the new CDF_MaybeBuiltin flag to handle C library functions. It's mostly an NFC/refactoring pass, but it does fix a bug in handling memset() when it expands to __builtin___memset_chk() because the latter has one more argument and memset() handling code was trying to match the exact number of arguments. Now the code is deduplicated and there's less room for mistakes. Differential Revision: https://reviews.llvm.org/D62557 Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp cfe/trunk/test/Analysis/string.c Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp?rev=364868&r1=364867&r2=364868&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp Mon Jul 1 16:02:10 2019 @@ -73,7 +73,38 @@ public: typedef void (CStringChecker::*FnCheck)(CheckerContext &, const CallExpr *) const; + CallDescriptionMap<FnCheck> 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::evalCopyCommon(Chec 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::evalMemcpy(CheckerC } 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::evalMempcpy(Checker } 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::evalMemmove(Checker } 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::evalMemcmp(CheckerC 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::evalstrLengthCommon } 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::evalStrcpy(CheckerC } 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::evalStrncpy(Checker } 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::evalStpcpy(CheckerC } 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::evalStrlcpy(Checker } 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::evalStrcat(CheckerC } 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::evalStrncat(Checker } 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::evalStrcpyCommon(Ch } 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::evalStrcmpCommon(Ch 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::evalStdCopyBackward 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::evalStdCopyCommon(C } 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::evalMemset(CheckerC } 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 @@ void CStringChecker::evalBzero(CheckerCo 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<CallExpr>(Call.getOriginExpr()); + if (!CE) + return nullptr; + + const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(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<CallExpr>(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<CallExpr>(Call.getOriginExpr()); + (this->*Callback)(C, CE); // If the evaluate call resulted in no change, chain to the next eval call // handler. Modified: cfe/trunk/test/Analysis/string.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/string.c?rev=364868&r1=364867&r2=364868&view=diff ============================================================================== --- cfe/trunk/test/Analysis/string.c (original) +++ cfe/trunk/test/Analysis/string.c Mon Jul 1 16:02:10 2019 @@ -1598,3 +1598,9 @@ void memset29_plain_int_zero() { 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}} +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
