http://llvm.org/bugs/show_bug.cgi?id=13807
On Wed, Jul 16, 2014 at 8:49 AM, Nico Weber <[email protected]> wrote: > 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 > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
