[clang-tidy] Fix a false positive in google-runtime-memset
google-runtime-memset no longer issues a warning if the fill char value is known to be an invalid fill char count. Namely, it no longer warns for these: memset(p, 0, 0); memset(p, -1, 0); In both cases, swapping the last two args would either be useless (there is no actual bug) or wrong (it would introduce a bug). Patch by Matt Armstrong! llvm-svn: 257320
This commit is contained in:
parent
d0f89cd721
commit
dcbe5a9d17
|
@ -51,25 +51,29 @@ static StringRef getAsString(const MatchFinder::MatchResult &Result,
|
||||||
void MemsetZeroLengthCheck::check(const MatchFinder::MatchResult &Result) {
|
void MemsetZeroLengthCheck::check(const MatchFinder::MatchResult &Result) {
|
||||||
const auto *Call = Result.Nodes.getNodeAs<CallExpr>("decl");
|
const auto *Call = Result.Nodes.getNodeAs<CallExpr>("decl");
|
||||||
|
|
||||||
|
// Note, this is:
|
||||||
|
// void *memset(void *buffer, int fill_char, size_t byte_count);
|
||||||
|
// Arg1 is fill_char, Arg2 is byte_count.
|
||||||
const Expr *Arg1 = Call->getArg(1);
|
const Expr *Arg1 = Call->getArg(1);
|
||||||
const Expr *Arg2 = Call->getArg(2);
|
const Expr *Arg2 = Call->getArg(2);
|
||||||
|
|
||||||
// Try to evaluate the second argument so we can also find values that are not
|
// Return if `byte_count` is not zero at compile time.
|
||||||
// just literals.
|
|
||||||
llvm::APSInt Value1, Value2;
|
llvm::APSInt Value1, Value2;
|
||||||
if (Arg2->isValueDependent() ||
|
if (Arg2->isValueDependent() ||
|
||||||
!Arg2->EvaluateAsInt(Value2, *Result.Context) || Value2 != 0)
|
!Arg2->EvaluateAsInt(Value2, *Result.Context) || Value2 != 0)
|
||||||
return;
|
return;
|
||||||
|
|
||||||
// If both arguments evaluate to zero emit a warning without fix suggestions.
|
// Return if `fill_char` is known to be zero or negative at compile
|
||||||
|
// time. In these cases, swapping the args would be a nop, or
|
||||||
|
// introduce a definite bug. The code is likely correct.
|
||||||
if (!Arg1->isValueDependent() &&
|
if (!Arg1->isValueDependent() &&
|
||||||
Arg1->EvaluateAsInt(Value1, *Result.Context) &&
|
Arg1->EvaluateAsInt(Value1, *Result.Context) &&
|
||||||
(Value1 == 0 || Value1.isNegative())) {
|
(Value1 == 0 || Value1.isNegative()))
|
||||||
diag(Call->getLocStart(), "memset of size zero");
|
|
||||||
return;
|
return;
|
||||||
}
|
|
||||||
|
|
||||||
// Emit a warning and fix-its to swap the arguments.
|
// `byte_count` is known to be zero at compile time, and `fill_char` is
|
||||||
|
// either not known or known to be a positive integer. Emit a warning
|
||||||
|
// and fix-its to swap the arguments.
|
||||||
auto D = diag(Call->getLocStart(),
|
auto D = diag(Call->getLocStart(),
|
||||||
"memset of size zero, potentially swapped arguments");
|
"memset of size zero, potentially swapped arguments");
|
||||||
SourceRange LHSRange = Arg1->getSourceRange();
|
SourceRange LHSRange = Arg1->getSourceRange();
|
||||||
|
|
|
@ -48,13 +48,15 @@ void foo(void *a, int xsize, int ysize) {
|
||||||
|
|
||||||
memset(a, -1, sizeof(int));
|
memset(a, -1, sizeof(int));
|
||||||
memset(a, 0xcd, 1);
|
memset(a, 0xcd, 1);
|
||||||
memset(a, v, 0);
|
|
||||||
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset of size zero
|
|
||||||
// CHECK-FIXES: memset(a, v, 0);
|
|
||||||
|
|
||||||
|
// Don't warn when the fill char and the length are both known to be
|
||||||
|
// zero. No bug is possible.
|
||||||
|
memset(a, 0, v);
|
||||||
|
memset(a, v, 0);
|
||||||
|
|
||||||
|
// -1 is clearly not a length by virtue of being negative, so no warning
|
||||||
|
// despite v == 0.
|
||||||
memset(a, -1, v);
|
memset(a, -1, v);
|
||||||
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset of size zero
|
|
||||||
// CHECK-FIXES: memset(a, -1, v);
|
|
||||||
|
|
||||||
memtmpl<0, int>();
|
memtmpl<0, int>();
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue