[cfe-commits] r137903 - in /cfe/trunk: include/clang/Basic/Builtins.def include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/Sema/SemaChecking.cpp test/Sema/warn-strlcpycat-size.c
Ted Kremenek
kremenek at apple.com
Wed Aug 17 16:40:36 PDT 2011
Author: kremenek
Date: Wed Aug 17 18:40:36 2011
New Revision: 137903
URL: http://llvm.org/viewvc/llvm-project?rev=137903&view=rev
Log:
Add experimental -Wstrlcpy-size warning that looks to see if the size argument for strlcpy/strlcat is the size of the *source*, and not the size of the *destination*. This warning is off by default (for now).
Warning logic provided by Geoff Keating.
Added:
cfe/trunk/test/Sema/warn-strlcpycat-size.c
Modified:
cfe/trunk/include/clang/Basic/Builtins.def
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaChecking.cpp
Modified: cfe/trunk/include/clang/Basic/Builtins.def
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=137903&r1=137902&r2=137903&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/Builtins.def (original)
+++ cfe/trunk/include/clang/Basic/Builtins.def Wed Aug 17 18:40:36 2011
@@ -664,6 +664,9 @@
// POSIX setjmp.h
LIBBUILTIN(_longjmp, "vJi", "fr", "setjmp.h", ALL_LANGUAGES)
LIBBUILTIN(siglongjmp, "vSJi", "fr", "setjmp.h", ALL_LANGUAGES)
+// non-standard but very common
+LIBBUILTIN(strlcpy, "zc*cC*z", "f", "string.h", ALL_LANGUAGES)
+LIBBUILTIN(strlcat, "zc*cC*z", "f", "string.h", ALL_LANGUAGES)
// id objc_msgSend(id, SEL, ...)
LIBBUILTIN(objc_msgSend, "GGH.", "f", "objc/message.h", OBJC_LANG)
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=137903&r1=137902&r2=137903&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Aug 17 18:40:36 2011
@@ -278,6 +278,13 @@
"argument to 'sizeof' in %0 call is the same pointer type %1 as the "
"%select{destination|source}2; expected %3 or an explicit length">,
InGroup<DiagGroup<"sizeof-pointer-memaccess">>;
+def warn_strlcpycat_wrong_size : Warning<
+ "size argument in %0 call appears to be size of the source; expected the size of "
+ "the destination">,
+ DefaultIgnore,
+ InGroup<DiagGroup<"strlcpy-size">>;
+def note_strlcpycat_wrong_size : Note<
+ "change size argument to be the size of the destination">;
/// main()
// static/inline main() are not errors in C, just in C++.
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=137903&r1=137902&r2=137903&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Wed Aug 17 18:40:36 2011
@@ -5973,6 +5973,9 @@
void CheckMemaccessArguments(const CallExpr *Call, CheckedMemoryFunction CMF,
IdentifierInfo *FnName);
+ void CheckStrlcpycatArguments(const CallExpr *Call,
+ IdentifierInfo *FnName);
+
void CheckReturnStackAddr(Expr *RetValExp, QualType lhsType,
SourceLocation ReturnLoc);
void CheckFloatComparison(SourceLocation loc, Expr* lex, Expr* rex);
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=137903&r1=137902&r2=137903&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Wed Aug 17 18:40:36 2011
@@ -319,7 +319,7 @@
TheCall->getCallee()->getLocStart());
}
- // Memset/memcpy/memmove/memcmp handling
+ // Builtin handling
int CMF = -1;
switch (FDecl->getBuiltinID()) {
case Builtin::BI__builtin_memset:
@@ -339,6 +339,11 @@
case Builtin::BImemmove:
CMF = CMF_Memmove;
break;
+
+ case Builtin::BIstrlcpy:
+ case Builtin::BIstrlcat:
+ CheckStrlcpycatArguments(TheCall, FnInfo);
+ break;
case Builtin::BI__builtin_memcmp:
CMF = CMF_Memcmp;
@@ -359,6 +364,7 @@
break;
}
+ // Memset/memcpy/memmove handling
if (CMF != -1)
CheckMemaccessArguments(TheCall, CheckedMemoryFunction(CMF), FnInfo);
@@ -1990,6 +1996,95 @@
}
}
+// A little helper routine: ignore addition and subtraction of integer literals.
+// This intentionally does not ignore all integer constant expressions because
+// we don't want to remove sizeof().
+static const Expr *ignoreLiteralAdditions(const Expr *Ex, ASTContext &Ctx) {
+ Ex = Ex->IgnoreParenCasts();
+
+ for (;;) {
+ const BinaryOperator * BO = dyn_cast<BinaryOperator>(Ex);
+ if (!BO || !BO->isAdditiveOp())
+ break;
+
+ const Expr *RHS = BO->getRHS()->IgnoreParenCasts();
+ const Expr *LHS = BO->getLHS()->IgnoreParenCasts();
+
+ if (isa<IntegerLiteral>(RHS))
+ Ex = LHS;
+ else if (isa<IntegerLiteral>(LHS))
+ Ex = RHS;
+ else
+ break;
+ }
+
+ return Ex;
+}
+
+// Warn if the user has made the 'size' argument to strlcpy or strlcat
+// be the size of the source, instead of the destination.
+void Sema::CheckStrlcpycatArguments(const CallExpr *Call,
+ IdentifierInfo *FnName) {
+
+ // Don't crash if the user has the wrong number of arguments
+ if (Call->getNumArgs() != 3)
+ return;
+
+ const Expr *SrcArg = ignoreLiteralAdditions(Call->getArg(1), Context);
+ const Expr *SizeArg = ignoreLiteralAdditions(Call->getArg(2), Context);
+ const Expr *CompareWithSrc = NULL;
+
+ // Look for 'strlcpy(dst, x, sizeof(x))'
+ if (const Expr *Ex = getSizeOfExprArg(SizeArg))
+ CompareWithSrc = Ex;
+ else {
+ // Look for 'strlcpy(dst, x, strlen(x))'
+ if (const CallExpr *SizeCall = dyn_cast<CallExpr>(SizeArg)) {
+ if (SizeCall->isBuiltinCall(Context) == Builtin::BIstrlen
+ && SizeCall->getNumArgs() == 1)
+ CompareWithSrc = ignoreLiteralAdditions(SizeCall->getArg(0), Context);
+ }
+ }
+
+ if (!CompareWithSrc)
+ return;
+
+ // Determine if the argument to sizeof/strlen is equal to the source
+ // argument. In principle there's all kinds of things you could do
+ // here, for instance creating an == expression and evaluating it with
+ // EvaluateAsBooleanCondition, but this uses a more direct technique:
+ const DeclRefExpr *SrcArgDRE = dyn_cast<DeclRefExpr>(SrcArg);
+ if (!SrcArgDRE)
+ return;
+
+ const DeclRefExpr *CompareWithSrcDRE = dyn_cast<DeclRefExpr>(CompareWithSrc);
+ if (!CompareWithSrcDRE ||
+ SrcArgDRE->getDecl() != CompareWithSrcDRE->getDecl())
+ return;
+
+ const Expr *OriginalSizeArg = Call->getArg(2);
+ Diag(CompareWithSrcDRE->getLocStart(), diag::warn_strlcpycat_wrong_size)
+ << OriginalSizeArg->getSourceRange() << FnName;
+
+ // Output a FIXIT hint if the destination is an array (rather than a
+ // pointer to an array). This could be enhanced to handle some
+ // pointers if we know the actual size, like if DstArg is 'array+2'
+ // we could say 'sizeof(array)-2'.
+ const Expr *DstArg = Call->getArg(0)->IgnoreParenImpCasts();
+
+ if (DstArg->getType()->isArrayType()) {
+ llvm::SmallString<128> sizeString;
+ llvm::raw_svector_ostream OS(sizeString);
+ OS << "sizeof(";
+ DstArg->printPretty(OS, Context, 0, Context.PrintingPolicy);
+ OS << ")";
+
+ Diag(OriginalSizeArg->getLocStart(), diag::note_strlcpycat_wrong_size)
+ << FixItHint::CreateReplacement(OriginalSizeArg->getSourceRange(),
+ OS.str());
+ }
+}
+
//===--- CHECK: Return Address of Stack Variable --------------------------===//
static Expr *EvalVal(Expr *E, SmallVectorImpl<DeclRefExpr *> &refVars);
Added: cfe/trunk/test/Sema/warn-strlcpycat-size.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-strlcpycat-size.c?rev=137903&view=auto
==============================================================================
--- cfe/trunk/test/Sema/warn-strlcpycat-size.c (added)
+++ cfe/trunk/test/Sema/warn-strlcpycat-size.c Wed Aug 17 18:40:36 2011
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -Wstrlcpy-size -verify -fsyntax-only %s
+
+typedef unsigned long size_t;
+size_t strlcpy (char * restrict dst, const char * restrict src, size_t size);
+size_t strlcat (char * restrict dst, const char * restrict src, size_t size);
+size_t strlen (const char *s);
+
+char s1[100];
+char s2[200];
+char * s3;
+
+struct {
+ char f1[100];
+ char f2[100][3];
+} s4, **s5;
+
+int x;
+
+void f(void)
+{
+ strlcpy(s1, s2, sizeof(s1)); // no warning
+ strlcpy(s1, s2, sizeof(s2)); // expected-warning {{size argument in 'strlcpy' call appears to be size of the source; expected the size of the destination}} expected-note {{change size argument to be the size of the destination}}
+ strlcpy(s1, s3, strlen(s3)+1); // expected-warning {{size argument in 'strlcpy' call appears to be size of the source; expected the size of the destination}} expected-note {{change size argument to be the size of the destination}}
+ strlcat(s2, s3, sizeof(s3)); // expected-warning {{size argument in 'strlcat' call appears to be size of the source; expected the size of the destination}} expected-note {{change size argument to be the size of the destination}}
+ strlcpy(s4.f1, s2, sizeof(s2)); // expected-warning {{size argument in 'strlcpy' call appears to be size of the source; expected the size of the destination}} expected-note {{change size argument to be the size of the destination}}
+ strlcpy((*s5)->f2[x], s2, sizeof(s2)); // expected-warning {{size argument in 'strlcpy' call appears to be size of the source; expected the size of the destination}} expected-note {{change size argument to be the size of the destination}}
+ strlcpy(s1+3, s2, sizeof(s2)); // expected-warning {{size argument in 'strlcpy' call appears to be size of the source; expected the size of the destination}}
+}
More information about the cfe-commits
mailing list