<div dir="ltr">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 :-) ).</div>
<div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Jul 16, 2014 at 7:30 AM, Benjamin Kramer <span dir="ltr"><<a href="mailto:benny.kra@googlemail.com" target="_blank">benny.kra@googlemail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: d0k<br>
Date: Wed Jul 16 09:30:19 2014<br>
New Revision: 213155<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=213155&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=213155&view=rev</a><br>
Log:<br>
[clang-tidy] Add a checker for zero-length memset.<br>
<br>
If there's memset(x, y, 0) in the code it's most likely a mistake. The<br>
checker suggests a fix-it to swap 'y' and '0'.<br>
<br>
I think this has the potential to be promoted into a general clang warning<br>
after some testing in clang-tidy.<br>
<br>
Differential Revision: <a href="http://reviews.llvm.org/D4535" target="_blank">http://reviews.llvm.org/D4535</a><br>
<br>
Added:<br>
    clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.cpp<br>
    clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.h<br>
    clang-tools-extra/trunk/test/clang-tidy/google-memset-zero-length.cpp<br>
Modified:<br>
    clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt<br>
    clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp<br>
<br>
Modified: clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt?rev=213155&r1=213154&r2=213155&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt?rev=213155&r1=213154&r2=213155&view=diff</a><br>

==============================================================================<br>
--- clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt (original)<br>
+++ clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt Wed Jul 16 09:30:19 2014<br>
@@ -5,6 +5,7 @@ add_clang_library(clangTidyGoogleModule<br>
   ExplicitConstructorCheck.cpp<br>
   ExplicitMakePairCheck.cpp<br>
   GoogleTidyModule.cpp<br>
+  MemsetZeroLengthCheck.cpp<br>
   NamedParameterCheck.cpp<br>
   OverloadedUnaryAndCheck.cpp<br>
   StringReferenceMemberCheck.cpp<br>
<br>
Modified: clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp?rev=213155&r1=213154&r2=213155&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp?rev=213155&r1=213154&r2=213155&view=diff</a><br>

==============================================================================<br>
--- clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp (original)<br>
+++ clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp Wed Jul 16 09:30:19 2014<br>
@@ -13,6 +13,7 @@<br>
 #include "AvoidCStyleCastsCheck.h"<br>
 #include "ExplicitConstructorCheck.h"<br>
 #include "ExplicitMakePairCheck.h"<br>
+#include "MemsetZeroLengthCheck.h"<br>
 #include "NamedParameterCheck.h"<br>
 #include "OverloadedUnaryAndCheck.h"<br>
 #include "StringReferenceMemberCheck.h"<br>
@@ -46,6 +47,9 @@ public:<br>
         "google-runtime-member-string-references",<br>
         new ClangTidyCheckFactory<runtime::StringReferenceMemberCheck>());<br>
     CheckFactories.addCheckFactory(<br>
+        "google-runtime-memset",<br>
+        new ClangTidyCheckFactory<runtime::MemsetZeroLengthCheck>());<br>
+    CheckFactories.addCheckFactory(<br>
         "google-readability-casting",<br>
         new ClangTidyCheckFactory<readability::AvoidCStyleCastsCheck>());<br>
     CheckFactories.addCheckFactory(<br>
<br>
Added: clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.cpp?rev=213155&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.cpp?rev=213155&view=auto</a><br>

==============================================================================<br>
--- clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.cpp (added)<br>
+++ clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.cpp Wed Jul 16 09:30:19 2014<br>
@@ -0,0 +1,85 @@<br>
+//===--- MemsetZeroLengthCheck.cpp - clang-tidy -------------------*- C++ -*-===//<br>
+//<br>
+//                     The LLVM Compiler Infrastructure<br>
+//<br>
+// This file is distributed under the University of Illinois Open Source<br>
+// License. See LICENSE.TXT for details.<br>
+//<br>
+//===----------------------------------------------------------------------===//<br>
+<br>
+#include "MemsetZeroLengthCheck.h"<br>
+#include "clang/ASTMatchers/ASTMatchFinder.h"<br>
+#include "clang/ASTMatchers/ASTMatchers.h"<br>
+#include "clang/AST/ASTContext.h"<br>
+#include "clang/Lex/Lexer.h"<br>
+<br>
+using namespace clang::ast_matchers;<br>
+<br>
+namespace clang {<br>
+namespace tidy {<br>
+namespace runtime {<br>
+<br>
+void<br>
+MemsetZeroLengthCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {<br>
+  auto InTemplateInstantiation = hasAncestor(<br>
+      decl(anyOf(recordDecl(ast_matchers::isTemplateInstantiation()),<br>
+                 functionDecl(ast_matchers::isTemplateInstantiation()))));<br>
+  // Look for memset(x, y, 0) as those is most likely an argument swap.<br>
+  // TODO: Also handle other standard functions that suffer from the same<br>
+  //       problem, e.g. memchr.<br>
+  Finder->addMatcher(<br>
+      callExpr(callee(functionDecl(hasName("::memset"))), argumentCountIs(3),<br>
+               unless(InTemplateInstantiation)).bind("decl"),<br>
+      this);<br>
+}<br>
+<br>
+/// \brief Get a StringRef representing a SourceRange.<br>
+static StringRef getAsString(const MatchFinder::MatchResult &Result,<br>
+                             SourceRange R) {<br>
+  const SourceManager &SM = *Result.SourceManager;<br>
+  // Don't even try to resolve macro or include contraptions. Not worth emitting<br>
+  // a fixit for.<br>
+  if (R.getBegin().isMacroID() ||<br>
+      !SM.isWrittenInSameFile(R.getBegin(), R.getEnd()))<br>
+    return StringRef();<br>
+<br>
+  const char *Begin = SM.getCharacterData(R.getBegin());<br>
+  const char *End = SM.getCharacterData(Lexer::getLocForEndOfToken(<br>
+      R.getEnd(), 0, SM, Result.Context->getLangOpts()));<br>
+<br>
+  return StringRef(Begin, End - Begin);<br>
+}<br>
+<br>
+void MemsetZeroLengthCheck::check(const MatchFinder::MatchResult &Result) {<br>
+  const auto *Call = Result.Nodes.getNodeAs<CallExpr>("decl");<br>
+<br>
+  const Expr *Arg1 = Call->getArg(1);<br>
+  const Expr *Arg2 = Call->getArg(2);<br>
+<br>
+  // Try to evaluate the second argument so we can also find values that are not<br>
+  // just literals. We don't emit a warning if the second argument also<br>
+  // evaluates to zero.<br>
+  llvm::APSInt Value1, Value2;<br>
+  if (!Arg2->EvaluateAsInt(Value2, *Result.Context) || Value2 != 0 ||<br>
+      (Arg1->EvaluateAsInt(Value1, *Result.Context) && Value1 == 0))<br>
+    return;<br>
+<br>
+  // Emit a warning and fix-its to swap the arguments.<br>
+  auto D = diag(Call->getLocStart(),<br>
+                "memset of size zero, potentially swapped arguments");<br>
+  SourceRange LHSRange = Arg1->getSourceRange();<br>
+  SourceRange RHSRange = Arg2->getSourceRange();<br>
+  StringRef RHSString = getAsString(Result, RHSRange);<br>
+  StringRef LHSString = getAsString(Result, LHSRange);<br>
+  if (LHSString.empty() || RHSString.empty())<br>
+    return;<br>
+<br>
+  D << FixItHint::CreateReplacement(CharSourceRange::getTokenRange(LHSRange),<br>
+                                    RHSString)<br>
+    << FixItHint::CreateReplacement(CharSourceRange::getTokenRange(RHSRange),<br>
+                                    LHSString);<br>
+}<br>
+<br>
+} // namespace runtime<br>
+} // namespace tidy<br>
+} // namespace clang<br>
<br>
Added: clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.h?rev=213155&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.h?rev=213155&view=auto</a><br>

==============================================================================<br>
--- clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.h (added)<br>
+++ clang-tools-extra/trunk/clang-tidy/google/MemsetZeroLengthCheck.h Wed Jul 16 09:30:19 2014<br>
@@ -0,0 +1,35 @@<br>
+//===--- MemsetZeroLengthCheck.h - clang-tidy ---------------------*- C++ -*-===//<br>
+//<br>
+//                     The LLVM Compiler Infrastructure<br>
+//<br>
+// This file is distributed under the University of Illinois Open Source<br>
+// License. See LICENSE.TXT for details.<br>
+//<br>
+//===----------------------------------------------------------------------===//<br>
+<br>
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_MEMSET_ZERO_LENGTH_CHECK_H<br>
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_MEMSET_ZERO_LENGTH_CHECK_H<br>
+<br>
+#include "../ClangTidy.h"<br>
+<br>
+namespace clang {<br>
+namespace tidy {<br>
+namespace runtime {<br>
+<br>
+/// \brief Finds calls to memset with a literal zero in the length argument.<br>
+///<br>
+/// This is most likely unintended and the length and value arguments are<br>
+/// swapped.<br>
+///<br>
+/// Corresponding cpplint.py check name: 'runtime/memset'.<br>
+class MemsetZeroLengthCheck : public ClangTidyCheck {<br>
+public:<br>
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;<br>
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;<br>
+};<br>
+<br>
+} // namespace runtime<br>
+} // namespace tidy<br>
+} // namespace clang<br>
+<br>
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_MEMSET_ZERO_LENGTH_CHECK_H<br>
<br>
Added: clang-tools-extra/trunk/test/clang-tidy/google-memset-zero-length.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-memset-zero-length.cpp?rev=213155&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-memset-zero-length.cpp?rev=213155&view=auto</a><br>

==============================================================================<br>
--- clang-tools-extra/trunk/test/clang-tidy/google-memset-zero-length.cpp (added)<br>
+++ clang-tools-extra/trunk/test/clang-tidy/google-memset-zero-length.cpp Wed Jul 16 09:30:19 2014<br>
@@ -0,0 +1,51 @@<br>
+// RUN: $(dirname %s)/check_clang_tidy_fix.sh %s google-runtime-memset %t<br>
+// REQUIRES: shell<br>
+<br>
+void *memset(void *, int, __SIZE_TYPE__);<br>
+<br>
+namespace std {<br>
+  using ::memset;<br>
+}<br>
+<br>
+template <int i><br>
+void memtmpl() {<br>
+  memset(0, sizeof(int), i);<br>
+  memset(0, sizeof(int), 0);<br>
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset of size zero, potentially swapped argument<br>
+// CHECK-FIXES: memset(0, 0, sizeof(int));<br>
+}<br>
+<br>
+void foo(void *a, int xsize, int ysize) {<br>
+  memset(a, sizeof(int), 0);<br>
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset of size zero, potentially swapped argument<br>
+// CHECK-FIXES: memset(a, 0, sizeof(int));<br>
+#define M memset(a, sizeof(int), 0);<br>
+  M<br>
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset of size zero, potentially swapped argument<br>
+// CHECK-FIXES: #define M memset(a, sizeof(int), 0);<br>
+  ::memset(a, xsize *<br>
+           ysize, 0);<br>
+// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: memset of size zero, potentially swapped argument<br>
+// CHECK-FIXES: ::memset(a, 0, xsize *<br>
+// CHECK-FIXES-NEXT: ysize);<br>
+  std::memset(a, sizeof(int), 0x00);<br>
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset of size zero, potentially swapped argument<br>
+// CHECK-FIXES: std::memset(a, 0x00, sizeof(int));<br>
+<br>
+  const int v = 0;<br>
+  memset(a, sizeof(int), v);<br>
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset of size zero, potentially swapped argument<br>
+// CHECK-FIXES: memset(a, v, sizeof(int));<br>
+<br>
+  memset(a, sizeof(int), v + v);<br>
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset of size zero, potentially swapped argument<br>
+// CHECK-FIXES: memset(a, v + v, sizeof(int));<br>
+<br>
+  memset(a, sizeof(int), v + 1);<br>
+<br>
+  memset(a, -1, sizeof(int));<br>
+  memset(a, 0xcd, 1);<br>
+  memset(a, v, 0);<br>
+<br>
+  memtmpl<0>();<br>
+}<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>