From: Nico Weber Date: Thu, 13 Oct 2011 22:30:23 +0000 (+0000) Subject: Extend -Wno-sizeof-array-argument to strncpy and friends. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=cda57822327499aeb6fe606f9cf5903cffca3444;p=clang Extend -Wno-sizeof-array-argument to strncpy and friends. This finds 2 bugs in chromium and 1 in hunspell, with 0 false positives. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@141902 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index f4b4c70a4d..dabbcb4734 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -6113,7 +6113,12 @@ private: CMF_Memset, CMF_Memcpy, CMF_Memmove, - CMF_Memcmp + CMF_Memcmp, + CMF_Strncpy, + CMF_Strncmp, + CMF_Strncasecmp, + CMF_Strncat, + CMF_Strndup }; void CheckMemaccessArguments(const CallExpr *Call, CheckedMemoryFunction CMF, diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index a9d79bb70a..e95feacbab 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -390,6 +390,30 @@ bool Sema::CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall) { CMF = CMF_Memcmp; break; + case Builtin::BI__builtin_strncpy: + case Builtin::BI__builtin___strncpy_chk: + case Builtin::BIstrncpy: + CMF = CMF_Strncpy; + break; + + case Builtin::BI__builtin_strncmp: + CMF = CMF_Strncmp; + break; + + case Builtin::BI__builtin_strncasecmp: + CMF = CMF_Strncasecmp; + break; + + case Builtin::BI__builtin_strncat: + case Builtin::BIstrncat: + CMF = CMF_Strncat; + break; + + case Builtin::BI__builtin_strndup: + case Builtin::BIstrndup: + CMF = CMF_Strndup; + break; + default: if (FDecl->getLinkage() == ExternalLinkage && (!getLangOptions().CPlusPlus || FDecl->isExternC())) { @@ -401,6 +425,16 @@ bool Sema::CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall) { CMF = CMF_Memmove; else if (FnInfo->isStr("memcmp")) CMF = CMF_Memcmp; + else if (FnInfo->isStr("strncpy")) + CMF = CMF_Strncpy; + else if (FnInfo->isStr("strncmp")) + CMF = CMF_Strncmp; + else if (FnInfo->isStr("strncasecmp")) + CMF = CMF_Strncasecmp; + else if (FnInfo->isStr("strncat")) + CMF = CMF_Strncat; + else if (FnInfo->isStr("strndup")) + CMF = CMF_Strndup; } break; } @@ -2124,11 +2158,13 @@ void Sema::CheckMemaccessArguments(const CallExpr *Call, IdentifierInfo *FnName) { // It is possible to have a non-standard definition of memset. Validate // we have enough arguments, and if not, abort further checking. - if (Call->getNumArgs() < 3) + unsigned ExpectedNumArgs = (CMF == CMF_Strndup ? 2 : 3); + if (Call->getNumArgs() < ExpectedNumArgs) return; - unsigned LastArg = (CMF == CMF_Memset? 1 : 2); - const Expr *LenExpr = Call->getArg(2)->IgnoreParenImpCasts(); + unsigned LastArg = (CMF == CMF_Memset || CMF == CMF_Strndup ? 1 : 2); + unsigned LenArg = (CMF == CMF_Strndup ? 1 : 2); + const Expr *LenExpr = Call->getArg(LenArg)->IgnoreParenImpCasts(); // We have special checking when the length is a sizeof expression. QualType SizeOfArgTy = getSizeOfArgType(LenExpr); @@ -2162,6 +2198,8 @@ void Sema::CheckMemaccessArguments(const CallExpr *Call, llvm::FoldingSetNodeID DestID; Dest->Profile(DestID, Context, true); if (DestID == SizeOfArgID) { + // TODO: For strncpy() and friends, this could suggest sizeof(dst) + // over sizeof(src) as well. unsigned ActionIdx = 0; // Default is to suggest dereferencing. if (const UnaryOperator *UnaryOp = dyn_cast(Dest)) if (UnaryOp->getOpcode() == UO_AddrOf) @@ -2169,9 +2207,10 @@ void Sema::CheckMemaccessArguments(const CallExpr *Call, if (Context.getTypeSize(PointeeTy) == Context.getCharWidth()) ActionIdx = 2; // If the pointee's size is sizeof(char), // suggest an explicit length. + unsigned DestSrcSelect = (CMF == CMF_Strndup ? 1 : ArgIdx); DiagRuntimeBehavior(SizeOfArg->getExprLoc(), Dest, PDiag(diag::warn_sizeof_pointer_expr_memaccess) - << FnName << ArgIdx << ActionIdx + << FnName << DestSrcSelect << ActionIdx << Dest->getSourceRange() << SizeOfArg->getSourceRange()); break; diff --git a/test/SemaCXX/warn-memset-bad-sizeof.cpp b/test/SemaCXX/warn-memset-bad-sizeof.cpp index f4876ab442..a018223cbd 100644 --- a/test/SemaCXX/warn-memset-bad-sizeof.cpp +++ b/test/SemaCXX/warn-memset-bad-sizeof.cpp @@ -112,3 +112,28 @@ void f(int* i) { memset(i, 0, sizeof(i)); } } + +extern "C" int strncmp(const char *s1, const char *s2, unsigned n); +extern "C" int strncasecmp(const char *s1, const char *s2, unsigned n); +extern "C" char *strncpy(char *det, const char *src, unsigned n); +extern "C" char *strncat(char *dst, const char *src, unsigned n); +extern "C" char *strndup(const char *src, unsigned n); + +void strcpy_and_friends() { + const char* FOO = "<- should be an array instead"; + const char* BAR = "<- this, too"; + + strncmp(FOO, BAR, sizeof(FOO)); // \ + // expected-warning {{argument to 'sizeof' in 'strncmp' call is the same expression as the destination}} + strncasecmp(FOO, BAR, sizeof(FOO)); // \ + // expected-warning {{argument to 'sizeof' in 'strncasecmp' call is the same expression as the destination}} + + char buff[80]; + + strncpy(buff, BAR, sizeof(BAR)); // \ + // expected-warning {{argument to 'sizeof' in 'strncpy' call is the same expression as the source}} + strncat(buff, BAR, sizeof(BAR)); // \ + // expected-warning {{argument to 'sizeof' in 'strncat' call is the same expression as the source}} + strndup(FOO, sizeof(FOO)); // \ + // expected-warning {{argument to 'sizeof' in 'strndup' call is the same expression as the source}} +}