diff --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp --- 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 @@ "cert-oop11-cpp"); CheckFactories.registerCheck( "cert-oop54-cpp"); + CheckFactories.registerCheck( + "cert-oop57-cpp"); CheckFactories.registerCheck( "cert-oop58-cpp"); diff --git a/clang-tools-extra/clang-tidy/cert/CMakeLists.txt b/clang-tools-extra/clang-tidy/cert/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/cert/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/cert/CMakeLists.txt @@ -8,6 +8,7 @@ FloatLoopCounter.cpp LimitedRandomnessCheck.cpp MutatingCopyCheck.cpp + NonTrivialTypesLibcMemoryCallsCheck.cpp PostfixOperatorCheck.cpp ProperlySeededRandomGeneratorCheck.cpp SetLongJmpCheck.cpp diff --git a/clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.h b/clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.h new file mode 100644 --- /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/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp b/clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp new file mode 100644 --- /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, + 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 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 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 RecordConstraint, + BindableMatcher 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("lazyConstruct")) { + diag(Caller->getBeginLoc(), "calling %0 on a non-trivially default " + "constructible class is undefined") + << cast(Caller->getCalleeDecl()); + } + if (const auto *Caller = Result.Nodes.getNodeAs("lazyCopy")) { + diag(Caller->getBeginLoc(), + "calling %0 on a non-trivially copyable class is undefined") + << cast(Caller->getCalleeDecl()); + } + if (const auto *Caller = Result.Nodes.getNodeAs("lazyCompare")) { + diag(Caller->getBeginLoc(), + "consider using comparison operators instead of calling %0") + << cast(Caller->getCalleeDecl()); + } +} + +} // namespace cert +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -74,6 +74,12 @@ ` check. Checks for usages of identifiers reserved for use by the implementation. + +- New :doc:`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 --- /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 +`_. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -105,6 +105,7 @@ `cert-mem57-cpp `_, `cert-msc50-cpp `_, `cert-msc51-cpp `_, + `cert-oop57-cpp `_, `cert-oop58-cpp `_, `clang-analyzer-core.DynamicTypePropagation `_, `clang-analyzer-core.uninitialized.CapturedBlockVariable `_, 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 --- /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' +}