[clang-tools-extra] a42c3eb - [clang-tidy] Add check for CERT-OOP57-CPP

via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 20 09:09:10 PST 2020


Author: Nathan
Date: 2020-01-20T17:09:03Z
New Revision: a42c3eb599cb3b83a07e6296d4ade213f1d74f0f

URL: https://github.com/llvm/llvm-project/commit/a42c3eb599cb3b83a07e6296d4ade213f1d74f0f
DIFF: https://github.com/llvm/llvm-project/commit/a42c3eb599cb3b83a07e6296d4ade213f1d74f0f.diff

LOG: [clang-tidy] Add check for CERT-OOP57-CPP

Summary:
This is a very basic warning implementation of [[ https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP57-CPP.+Prefer+special+member+functions+and+overloaded+operators+to+C+Standard+Library+functions | Prefer special member functions and overloaded operators to C Standard Library functions ]]

It absolutely needs some fine tuning though.

Reviewers: alexfh, hokein, aaron.ballman, JonasToth

Reviewed By: aaron.ballman

Subscribers: merge_guards_bot, Eugene.Zelenko, mgorny, xazax.hun, cfe-commits

Tags: #clang, #clang-tools-extra

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

Added: 
    clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
    clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.h
    clang-tools-extra/docs/clang-tidy/checks/cert-oop57-cpp.rst
    clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp

Modified: 
    clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
    clang-tools-extra/clang-tidy/cert/CMakeLists.txt
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/docs/clang-tidy/checks/list.rst

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
index ab1fbbe94402..226526d31970 100644
--- a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
@@ -25,6 +25,7 @@
 #include "FloatLoopCounter.h"
 #include "LimitedRandomnessCheck.h"
 #include "MutatingCopyCheck.h"
+#include "NonTrivialTypesLibcMemoryCallsCheck.h"
 #include "PostfixOperatorCheck.h"
 #include "ProperlySeededRandomGeneratorCheck.h"
 #include "SetLongJmpCheck.h"
@@ -73,6 +74,8 @@ class CERTModule : public ClangTidyModule {
         "cert-oop11-cpp");
     CheckFactories.registerCheck<bugprone::UnhandledSelfAssignmentCheck>(
         "cert-oop54-cpp");
+    CheckFactories.registerCheck<NonTrivialTypesLibcMemoryCallsCheck>(
+        "cert-oop57-cpp");
     CheckFactories.registerCheck<MutatingCopyCheck>(
         "cert-oop58-cpp");
 

diff  --git a/clang-tools-extra/clang-tidy/cert/CMakeLists.txt b/clang-tools-extra/clang-tidy/cert/CMakeLists.txt
index 66ea2a13acdd..91f329d2b034 100644
--- a/clang-tools-extra/clang-tidy/cert/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/cert/CMakeLists.txt
@@ -8,6 +8,7 @@ add_clang_library(clangTidyCERTModule
   FloatLoopCounter.cpp
   LimitedRandomnessCheck.cpp
   MutatingCopyCheck.cpp
+  NonTrivialTypesLibcMemoryCallsCheck.cpp
   PostfixOperatorCheck.cpp
   ProperlySeededRandomGeneratorCheck.cpp
   SetLongJmpCheck.cpp

diff  --git a/clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp b/clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
new file mode 100644
index 000000000000..a7bd38173eb0
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
@@ -0,0 +1,152 @@
+//===--- NonTrivialTypesLibcMemoryCallsCheck.cpp - clang-tidy ----------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "NonTrivialTypesLibcMemoryCallsCheck.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/Decl.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchersInternal.h"
+#include "clang/ASTMatchers/ASTMatchersMacros.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cert {
+
+namespace {
+AST_MATCHER(CXXRecordDecl, isTriviallyDefaultConstructible) {
+  return Node.hasTrivialDefaultConstructor();
+}
+AST_MATCHER(CXXRecordDecl, isTriviallyCopyable) {
+  return Node.hasTrivialCopyAssignment() && Node.hasTrivialCopyConstructor();
+}
+AST_MATCHER_P(NamedDecl, hasAnyNameStdString, std::vector<std::string>,
+              String) {
+  return ast_matchers::internal::HasNameMatcher(String).matchesNode(Node);
+}
+} // namespace
+
+static const char BuiltinMemSet[] = "::std::memset;"
+                                    "::memset;";
+static const char BuiltinMemCpy[] = "::std::memcpy;"
+                                    "::memcpy;"
+                                    "::std::memmove;"
+                                    "::memmove;"
+                                    "::std::strcpy;"
+                                    "::strcpy;"
+                                    "::memccpy;"
+                                    "::stpncpy;"
+                                    "::strncpy;";
+static const char BuiltinMemCmp[] = "::std::memcmp;"
+                                    "::memcmp;"
+                                    "::std::strcmp;"
+                                    "::strcmp;"
+                                    "::strncmp;";
+static constexpr llvm::StringRef ComparisonOperators[] = {
+    "operator==", "operator!=", "operator<",
+    "operator>",  "operator<=", "operator>="};
+
+static std::vector<std::string> parseStringListPair(StringRef LHS,
+                                                    StringRef RHS) {
+  if (LHS.empty()) {
+    if (RHS.empty())
+      return {};
+    return utils::options::parseStringList(RHS);
+  }
+  if (RHS.empty())
+    return utils::options::parseStringList(LHS);
+  llvm::SmallString<512> Buffer;
+  return utils::options::parseStringList((LHS + RHS).toStringRef(Buffer));
+}
+
+NonTrivialTypesLibcMemoryCallsCheck::NonTrivialTypesLibcMemoryCallsCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      MemSetNames(Options.get("MemSetNames", "")),
+      MemCpyNames(Options.get("MemCpyNames", "")),
+      MemCmpNames(Options.get("MemCmpNames", "")) {}
+
+void NonTrivialTypesLibcMemoryCallsCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "MemSetNames", MemSetNames);
+  Options.store(Opts, "MemCpyNames", MemCpyNames);
+  Options.store(Opts, "MemCmpNames", MemCmpNames);
+}
+
+void NonTrivialTypesLibcMemoryCallsCheck::registerMatchers(
+    MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC)
+    return;
+
+  using namespace ast_matchers::internal;
+  auto IsStructPointer = [](Matcher<CXXRecordDecl> Constraint = anything(),
+                            bool Bind = false) {
+    return expr(unaryOperator(
+        hasOperatorName("&"),
+        hasUnaryOperand(declRefExpr(
+            allOf(hasType(cxxRecordDecl(Constraint)),
+                  hasType(Bind ? qualType().bind("Record") : qualType()))))));
+  };
+  auto IsRecordSizeOf =
+      expr(sizeOfExpr(hasArgumentOfType(equalsBoundNode("Record"))));
+  auto ArgChecker = [&](Matcher<CXXRecordDecl> RecordConstraint,
+                        BindableMatcher<Stmt> SecondArg) {
+    return allOf(argumentCountIs(3),
+                 hasArgument(0, IsStructPointer(RecordConstraint, true)),
+                 hasArgument(1, SecondArg), hasArgument(2, IsRecordSizeOf));
+  };
+
+  Finder->addMatcher(
+      callExpr(callee(namedDecl(hasAnyNameStdString(
+                   parseStringListPair(BuiltinMemSet, MemSetNames)))),
+               ArgChecker(unless(isTriviallyDefaultConstructible()),
+                          expr(integerLiteral(equals(0)))))
+          .bind("lazyConstruct"),
+      this);
+  Finder->addMatcher(
+      callExpr(callee(namedDecl(hasAnyNameStdString(
+                   parseStringListPair(BuiltinMemCpy, MemCpyNames)))),
+               ArgChecker(unless(isTriviallyCopyable()), IsStructPointer()))
+          .bind("lazyCopy"),
+      this);
+  Finder->addMatcher(
+      callExpr(callee(namedDecl(hasAnyNameStdString(
+                   parseStringListPair(BuiltinMemCmp, MemCmpNames)))),
+               ArgChecker(hasMethod(hasAnyName(ComparisonOperators)),
+                          IsStructPointer()))
+          .bind("lazyCompare"),
+      this);
+}
+
+void NonTrivialTypesLibcMemoryCallsCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  if (const auto *Caller = Result.Nodes.getNodeAs<CallExpr>("lazyConstruct")) {
+    diag(Caller->getBeginLoc(), "calling %0 on a non-trivially default "
+                                "constructible class is undefined")
+        << cast<NamedDecl>(Caller->getCalleeDecl());
+  }
+  if (const auto *Caller = Result.Nodes.getNodeAs<CallExpr>("lazyCopy")) {
+    diag(Caller->getBeginLoc(),
+         "calling %0 on a non-trivially copyable class is undefined")
+        << cast<NamedDecl>(Caller->getCalleeDecl());
+  }
+  if (const auto *Caller = Result.Nodes.getNodeAs<CallExpr>("lazyCompare")) {
+    diag(Caller->getBeginLoc(),
+         "consider using comparison operators instead of calling %0")
+        << cast<NamedDecl>(Caller->getCalleeDecl());
+  }
+}
+
+} // namespace cert
+} // namespace tidy
+} // namespace clang

diff  --git a/clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.h b/clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.h
new file mode 100644
index 000000000000..2675e27b11ac
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.h
@@ -0,0 +1,41 @@
+//===--- NonTrivialTypesLibcMemoryCallsCheck.h - clang-tidy -----*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_NONTRIVIALTYPESLIBCMEMORYCALLSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_NONTRIVIALTYPESLIBCMEMORYCALLSCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace cert {
+
+/// Flags use of the `C` standard library functions 'memset', 'memcpy' and
+/// 'memcmp' and similar derivatives on non-trivial types.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cert-oop57-cpp.html
+class NonTrivialTypesLibcMemoryCallsCheck : public ClangTidyCheck {
+public:
+  NonTrivialTypesLibcMemoryCallsCheck(StringRef Name,
+                                      ClangTidyContext *Context);
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  const std::string MemSetNames;
+  const std::string MemCpyNames;
+  const std::string MemCmpNames;
+};
+
+} // namespace cert
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_NOTTRIVIALTYPESLIBCMEMORYCALLSCHECK_H

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 49f0afc73c78..1e0f76b17c5c 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -74,6 +74,12 @@ New checks
   <clang-tidy/checks/bugprone-reserved-identifier>` check.
 
   Checks for usages of identifiers reserved for use by the implementation.
+  
+- New :doc:`cert-oop57-cpp
+  <clang-tidy/checks/cert-oop57-cpp>` check.
+  
+  Flags use of the `C` standard library functions ``memset``, ``memcpy`` and
+  ``memcmp`` and similar derivatives on non-trivial types.
 
 New aliases
 ^^^^^^^^^^^

diff  --git a/clang-tools-extra/docs/clang-tidy/checks/cert-oop57-cpp.rst b/clang-tools-extra/docs/clang-tidy/checks/cert-oop57-cpp.rst
new file mode 100644
index 000000000000..ca789766a6e2
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/cert-oop57-cpp.rst
@@ -0,0 +1,40 @@
+.. title:: clang-tidy - cert-oop57-cpp
+
+cert-oop57-cpp
+==============
+
+  Flags use of the `C` standard library functions ``memset``, ``memcpy`` and
+  ``memcmp`` and similar derivatives on non-trivial types.
+
+Options
+-------
+
+.. option:: MemSetNames
+
+   Specify extra functions to flag that act similarily to ``memset``.
+   Specify names in a semicolon delimited list.
+   Default is an empty string.
+   The check will detect the following functions:
+   `memset`, `std::memset`.
+
+.. option:: MemCpyNames
+
+   Specify extra functions to flag that act similarily to ``memcpy``.
+   Specify names in a semicolon delimited list.
+   Default is an empty string.
+   The check will detect the following functions:
+   `std::memcpy`, `memcpy`, `std::memmove`, `memmove`, `std::strcpy`,
+   `strcpy`, `memccpy`, `stpncpy`, `strncpy`.
+
+.. option:: MemCmpNames
+
+   Specify extra functions to flag that act similarily to ``memcmp``.
+   Specify names in a semicolon delimited list.
+   Default is an empty string.
+   The check will detect the following functions:
+   `std::memcmp`, `memcmp`, `std::strcmp`, `strcmp`, `strncmp`.
+
+This check corresponds to the CERT C++ Coding Standard rule
+`OOP57-CPP. Prefer special member functions and overloaded operators to C 
+Standard Library functions
+<https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP57-CPP.+Prefer+special+member+functions+and+overloaded+operators+to+C+Standard+Library+functions>`_.

diff  --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 5787ffcac5e1..c958837ae4ba 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -105,6 +105,7 @@ Clang-Tidy Checks
    `cert-mem57-cpp <cert-mem57-cpp.html>`_,
    `cert-msc50-cpp <cert-msc50-cpp.html>`_,
    `cert-msc51-cpp <cert-msc51-cpp.html>`_,
+   `cert-oop57-cpp <cert-oop57-cpp.html>`_,
    `cert-oop58-cpp <cert-oop58-cpp.html>`_,
    `clang-analyzer-core.DynamicTypePropagation <clang-analyzer-core.DynamicTypePropagation.html>`_,
    `clang-analyzer-core.uninitialized.CapturedBlockVariable <clang-analyzer-core.uninitialized.CapturedBlockVariable.html>`_,

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp b/clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
new file mode 100644
index 000000000000..a5a38734ce6a
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
@@ -0,0 +1,90 @@
+// RUN: %check_clang_tidy %s cert-oop57-cpp %t -- \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: cert-oop57-cpp.MemSetNames, value: mymemset}, \
+// RUN:  {key: cert-oop57-cpp.MemCpyNames, value: mymemcpy}, \
+// RUN:  {key: cert-oop57-cpp.MemCmpNames, value: mymemcmp}]}' \
+// RUN: --
+
+void mymemset(void *, unsigned char, decltype(sizeof(int)));
+void mymemcpy(void *, const void *, decltype(sizeof(int)));
+int mymemcmp(const void *, const void *, decltype(sizeof(int)));
+
+namespace std {
+void memset(void *, unsigned char, decltype(sizeof(int)));
+void memcpy(void *, const void *, decltype(sizeof(int)));
+void memmove(void *, const void *, decltype(sizeof(int)));
+void strcpy(void *, const void *, decltype(sizeof(int)));
+int memcmp(const void *, const void *, decltype(sizeof(int)));
+int strcmp(const void *, const void *, decltype(sizeof(int)));
+} // namespace std
+
+struct Trivial {
+  int I;
+  int J;
+};
+
+struct NonTrivial {
+  int I;
+  int J;
+
+  NonTrivial() : I(0), J(0) {}
+  NonTrivial &operator=(const NonTrivial &Other) {
+    I = Other.I;
+    J = Other.J;
+    return *this;
+  }
+
+  bool operator==(const Trivial &Other) const {
+    return I == Other.I && J == Other.J;
+  }
+  bool operator!=(const Trivial &Other) const {
+    return !(*this == Other);
+  }
+};
+
+void foo(const Trivial &Other) {
+  Trivial Data;
+  std::memset(&Data, 0, sizeof(Data));
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: calling 'memset' on a non-trivially default constructible class is undefined
+  std::memset(&Data, 0, sizeof(Trivial));
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: calling 'memset' on a non-trivially default constructible class is undefined
+  std::memcpy(&Data, &Other, sizeof(Data));
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: calling 'memcpy' on a non-trivially copyable class is undefined
+  std::memmove(&Data, &Other, sizeof(Data));
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: calling 'memmove' on a non-trivially copyable class is undefined
+  std::strcpy(&Data, &Other, sizeof(Data));
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: calling 'strcpy' on a non-trivially copyable class is undefined
+  std::memcmp(&Data, &Other, sizeof(Data));
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: consider using comparison operators instead of calling 'memcmp'
+  std::strcmp(&Data, &Other, sizeof(Data));
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: consider using comparison operators instead of calling 'strcmp'
+}
+
+void bar(const NonTrivial &Other) {
+  NonTrivial Data;
+  std::memset(&Data, 0, sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling 'memset' on a non-trivially default constructible class is undefined
+  // Check it detects sizeof(Type) as well as sizeof(Instantiation)
+  std::memset(&Data, 0, sizeof(NonTrivial));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling 'memset' on a non-trivially default constructible class is undefined
+  std::memcpy(&Data, &Other, sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling 'memcpy' on a non-trivially copyable class is undefined
+  std::memmove(&Data, &Other, sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling 'memmove' on a non-trivially copyable class is undefined
+  std::strcpy(&Data, &Other, sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling 'strcpy' on a non-trivially copyable class is undefined
+  std::memcmp(&Data, &Other, sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: consider using comparison operators instead of calling 'memcmp'
+  std::strcmp(&Data, &Other, sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: consider using comparison operators instead of calling 'strcmp'
+}
+
+void baz(const NonTrivial &Other) {
+  NonTrivial Data;
+  mymemset(&Data, 0, sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling 'mymemset' on a non-trivially default constructible class is undefined
+  mymemcpy(&Data, &Other, sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling 'mymemcpy' on a non-trivially copyable class is undefined
+  mymemcmp(&Data, &Other, sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: consider using comparison operators instead of calling 'mymemcmp'
+}


        


More information about the cfe-commits mailing list