diff options
-rw-r--r-- | clang-tools-extra/clang-tidy/google/MemsetZeroLengthCheck.cpp | 18 | ||||
-rw-r--r-- | clang-tools-extra/test/clang-tidy/google-runtime-memset-zero-length.cpp | 10 |
2 files changed, 17 insertions, 11 deletions
diff --git a/clang-tools-extra/clang-tidy/google/MemsetZeroLengthCheck.cpp b/clang-tools-extra/clang-tidy/google/MemsetZeroLengthCheck.cpp index 4c0b8e01f22..bcf4b5b3b61 100644 --- a/clang-tools-extra/clang-tidy/google/MemsetZeroLengthCheck.cpp +++ b/clang-tools-extra/clang-tidy/google/MemsetZeroLengthCheck.cpp @@ -51,25 +51,29 @@ static StringRef getAsString(const MatchFinder::MatchResult &Result, void MemsetZeroLengthCheck::check(const MatchFinder::MatchResult &Result) { 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 *Arg2 = Call->getArg(2); - // Try to evaluate the second argument so we can also find values that are not - // just literals. + // Return if `byte_count` is not zero at compile time. llvm::APSInt Value1, Value2; if (Arg2->isValueDependent() || !Arg2->EvaluateAsInt(Value2, *Result.Context) || Value2 != 0) 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() && Arg1->EvaluateAsInt(Value1, *Result.Context) && - (Value1 == 0 || Value1.isNegative())) { - diag(Call->getLocStart(), "memset of size zero"); + (Value1 == 0 || Value1.isNegative())) 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(), "memset of size zero, potentially swapped arguments"); SourceRange LHSRange = Arg1->getSourceRange(); diff --git a/clang-tools-extra/test/clang-tidy/google-runtime-memset-zero-length.cpp b/clang-tools-extra/test/clang-tidy/google-runtime-memset-zero-length.cpp index db75cb0fedd..7599c755b48 100644 --- a/clang-tools-extra/test/clang-tidy/google-runtime-memset-zero-length.cpp +++ b/clang-tools-extra/test/clang-tidy/google-runtime-memset-zero-length.cpp @@ -48,13 +48,15 @@ void foo(void *a, int xsize, int ysize) { memset(a, -1, sizeof(int)); memset(a, 0xcd, 1); + + // 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); -// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset of size zero -// CHECK-FIXES: 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); -// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset of size zero -// CHECK-FIXES: memset(a, -1, v); memtmpl<0, int>(); } |