Author: george.karpenkov Date: Wed Aug 22 17:02:35 2018 New Revision: 340501
URL: http://llvm.org/viewvc/llvm-project?rev=340501&view=rev Log: Revert "[CStringSyntaxChecker] Check strlcat sizeof check" This reverts commit 3073790e87378fea9a68fb052185fec9596ef135. The check is not correct, strlact(dest, "mystr", sizeof(dest)) is fine. Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp cfe/trunk/test/Analysis/cstring-syntax.c Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp?rev=340501&r1=340500&r2=340501&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp Wed Aug 22 17:02:35 2018 @@ -90,16 +90,7 @@ class WalkAST: public StmtVisitor<WalkAS /// strlcpy(dst, "abcd", 4); /// strlcpy(dst + 3, "abcd", 2); /// strlcpy(dst, "abcd", cpy); - /// Identify erroneous patterns in the last argument to strlcat - the number - /// of bytes to copy. - /// The bad pattern checked is when the last argument is basically - /// pointing to the destination buffer size or argument larger or - /// equal to. - /// char dst[2]; - /// strlcat(dst, src2, sizeof(dst)); - /// strlcat(dst, src2, 2); - /// strlcat(dst, src2, 10); - bool containsBadStrlcpyStrlcatPattern(const CallExpr *CE); + bool containsBadStrlcpyPattern(const CallExpr *CE); public: WalkAST(const CheckerBase *Checker, BugReporter &BR, AnalysisDeclContext *AC) @@ -151,21 +142,15 @@ bool WalkAST::containsBadStrncatPattern( return false; } -bool WalkAST::containsBadStrlcpyStrlcatPattern(const CallExpr *CE) { +bool WalkAST::containsBadStrlcpyPattern(const CallExpr *CE) { if (CE->getNumArgs() != 3) return false; - const FunctionDecl *FD = CE->getDirectCallee(); - bool Append = CheckerContext::isCLibraryFunction(FD, "strlcat"); const Expr *DstArg = CE->getArg(0); const Expr *LenArg = CE->getArg(2); const auto *DstArgDecl = dyn_cast<DeclRefExpr>(DstArg->IgnoreParenImpCasts()); const auto *LenArgDecl = dyn_cast<DeclRefExpr>(LenArg->IgnoreParenLValueCasts()); uint64_t DstOff = 0; - // - sizeof(dst) - // strlcat appends at most size - strlen(dst) - 1 - if (Append && isSizeof(LenArg, DstArg)) - return true; // - size_t dstlen = sizeof(dst) if (LenArgDecl) { const auto *LenArgVal = dyn_cast<VarDecl>(LenArgDecl->getDecl()); @@ -196,10 +181,7 @@ bool WalkAST::containsBadStrlcpyStrlcatP if (const auto *Buffer = dyn_cast<ConstantArrayType>(DstArgDecl->getType())) { ASTContext &C = BR.getContext(); uint64_t BufferLen = C.getTypeSize(Buffer) / 8; - auto RemainingBufferLen = BufferLen - DstOff; - if (Append) - RemainingBufferLen -= 1; - if (RemainingBufferLen < ILRawVal) + if ((BufferLen - DstOff) < ILRawVal) return true; } } @@ -238,7 +220,7 @@ void WalkAST::VisitCallExpr(CallExpr *CE LenArg->getSourceRange()); } } else if (CheckerContext::isCLibraryFunction(FD, "strlcpy")) { - if (containsBadStrlcpyStrlcatPattern(CE)) { + if (containsBadStrlcpyPattern(CE)) { const Expr *DstArg = CE->getArg(0); const Expr *LenArg = CE->getArg(2); PathDiagnosticLocation Loc = @@ -254,34 +236,6 @@ void WalkAST::VisitCallExpr(CallExpr *CE BR.EmitBasicReport(FD, Checker, "Anti-pattern in the argument", "C String API", os.str(), Loc, - LenArg->getSourceRange()); - } - } else if (CheckerContext::isCLibraryFunction(FD, "strlcat")) { - if (containsBadStrlcpyStrlcatPattern(CE)) { - const Expr *DstArg = CE->getArg(0); - const Expr *LenArg = CE->getArg(2); - PathDiagnosticLocation Loc = - PathDiagnosticLocation::createBegin(LenArg, BR.getSourceManager(), AC); - - StringRef DstName = getPrintableName(DstArg); - StringRef LenName = getPrintableName(LenArg); - - SmallString<256> S; - llvm::raw_svector_ostream os(S); - os << "The third argument allows to potentially copy more bytes than it should. "; - os << "Replace with the value "; - if (!LenName.empty()) - os << "'" << LenName << "'"; - else - os << " <size> "; - if (!DstName.empty()) - os << " - strlen(" << DstName << ")"; - else - os << " - strlen(<destination buffer>)"; - os << " - 1 or lower"; - - BR.EmitBasicReport(FD, Checker, "Anti-pattern in the argument", - "C String API", os.str(), Loc, LenArg->getSourceRange()); } } Modified: cfe/trunk/test/Analysis/cstring-syntax.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cstring-syntax.c?rev=340501&r1=340500&r2=340501&view=diff ============================================================================== --- cfe/trunk/test/Analysis/cstring-syntax.c (original) +++ cfe/trunk/test/Analysis/cstring-syntax.c Wed Aug 22 17:02:35 2018 @@ -7,7 +7,6 @@ typedef __SIZE_TYPE__ size_t; char *strncat(char *, const char *, size_t); size_t strlen (const char *s); size_t strlcpy(char *, const char *, size_t); -size_t strlcat(char *, const char *, size_t); void testStrncat(const char *src) { char dest[10]; @@ -34,19 +33,3 @@ void testStrlcpy(const char *src) { strlcpy(dest + 5, src, 5); strlcpy(dest + 5, src, 10); // expected-warning {{The third argument is larger than the size of the input buffer.}} } - -void testStrlcat(const char *src) { - char dest[10]; - size_t badlen = 10; - size_t ulen; - strlcpy(dest, "aaaaa", sizeof("aaaaa") - 1); - strlcat(dest, "bbbb", (sizeof("bbbb") - 1) - sizeof(dest) - 1); - strlcpy(dest, "012345678", sizeof(dest)); - strlcat(dest, "910", sizeof(dest)); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value <size> - strlen(dest) - 1 or lower}} - strlcpy(dest, "0123456789", sizeof(dest)); - strlcat(dest, "0123456789", badlen); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value 'badlen' - strlen(dest) - 1 or lower}} - strlcat(dest, "0123456789", badlen - strlen(dest) - 1); - strlcat(dest, src, ulen); - strlcpy(dest, src, 5); - strlcat(dest + 5, src, badlen); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value 'badlen' - strlen(<destination buffer>) - 1 or lower}} -} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
