Author: alexfh
Date: Mon Jan 11 04:26:29 2016
New Revision: 257320
URL: http://llvm.org/viewvc/llvm-project?rev=257320&view=rev
Log:
[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!
Modified:
clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/google-runtime-memset-zero-length.cpp
Modified: clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.cpp?rev=257320&r1=257319&r2=257320&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.cpp
(original)
+++ clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.cpp Mon Jan
11 04:26:29 2016
@@ -51,25 +51,29 @@ static StringRef getAsString(const Match
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();
Modified:
clang-tools-extra/trunk/test/clang-tidy/google-runtime-memset-zero-length.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-runtime-memset-zero-length.cpp?rev=257320&r1=257319&r2=257320&view=diff
==============================================================================
---
clang-tools-extra/trunk/test/clang-tidy/google-runtime-memset-zero-length.cpp
(original)
+++
clang-tools-extra/trunk/test/clang-tidy/google-runtime-memset-zero-length.cpp
Mon Jan 11 04:26:29 2016
@@ -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>();
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
