gcc already has this warning ("memset used with constant zero length
parameter; this could be due to transposed parameters"), so I think this
can just become a real warning immediately. (One problem with the gcc
warning is that they emit it after optimizations, so if you do memset(ptr,
value, A - B) and A and B are known at compile time (due to them being
template arguments, for example) it'll warn too. So don't do that :-) ).On Wed, Jul 16, 2014 at 7:30 AM, Benjamin Kramer <[email protected]> wrote: > Author: d0k > Date: Wed Jul 16 09:30:19 2014 > New Revision: 213155 > > URL: http://llvm.org/viewvc/llvm-project?rev=213155&view=rev > Log: > [clang-tidy] Add a checker for zero-length memset. > > If there's memset(x, y, 0) in the code it's most likely a mistake. The > checker suggests a fix-it to swap 'y' and '0'. > > I think this has the potential to be promoted into a general clang warning > after some testing in clang-tidy. > > Differential Revision: http://reviews.llvm.org/D4535 > > Added: > clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.cpp > clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.h > clang-tools-extra/trunk/test/clang-tidy/google-memset-zero-length.cpp > Modified: > clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt > clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp > > Modified: clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt > URL: > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt?rev=213155&r1=213154&r2=213155&view=diff > > ============================================================================== > --- clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt (original) > +++ clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt Wed Jul 16 > 09:30:19 2014 > @@ -5,6 +5,7 @@ add_clang_library(clangTidyGoogleModule > ExplicitConstructorCheck.cpp > ExplicitMakePairCheck.cpp > GoogleTidyModule.cpp > + MemsetZeroLengthCheck.cpp > NamedParameterCheck.cpp > OverloadedUnaryAndCheck.cpp > StringReferenceMemberCheck.cpp > > Modified: clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp > URL: > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp?rev=213155&r1=213154&r2=213155&view=diff > > ============================================================================== > --- clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp > (original) > +++ clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp Wed Jul > 16 09:30:19 2014 > @@ -13,6 +13,7 @@ > #include "AvoidCStyleCastsCheck.h" > #include "ExplicitConstructorCheck.h" > #include "ExplicitMakePairCheck.h" > +#include "MemsetZeroLengthCheck.h" > #include "NamedParameterCheck.h" > #include "OverloadedUnaryAndCheck.h" > #include "StringReferenceMemberCheck.h" > @@ -46,6 +47,9 @@ public: > "google-runtime-member-string-references", > new ClangTidyCheckFactory<runtime::StringReferenceMemberCheck>()); > CheckFactories.addCheckFactory( > + "google-runtime-memset", > + new ClangTidyCheckFactory<runtime::MemsetZeroLengthCheck>()); > + CheckFactories.addCheckFactory( > "google-readability-casting", > new ClangTidyCheckFactory<readability::AvoidCStyleCastsCheck>()); > CheckFactories.addCheckFactory( > > Added: 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=213155&view=auto > > ============================================================================== > --- clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.cpp > (added) > +++ clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.cpp > Wed Jul 16 09:30:19 2014 > @@ -0,0 +1,85 @@ > +//===--- MemsetZeroLengthCheck.cpp - clang-tidy -------------------*- C++ > -*-===// > +// > +// The LLVM Compiler Infrastructure > +// > +// This file is distributed under the University of Illinois Open Source > +// License. See LICENSE.TXT for details. > +// > > +//===----------------------------------------------------------------------===// > + > +#include "MemsetZeroLengthCheck.h" > +#include "clang/ASTMatchers/ASTMatchFinder.h" > +#include "clang/ASTMatchers/ASTMatchers.h" > +#include "clang/AST/ASTContext.h" > +#include "clang/Lex/Lexer.h" > + > +using namespace clang::ast_matchers; > + > +namespace clang { > +namespace tidy { > +namespace runtime { > + > +void > +MemsetZeroLengthCheck::registerMatchers(ast_matchers::MatchFinder > *Finder) { > + auto InTemplateInstantiation = hasAncestor( > + decl(anyOf(recordDecl(ast_matchers::isTemplateInstantiation()), > + functionDecl(ast_matchers::isTemplateInstantiation())))); > + // Look for memset(x, y, 0) as those is most likely an argument swap. > + // TODO: Also handle other standard functions that suffer from the same > + // problem, e.g. memchr. > + Finder->addMatcher( > + callExpr(callee(functionDecl(hasName("::memset"))), > argumentCountIs(3), > + unless(InTemplateInstantiation)).bind("decl"), > + this); > +} > + > +/// \brief Get a StringRef representing a SourceRange. > +static StringRef getAsString(const MatchFinder::MatchResult &Result, > + SourceRange R) { > + const SourceManager &SM = *Result.SourceManager; > + // Don't even try to resolve macro or include contraptions. Not worth > emitting > + // a fixit for. > + if (R.getBegin().isMacroID() || > + !SM.isWrittenInSameFile(R.getBegin(), R.getEnd())) > + return StringRef(); > + > + const char *Begin = SM.getCharacterData(R.getBegin()); > + const char *End = SM.getCharacterData(Lexer::getLocForEndOfToken( > + R.getEnd(), 0, SM, Result.Context->getLangOpts())); > + > + return StringRef(Begin, End - Begin); > +} > + > +void MemsetZeroLengthCheck::check(const MatchFinder::MatchResult &Result) > { > + const auto *Call = Result.Nodes.getNodeAs<CallExpr>("decl"); > + > + 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. We don't emit a warning if the second argument also > + // evaluates to zero. > + llvm::APSInt Value1, Value2; > + if (!Arg2->EvaluateAsInt(Value2, *Result.Context) || Value2 != 0 || > + (Arg1->EvaluateAsInt(Value1, *Result.Context) && Value1 == 0)) > + return; > + > + // 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(); > + SourceRange RHSRange = Arg2->getSourceRange(); > + StringRef RHSString = getAsString(Result, RHSRange); > + StringRef LHSString = getAsString(Result, LHSRange); > + if (LHSString.empty() || RHSString.empty()) > + return; > + > + D << > FixItHint::CreateReplacement(CharSourceRange::getTokenRange(LHSRange), > + RHSString) > + << > FixItHint::CreateReplacement(CharSourceRange::getTokenRange(RHSRange), > + LHSString); > +} > + > +} // namespace runtime > +} // namespace tidy > +} // namespace clang > > Added: clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.h > URL: > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.h?rev=213155&view=auto > > ============================================================================== > --- clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.h > (added) > +++ clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.h Wed > Jul 16 09:30:19 2014 > @@ -0,0 +1,35 @@ > +//===--- MemsetZeroLengthCheck.h - clang-tidy ---------------------*- C++ > -*-===// > +// > +// The LLVM Compiler Infrastructure > +// > +// This file is distributed under the University of Illinois Open Source > +// License. See LICENSE.TXT for details. > +// > > +//===----------------------------------------------------------------------===// > + > +#ifndef > LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_MEMSET_ZERO_LENGTH_CHECK_H > +#define > LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_MEMSET_ZERO_LENGTH_CHECK_H > + > +#include "../ClangTidy.h" > + > +namespace clang { > +namespace tidy { > +namespace runtime { > + > +/// \brief Finds calls to memset with a literal zero in the length > argument. > +/// > +/// This is most likely unintended and the length and value arguments are > +/// swapped. > +/// > +/// Corresponding cpplint.py check name: 'runtime/memset'. > +class MemsetZeroLengthCheck : public ClangTidyCheck { > +public: > + void registerMatchers(ast_matchers::MatchFinder *Finder) override; > + void check(const ast_matchers::MatchFinder::MatchResult &Result) > override; > +}; > + > +} // namespace runtime > +} // namespace tidy > +} // namespace clang > + > +#endif // > LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_MEMSET_ZERO_LENGTH_CHECK_H > > Added: > clang-tools-extra/trunk/test/clang-tidy/google-memset-zero-length.cpp > URL: > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-memset-zero-length.cpp?rev=213155&view=auto > > ============================================================================== > --- clang-tools-extra/trunk/test/clang-tidy/google-memset-zero-length.cpp > (added) > +++ clang-tools-extra/trunk/test/clang-tidy/google-memset-zero-length.cpp > Wed Jul 16 09:30:19 2014 > @@ -0,0 +1,51 @@ > +// RUN: $(dirname %s)/check_clang_tidy_fix.sh %s google-runtime-memset %t > +// REQUIRES: shell > + > +void *memset(void *, int, __SIZE_TYPE__); > + > +namespace std { > + using ::memset; > +} > + > +template <int i> > +void memtmpl() { > + memset(0, sizeof(int), i); > + memset(0, sizeof(int), 0); > +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset of size zero, > potentially swapped argument > +// CHECK-FIXES: memset(0, 0, sizeof(int)); > +} > + > +void foo(void *a, int xsize, int ysize) { > + memset(a, sizeof(int), 0); > +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset of size zero, > potentially swapped argument > +// CHECK-FIXES: memset(a, 0, sizeof(int)); > +#define M memset(a, sizeof(int), 0); > + M > +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset of size zero, > potentially swapped argument > +// CHECK-FIXES: #define M memset(a, sizeof(int), 0); > + ::memset(a, xsize * > + ysize, 0); > +// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: memset of size zero, > potentially swapped argument > +// CHECK-FIXES: ::memset(a, 0, xsize * > +// CHECK-FIXES-NEXT: ysize); > + std::memset(a, sizeof(int), 0x00); > +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset of size zero, > potentially swapped argument > +// CHECK-FIXES: std::memset(a, 0x00, sizeof(int)); > + > + const int v = 0; > + memset(a, sizeof(int), v); > +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset of size zero, > potentially swapped argument > +// CHECK-FIXES: memset(a, v, sizeof(int)); > + > + memset(a, sizeof(int), v + v); > +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset of size zero, > potentially swapped argument > +// CHECK-FIXES: memset(a, v + v, sizeof(int)); > + > + memset(a, sizeof(int), v + 1); > + > + memset(a, -1, sizeof(int)); > + memset(a, 0xcd, 1); > + memset(a, v, 0); > + > + memtmpl<0>(); > +} > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
