Warn on mismatched parentheses in memcmp and friends.
Thisadds a new warning that warns on code like this:
if (memcmp(a, b, sizeof(a) != 0))
The warning looks like:
test4.cc:5:30: warning: size argument in 'memcmp' call is a comparison [-Wmemsize-comparison]
if (memcmp(a, b, sizeof(a) != 0))
~~~~~~~~~~^~~~
test4.cc:5:7: note: did you mean to compare the result of 'memcmp' instead?
if (memcmp(a, b, sizeof(a) != 0))
^ ~
)
test4.cc:5:20: note: explicitly cast the argument to size_t to silence this warning
if (memcmp(a, b, sizeof(a) != 0))
^
(size_t)( )
1 warning generated.
This found 2 bugs in chromium and has 0 false positives on both chromium and
llvm.
The idea of triggering this warning on a binop in the size argument is due to
rnk.
llvm-svn: 198063
This commit is contained in:
Nico Weber
parent
63701d2533
commit
0e6daefe8f
|
|
@ -390,11 +390,18 @@ def warn_sizeof_pointer_type_memaccess : Warning<
|
|||
"%select{destination|source}2; expected %3 or an explicit length">,
|
||||
InGroup<SizeofPointerMemaccess>;
|
||||
def warn_strlcpycat_wrong_size : Warning<
|
||||
"size argument in %0 call appears to be size of the source; expected the size of "
|
||||
"the destination">,
|
||||
"size argument in %0 call appears to be size of the source; "
|
||||
"expected the size of the destination">,
|
||||
InGroup<DiagGroup<"strlcpy-strlcat-size">>;
|
||||
def note_strlcpycat_wrong_size : Note<
|
||||
"change size argument to be the size of the destination">;
|
||||
def warn_memsize_comparison : Warning<
|
||||
"size argument in %0 call is a comparison">,
|
||||
InGroup<DiagGroup<"memsize-comparison">>;
|
||||
def warn_memsize_comparison_paren_note : Note<
|
||||
"did you mean to compare the result of %0 instead?">;
|
||||
def warn_memsize_comparison_cast_note : Note<
|
||||
"explicitly cast the argument to size_t to silence this warning">;
|
||||
|
||||
def warn_strncat_large_size : Warning<
|
||||
"the value of the size argument in 'strncat' is too large, might lead to a "
|
||||
|
|
|
|||
|
|
@ -622,7 +622,7 @@ bool Sema::CheckARMBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall) {
|
|||
RHS.get(), AA_Assigning))
|
||||
return true;
|
||||
}
|
||||
|
||||
|
||||
// For NEON intrinsics which take an immediate value as part of the
|
||||
// instruction, range check them here.
|
||||
unsigned i = 0, l = 0, u = 0;
|
||||
|
|
@ -3560,6 +3560,40 @@ void Sema::CheckFormatString(const StringLiteral *FExpr,
|
|||
|
||||
//===--- CHECK: Standard memory functions ---------------------------------===//
|
||||
|
||||
/// \brief Takes the expression passed to the size_t parameter of functions
|
||||
/// such as memcmp, strncat, etc and warns if it's a comparison.
|
||||
///
|
||||
/// This is to catch typos like `if (memcmp(&a, &b, sizeof(a) > 0))`.
|
||||
static bool CheckMemorySizeofForComparison(Sema &S, const Expr *E,
|
||||
IdentifierInfo *FnName,
|
||||
SourceLocation FnLoc,
|
||||
SourceLocation RParenLoc) {
|
||||
const BinaryOperator *Size = dyn_cast<BinaryOperator>(E);
|
||||
if (!Size)
|
||||
return false;
|
||||
|
||||
// if E is binop and op is >, <, >=, <=, ==, &&, ||:
|
||||
if (!Size->isComparisonOp() && !Size->isEqualityOp() && !Size->isLogicalOp())
|
||||
return false;
|
||||
|
||||
Preprocessor &PP = S.getPreprocessor();
|
||||
SourceRange SizeRange = Size->getSourceRange();
|
||||
S.Diag(Size->getOperatorLoc(), diag::warn_memsize_comparison)
|
||||
<< SizeRange << FnName;
|
||||
S.Diag(FnLoc, diag::warn_memsize_comparison_paren_note)
|
||||
<< FnName
|
||||
<< FixItHint::CreateInsertion(
|
||||
PP.getLocForEndOfToken(Size->getLHS()->getLocEnd()),
|
||||
")")
|
||||
<< FixItHint::CreateRemoval(RParenLoc);
|
||||
S.Diag(SizeRange.getBegin(), diag::warn_memsize_comparison_cast_note)
|
||||
<< FixItHint::CreateInsertion(SizeRange.getBegin(), "(size_t)(")
|
||||
<< FixItHint::CreateInsertion(
|
||||
PP.getLocForEndOfToken(SizeRange.getEnd()), ")");
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
/// \brief Determine whether the given type is a dynamic class type (e.g.,
|
||||
/// whether it has a vtable).
|
||||
static bool isDynamicClassType(QualType T) {
|
||||
|
|
@ -3615,6 +3649,10 @@ void Sema::CheckMemaccessArguments(const CallExpr *Call,
|
|||
unsigned LenArg = (BId == Builtin::BIstrndup ? 1 : 2);
|
||||
const Expr *LenExpr = Call->getArg(LenArg)->IgnoreParenImpCasts();
|
||||
|
||||
if (CheckMemorySizeofForComparison(*this, LenExpr, FnName,
|
||||
Call->getLocStart(), Call->getRParenLoc()))
|
||||
return;
|
||||
|
||||
// We have special checking when the length is a sizeof expression.
|
||||
QualType SizeOfArgTy = getSizeOfArgType(LenExpr);
|
||||
const Expr *SizeOfArg = getSizeOfExprArg(LenExpr);
|
||||
|
|
@ -3798,6 +3836,10 @@ void Sema::CheckStrlcpycatArguments(const CallExpr *Call,
|
|||
const Expr *SrcArg = ignoreLiteralAdditions(Call->getArg(1), Context);
|
||||
const Expr *SizeArg = ignoreLiteralAdditions(Call->getArg(2), Context);
|
||||
const Expr *CompareWithSrc = NULL;
|
||||
|
||||
if (CheckMemorySizeofForComparison(*this, SizeArg, FnName,
|
||||
Call->getLocStart(), Call->getRParenLoc()))
|
||||
return;
|
||||
|
||||
// Look for 'strlcpy(dst, x, sizeof(x))'
|
||||
if (const Expr *Ex = getSizeOfExprArg(SizeArg))
|
||||
|
|
@ -3880,6 +3922,10 @@ void Sema::CheckStrncatArguments(const CallExpr *CE,
|
|||
const Expr *SrcArg = CE->getArg(1)->IgnoreParenCasts();
|
||||
const Expr *LenArg = CE->getArg(2)->IgnoreParenCasts();
|
||||
|
||||
if (CheckMemorySizeofForComparison(*this, LenArg, FnName, CE->getLocStart(),
|
||||
CE->getRParenLoc()))
|
||||
return;
|
||||
|
||||
// Identify common expressions, which are wrongly used as the size argument
|
||||
// to strncat and may lead to buffer overflows.
|
||||
unsigned PatternType = 0;
|
||||
|
|
@ -5235,7 +5281,7 @@ void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
|
|||
if (Target->isSpecificBuiltinType(BuiltinType::Bool)) {
|
||||
if (isa<StringLiteral>(E))
|
||||
// Warn on string literal to bool. Checks for string literals in logical
|
||||
// expressions, for instances, assert(0 && "error here"), is prevented
|
||||
// expressions, for instances, assert(0 && "error here"), are prevented
|
||||
// by a check in AnalyzeImplicitConversions().
|
||||
return DiagnoseImpCast(S, E, T, CC,
|
||||
diag::warn_impcast_string_literal_to_bool);
|
||||
|
|
|
|||
|
|
@ -0,0 +1,93 @@
|
|||
// RUN: %clang_cc1 -fsyntax-only -verify %s
|
||||
//
|
||||
|
||||
typedef __SIZE_TYPE__ size_t;
|
||||
extern "C" void *memset(void *, int, size_t);
|
||||
extern "C" void *memmove(void *s1, const void *s2, size_t n);
|
||||
extern "C" void *memcpy(void *s1, const void *s2, size_t n);
|
||||
extern "C" void *memcmp(void *s1, const void *s2, size_t n);
|
||||
extern "C" int strncmp(const char *s1, const char *s2, size_t n);
|
||||
extern "C" int strncasecmp(const char *s1, const char *s2, size_t n);
|
||||
extern "C" char *strncpy(char *dst, const char *src, size_t n);
|
||||
extern "C" char *strncat(char *dst, const char *src, size_t n);
|
||||
extern "C" char *strndup(const char *src, size_t n);
|
||||
extern "C" size_t strlcpy(char *dst, const char *src, size_t size);
|
||||
extern "C" size_t strlcat(char *dst, const char *src, size_t size);
|
||||
|
||||
void f() {
|
||||
char b1[80], b2[80];
|
||||
if (memset(b1, 0, sizeof(b1) != 0)) {} // \
|
||||
expected-warning{{size argument in 'memset' call is a comparison}} \
|
||||
expected-note {{did you mean to compare}} \
|
||||
expected-note {{explicitly cast the argument}}
|
||||
if (memset(b1, 0, sizeof(b1)) != 0) {}
|
||||
|
||||
if (memmove(b1, b2, sizeof(b1) == 0)) {} // \
|
||||
expected-warning{{size argument in 'memmove' call is a comparison}} \
|
||||
expected-note {{did you mean to compare}} \
|
||||
expected-note {{explicitly cast the argument}}
|
||||
if (memmove(b1, b2, sizeof(b1)) == 0) {}
|
||||
|
||||
if (memcpy(b1, b2, sizeof(b1) < 0)) {} // \
|
||||
expected-warning{{size argument in 'memcpy' call is a comparison}} \
|
||||
expected-note {{did you mean to compare}} \
|
||||
expected-note {{explicitly cast the argument}}
|
||||
if (memcpy(b1, b2, sizeof(b1)) < 0) {}
|
||||
|
||||
if (memcmp(b1, b2, sizeof(b1) <= 0)) {} // \
|
||||
expected-warning{{size argument in 'memcmp' call is a comparison}} \
|
||||
expected-note {{did you mean to compare}} \
|
||||
expected-note {{explicitly cast the argument}}
|
||||
if (memcmp(b1, b2, sizeof(b1)) <= 0) {}
|
||||
|
||||
if (strncmp(b1, b2, sizeof(b1) > 0)) {} // \
|
||||
expected-warning{{size argument in 'strncmp' call is a comparison}} \
|
||||
expected-note {{did you mean to compare}} \
|
||||
expected-note {{explicitly cast the argument}}
|
||||
if (strncmp(b1, b2, sizeof(b1)) > 0) {}
|
||||
|
||||
if (strncasecmp(b1, b2, sizeof(b1) >= 0)) {} // \
|
||||
expected-warning{{size argument in 'strncasecmp' call is a comparison}} \
|
||||
expected-note {{did you mean to compare}} \
|
||||
expected-note {{explicitly cast the argument}}
|
||||
if (strncasecmp(b1, b2, sizeof(b1)) >= 0) {}
|
||||
|
||||
if (strncpy(b1, b2, sizeof(b1) == 0 || true)) {} // \
|
||||
expected-warning{{size argument in 'strncpy' call is a comparison}} \
|
||||
expected-note {{did you mean to compare}} \
|
||||
expected-note {{explicitly cast the argument}}
|
||||
if (strncpy(b1, b2, sizeof(b1)) == 0 || true) {}
|
||||
|
||||
if (strncat(b1, b2, sizeof(b1) - 1 >= 0 && true)) {} // \
|
||||
expected-warning{{size argument in 'strncat' call is a comparison}} \
|
||||
expected-note {{did you mean to compare}} \
|
||||
expected-note {{explicitly cast the argument}}
|
||||
if (strncat(b1, b2, sizeof(b1) - 1) >= 0 && true) {}
|
||||
|
||||
if (strndup(b1, sizeof(b1) != 0)) {} // \
|
||||
expected-warning{{size argument in 'strndup' call is a comparison}} \
|
||||
expected-note {{did you mean to compare}} \
|
||||
expected-note {{explicitly cast the argument}}
|
||||
if (strndup(b1, sizeof(b1)) != 0) {}
|
||||
|
||||
if (strlcpy(b1, b2, sizeof(b1) != 0)) {} // \
|
||||
expected-warning{{size argument in 'strlcpy' call is a comparison}} \
|
||||
expected-note {{did you mean to compare}} \
|
||||
expected-note {{explicitly cast the argument}}
|
||||
if (strlcpy(b1, b2, sizeof(b1)) != 0) {}
|
||||
|
||||
if (strlcat(b1, b2, sizeof(b1) != 0)) {} // \
|
||||
expected-warning{{size argument in 'strlcat' call is a comparison}} \
|
||||
expected-note {{did you mean to compare}} \
|
||||
expected-note {{explicitly cast the argument}}
|
||||
if (strlcat(b1, b2, sizeof(b1)) != 0) {}
|
||||
|
||||
if (memset(b1, 0, sizeof(b1) / 2)) {}
|
||||
if (memset(b1, 0, sizeof(b1) >> 2)) {}
|
||||
if (memset(b1, 0, 4 << 2)) {}
|
||||
if (memset(b1, 0, 4 + 2)) {}
|
||||
if (memset(b1, 0, 4 - 2)) {}
|
||||
if (memset(b1, 0, 4 * 2)) {}
|
||||
|
||||
if (memset(b1, 0, (size_t)(sizeof(b1) != 0))) {}
|
||||
}
|
||||
Loading…
Reference in New Issue