[clang-tidy] Add bugprone-suspicious-memset-usage check

Created new module bugprone and placed the check in that.

Finds memset() calls with potential mistakes in their arguments.
Replaces and extends the existing google-runtime-memset-zero-length check.

Cases covered:
* Fill value is a character '0'. Integer 0 might have been intended.
* Fill value is out of char range and gets truncated.
* Byte count is zero. Potentially swapped with the fill value argument.

Patch by: Reka Nikolett Kovacs

Differential Revision: https://reviews.llvm.org/D32700

llvm-svn: 308020

clang-tools-extra/clang-tidy/CMakeLists.txt

@@ -28,6 +28,7 @@ add_clang_library(clangTidy
 
 add_subdirectory(android)
 add_subdirectory(boost)
+add_subdirectory(bugprone)
 add_subdirectory(cert)
 add_subdirectory(cppcoreguidelines)
 add_subdirectory(google)

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

@@ -0,0 +1,38 @@
+//===--- BugproneTidyModule.cpp - clang-tidy ------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "../ClangTidy.h"
+#include "../ClangTidyModule.h"
+#include "../ClangTidyModuleRegistry.h"
+#include "SuspiciousMemsetUsageCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+class BugproneModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+    CheckFactories.registerCheck<SuspiciousMemsetUsageCheck>(
+        "bugprone-suspicious-memset-usage");
+  }
+};
+
+} // namespace bugprone
+
+// Register the BugproneTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add<bugprone::BugproneModule>
+    X("bugprone-module", "Adds checks for bugprone code constructs.");
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the BugproneModule.
+volatile int BugproneModuleAnchorSource = 0;
+
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt

@@ -0,0 +1,16 @@
+set(LLVM_LINK_COMPONENTS support)
+
+add_clang_library(clangTidyBugproneModule
+  BugproneTidyModule.cpp
+  SuspiciousMemsetUsageCheck.cpp
+
+  LINK_LIBS
+  clangAnalysis
+  clangAST
+  clangASTMatchers
+  clangBasic
+  clangLex
+  clangTidy
+  clangTidyUtils
+  clangTooling
+  )

clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.cpp

@@ -0,0 +1,127 @@
+//===--- SuspiciousMemsetUsageCheck.cpp - clang-tidy-----------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "SuspiciousMemsetUsageCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Tooling/FixIt.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+void SuspiciousMemsetUsageCheck::registerMatchers(MatchFinder *Finder) {
+  // Note: void *memset(void *buffer, int fill_char, size_t byte_count);
+  // Look for memset(x, '0', z). Probably memset(x, 0, z) was intended.
+  Finder->addMatcher(
+      callExpr(
+          callee(functionDecl(hasName("::memset"))),
+          hasArgument(1, characterLiteral(equals(static_cast<unsigned>('0')))
+                             .bind("char-zero-fill")),
+          unless(
+              eachOf(hasArgument(0, anyOf(hasType(pointsTo(isAnyCharacter())),
+                                          hasType(arrayType(hasElementType(
+                                              isAnyCharacter()))))),
+                     isInTemplateInstantiation()))),
+      this);
+
+  // Look for memset with an integer literal in its fill_char argument.
+  // Will check if it gets truncated.
+  Finder->addMatcher(callExpr(callee(functionDecl(hasName("::memset"))),
+                              hasArgument(1, integerLiteral().bind("num-fill")),
+                              unless(isInTemplateInstantiation())),
+                     this);
+
+  // Look for memset(x, y, 0) as that is most likely an argument swap.
+  Finder->addMatcher(
+      callExpr(callee(functionDecl(hasName("::memset"))),
+               unless(hasArgument(1, anyOf(characterLiteral(equals(
+                                               static_cast<unsigned>('0'))),
+                                           integerLiteral()))),
+               unless(isInTemplateInstantiation()))
+          .bind("call"),
+      this);
+}
+
+void SuspiciousMemsetUsageCheck::check(const MatchFinder::MatchResult &Result) {
+  if (const auto *CharZeroFill =
+          Result.Nodes.getNodeAs<CharacterLiteral>("char-zero-fill")) {
+    // Case 1: fill_char of memset() is a character '0'. Probably an
+    // integer zero was intended.
+
+    SourceRange CharRange = CharZeroFill->getSourceRange();
+    auto Diag =
+        diag(CharZeroFill->getLocStart(), "memset fill value is char '0', "
+                                          "potentially mistaken for int 0");
+
+    // Only suggest a fix if no macros are involved.
+    if (CharRange.getBegin().isMacroID())
+      return;
+    Diag << FixItHint::CreateReplacement(
+        CharSourceRange::getTokenRange(CharRange), "0");
+  }
+
+  else if (const auto *NumFill =
+               Result.Nodes.getNodeAs<IntegerLiteral>("num-fill")) {
+    // Case 2: fill_char of memset() is larger in size than an unsigned char
+    // so it gets truncated during conversion.
+
+    llvm::APSInt NumValue;
+    const auto UCharMax = (1 << Result.Context->getCharWidth()) - 1;
+    if (!NumFill->EvaluateAsInt(NumValue, *Result.Context) ||
+        (NumValue >= 0 && NumValue <= UCharMax))
+      return;
+
+    diag(NumFill->getLocStart(), "memset fill value is out of unsigned "
+                                 "character range, gets truncated");
+  }
+
+  else if (const auto *Call = Result.Nodes.getNodeAs<CallExpr>("call")) {
+    // Case 3: byte_count of memset() is zero. This is most likely an
+    // argument swap.
+
+    const Expr *FillChar = Call->getArg(1);
+    const Expr *ByteCount = Call->getArg(2);
+
+    // Return if `byte_count` is not zero at compile time.
+    llvm::APSInt Value1, Value2;
+    if (ByteCount->isValueDependent() ||
+        !ByteCount->EvaluateAsInt(Value2, *Result.Context) || Value2 != 0)
+      return;
+
+    // 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 (!FillChar->isValueDependent() &&
+        FillChar->EvaluateAsInt(Value1, *Result.Context) &&
+        (Value1 == 0 || Value1.isNegative()))
+      return;
+
+    // `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");
+    StringRef RHSString = tooling::fixit::getText(*ByteCount, *Result.Context);
+    StringRef LHSString = tooling::fixit::getText(*FillChar, *Result.Context);
+    if (LHSString.empty() || RHSString.empty())
+      return;
+
+    D << tooling::fixit::createReplacement(*FillChar, RHSString)
+      << tooling::fixit::createReplacement(*ByteCount, LHSString);
+  }
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/bugprone/SuspiciousMemsetUsageCheck.h

@@ -0,0 +1,35 @@
+//===--- SuspiciousMemsetUsageCheck.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_BUGPRONE_SUSPICIOUS_MEMSET_USAGE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUS_MEMSET_USAGE_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Finds memset calls with potential mistakes in their arguments.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-suspicious-memset-usage.html
+class SuspiciousMemsetUsageCheck : public ClangTidyCheck {
+public:
+  SuspiciousMemsetUsageCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUS_MEMSET_USAGE_H

clang-tools-extra/clang-tidy/google/CMakeLists.txt

@@ -8,7 +8,6 @@ add_clang_library(clangTidyGoogleModule
   GlobalNamesInHeadersCheck.cpp
   GoogleTidyModule.cpp
   IntegerTypesCheck.cpp
-  MemsetZeroLengthCheck.cpp
   NonConstReferences.cpp
   OverloadedUnaryAndCheck.cpp
   StringReferenceMemberCheck.cpp

clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp

@@ -20,7 +20,6 @@
 #include "ExplicitMakePairCheck.h"
 #include "GlobalNamesInHeadersCheck.h"
 #include "IntegerTypesCheck.h"
-#include "MemsetZeroLengthCheck.h"
 #include "NonConstReferences.h"
 #include "OverloadedUnaryAndCheck.h"
 #include "StringReferenceMemberCheck.h"
@@ -55,8 +54,6 @@ public:
         "google-runtime-references");
     CheckFactories.registerCheck<runtime::StringReferenceMemberCheck>(
         "google-runtime-member-string-references");
-    CheckFactories.registerCheck<runtime::MemsetZeroLengthCheck>(
-        "google-runtime-memset");
     CheckFactories.registerCheck<readability::AvoidCStyleCastsCheck>(
         "google-readability-casting");
     CheckFactories.registerCheck<readability::TodoCommentCheck>(

clang-tools-extra/clang-tidy/google/MemsetZeroLengthCheck.cpp

@@ -1,96 +0,0 @@
-//===--- 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/AST/ASTContext.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "clang/ASTMatchers/ASTMatchers.h"
-#include "clang/Lex/Lexer.h"
-
-using namespace clang::ast_matchers;
-
-namespace clang {
-namespace tidy {
-namespace google {
-namespace runtime {
-
-void MemsetZeroLengthCheck::registerMatchers(
-    ast_matchers::MatchFinder *Finder) {
-  // 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(isInTemplateInstantiation()))
-                         .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");
-
-  // 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);
-
-  // 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;
-
-  // 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()))
-    return;
-
-  // `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();
-  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 google
-} // namespace tidy
-} // namespace clang

clang-tools-extra/clang-tidy/google/MemsetZeroLengthCheck.h

@@ -1,39 +0,0 @@
-//===--- 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_MEMSETZEROLENGTHCHECK_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_MEMSETZEROLENGTHCHECK_H
-
-#include "../ClangTidy.h"
-
-namespace clang {
-namespace tidy {
-namespace google {
-namespace runtime {
-
-/// 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:
-  MemsetZeroLengthCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
-  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
-  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
-};
-
-} // namespace runtime
-} // namespace google
-} // namespace tidy
-} // namespace clang
-
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_MEMSETZEROLENGTHCHECK_H

clang-tools-extra/clang-tidy/tool/CMakeLists.txt

@@ -15,6 +15,7 @@ target_link_libraries(clang-tidy
   clangTidy
   clangTidyAndroidModule
   clangTidyBoostModule
+  clangTidyBugproneModule
   clangTidyCERTModule
   clangTidyCppCoreGuidelinesModule
   clangTidyGoogleModule

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

@@ -462,6 +462,11 @@ extern volatile int BoostModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED BoostModuleAnchorDestination =
     BoostModuleAnchorSource;
 
+// This anchor is used to force the linker to link the BugproneModule.
+extern volatile int BugproneModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED BugproneModuleAnchorDestination =
+    BugproneModuleAnchorSource;
+
 // This anchor is used to force the linker to link the LLVMModule.
 extern volatile int LLVMModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED LLVMModuleAnchorDestination =

clang-tools-extra/docs/ReleaseNotes.rst

@@ -79,6 +79,12 @@ Improvements to clang-tidy
   Checks if the required file flag ``SOCK_CLOEXEC`` is present in the argument of
   ``socket()``.
 
+- New `bugprone-suspicious-memset-usage
+  <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-suspicious-memset-usage.html>`_ check
+
+  Finds ``memset()`` calls with potential mistakes in their arguments.
+  Replaces and extends the ``google-runtime-memset`` check.
+
 - New `cert-dcl21-cpp
   <http://clang.llvm.org/extra/clang-tidy/checks/cert-dcl21-cpp.html>`_ check
 
@@ -157,6 +163,10 @@ Improvements to clang-tidy
 - Support clang-formatting of the code around applied fixes (``-format-style``
   command-line option).
 
+- New `bugprone` module
+
+  Adds checks that target bugprone code constructs.
+
 - New `hicpp` module
 
   Adds checks that implement the `High Integrity C++ Coding Standard <http://www.codingstandard.com/section/index/>`_ and other safety

clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-memset-usage.rst

@@ -0,0 +1,54 @@
+.. title:: clang-tidy - bugprone-suspicious-memset-usage
+
+bugprone-suspicious-memset-usage
+================================
+
+This check finds ``memset()`` calls with potential mistakes in their arguments.
+Considering the function as ``void* memset(void* destination, int fill_value,
+size_t byte_count)``, the following cases are covered:
+
+**Case 1: Fill value is a character ``'0'``**
+
+Filling up a memory area with ASCII code 48 characters is not customary,
+possibly integer zeroes were intended instead.
+The check offers a replacement of ``'0'`` with ``0``. Memsetting character
+pointers with ``'0'`` is allowed.
+
+**Case 2: Fill value is truncated**
+
+Memset converts ``fill_value`` to ``unsigned char`` before using it. If
+``fill_value`` is out of unsigned character range, it gets truncated
+and memory will not contain the desired pattern.
+
+**Case 3: Byte count is zero**
+
+Calling memset with a literal zero in its ``byte_count`` argument is likely
+to be unintended and swapped with ``fill_value``. The check offers to swap
+these two arguments.
+
+Corresponding cpplint.py check name: ``runtime/memset``.
+
+
+Examples:
+
+.. code-block:: c++
+
+  void foo() {
+    int i[5] = {1, 2, 3, 4, 5};
+    int *ip = i;
+    char c = '1';
+    char *cp = &c;
+    int v = 0;
+
+    // Case 1
+    memset(ip, '0', 1); // suspicious
+    memset(cp, '0', 1); // OK
+
+    // Case 2
+    memset(ip, 0xabcd, 1); // fill value gets truncated
+    memset(ip, 0x00, 1);   // OK
+
+    // Case 3
+    memset(ip, sizeof(int), v); // zero length, potentially swapped
+    memset(ip, 0, 1);           // OK
+  }

clang-tools-extra/docs/clang-tidy/checks/google-runtime-memset.rst

@@ -1,10 +0,0 @@
-.. title:: clang-tidy - google-runtime-memset
-
-google-runtime-memset
-=====================
-
-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`.

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -9,6 +9,7 @@ Clang-Tidy Checks
    android-cloexec-open
    android-cloexec-socket
    boost-use-to-string
+   bugprone-suspicious-memset-usage
    cert-dcl03-c (redirects to misc-static-assert) <cert-dcl03-c>
    cert-dcl21-cpp
    cert-dcl50-cpp
@@ -55,7 +56,6 @@ Clang-Tidy Checks
    google-readability-todo
    google-runtime-int
    google-runtime-member-string-references
-   google-runtime-memset
    google-runtime-operator
    google-runtime-references
    hicpp-explicit-conversions

clang-tools-extra/docs/clang-tidy/index.rst

@@ -57,6 +57,7 @@ Name prefix            Description
 ====================== =========================================================
 ``android-``           Checks related to Android.
 ``boost-``             Checks related to Boost library.
+``bugprone-``          Checks that target bugprone code constructs.
 ``cert-``              Checks related to CERT Secure Coding Guidelines.
 ``cppcoreguidelines-`` Checks related to C++ Core Guidelines.
 ``clang-analyzer-``    Clang Static Analyzer checks.

clang-tools-extra/test/clang-tidy/bugprone-suspicious-memset-usage.cpp

@@ -0,0 +1,77 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-memset-usage %t
+
+void *memset(void *, int, __SIZE_TYPE__);
+
+namespace std {
+  using ::memset;
+}
+
+template <typename T>
+void mtempl(int *ptr) {
+  memset(ptr, '0', sizeof(T));
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: memset fill value is char '0', potentially mistaken for int 0 [bugprone-suspicious-memset-usage]
+// CHECK-FIXES: memset(ptr, 0, sizeof(T));
+  memset(ptr, 256, sizeof(T));
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: memset fill value is out of unsigned character range, gets truncated [bugprone-suspicious-memset-usage]
+  memset(0, sizeof(T), 0);
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset of size zero, potentially swapped arguments [bugprone-suspicious-memset-usage]
+// CHECK-FIXES: memset(0, 0, sizeof(T));
+  memset(0, sizeof(int), 0);
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset of size zero, potentially swapped arguments [bugprone-suspicious-memset-usage]
+// CHECK-FIXES: memset(0, 0, sizeof(int));
+}
+
+void foo(int xsize, int ysize) {
+  int i[5] = {1, 2, 3, 4, 5};
+  char ca[3] = {'a', 'b', 'c'};
+  int *p = i;
+  int l = 5;
+  char z = '1';
+  char *c = &z;
+  int v = 0;
+
+  memset(p, '0', l);
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: memset fill value is char '0', potentially mistaken for int 0 [bugprone-suspicious-memset-usage]
+// CHECK-FIXES: memset(p, 0, l);
+
+  memset(p, 0xabcd, l);
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: memset fill value is out of unsigned character range, gets truncated [bugprone-suspicious-memset-usage]
+
+  memset(p, sizeof(int), 0);
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset of size zero, potentially swapped arguments [bugprone-suspicious-memset-usage]
+// CHECK-FIXES: memset(p, 0, sizeof(int));
+  std::memset(p, sizeof(int), 0x00);
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset of size zero, potentially swapped arguments [bugprone-suspicious-memset-usage]
+// CHECK-FIXES: std::memset(p, 0x00, sizeof(int));
+
+#define M_CHAR_ZERO memset(p, '0', l);
+  M_CHAR_ZERO
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset fill value is char '0', potentially mistaken for int 0 [bugprone-suspicious-memset-usage]
+
+#define M_OUTSIDE_RANGE memset(p, 0xabcd, l);
+  M_OUTSIDE_RANGE
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset fill value is out of unsigned character range, gets truncated [bugprone-suspicious-memset-usage]
+
+#define M_ZERO_LENGTH memset(p, sizeof(int), 0);
+  M_ZERO_LENGTH
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset of size zero, potentially swapped arguments [bugprone-suspicious-memset-usage]
+
+  memset(p, '2', l);
+  memset(p, 0, l);
+  memset(c, '0', 1);
+  memset(ca, '0', sizeof(ca));
+
+  memset(p, 0x00, l);
+  mtempl<int>(p);
+
+  memset(p, sizeof(int), v + 1);
+  memset(p, 0xcd, 1);
+
+  // Don't warn when the fill char and the length are both known to be
+  // zero.  No bug is possible.
+  memset(p, 0, v);
+
+  // -1 is clearly not a length by virtue of being negative, so no warning
+  // despite v == 0.
+  memset(p, -1, v);
+}

clang-tools-extra/test/clang-tidy/google-runtime-memset-zero-length.cpp

@@ -1,62 +0,0 @@
-// RUN: %check_clang_tidy %s google-runtime-memset %t
-
-void *memset(void *, int, __SIZE_TYPE__);
-
-namespace std {
-  using ::memset;
-}
-
-template <int i, typename T>
-void memtmpl() {
-  memset(0, sizeof(int), i);
-  memset(0, sizeof(T), sizeof(T));
-  memset(0, sizeof(T), 0);
-// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: memset of size zero, potentially swapped argument
-// CHECK-FIXES: memset(0, 0, sizeof(T));
-  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);
-
-  // 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);
-
-  // -1 is clearly not a length by virtue of being negative, so no warning
-  // despite v == 0.
-  memset(a, -1, v);
-
-  memtmpl<0, int>();
-}