[clang-tools-extra] r213155 - [clang-tidy] Add a checker for zero-length memset.
David Blaikie
dblaikie at gmail.com
Wed Jul 16 09:48:52 PDT 2014
http://llvm.org/bugs/show_bug.cgi?id=13807
On Wed, Jul 16, 2014 at 8:49 AM, Nico Weber <thakis at chromium.org> 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 <benny.kra at googlemail.com>
> 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
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
More information about the cfe-commits
mailing list