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 f4b4c70..dabbcb4 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -6113,7 +6113,12 @@
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 a9d79bb..e95feac 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -390,6 +390,30 @@
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 @@
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 @@
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 @@
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<UnaryOperator>(Dest))
if (UnaryOp->getOpcode() == UO_AddrOf)
@@ -2169,9 +2207,10 @@
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 f4876ab..a018223 100644
--- a/test/SemaCXX/warn-memset-bad-sizeof.cpp
+++ b/test/SemaCXX/warn-memset-bad-sizeof.cpp
@@ -112,3 +112,28 @@
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}}
+}
