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 @@ -24,6 +24,7 @@ #include "FloatLoopCounter.h" #include "LimitedRandomnessCheck.h" #include "MutatingCopyCheck.h" +#include "NotTrivialTypesLibcMemoryCallsCheck.h" #include "PostfixOperatorCheck.h" #include "ProperlySeededRandomGeneratorCheck.h" #include "SetLongJmpCheck.h" @@ -70,6 +71,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 + NotTrivialTypesLibcMemoryCallsCheck.cpp PostfixOperatorCheck.cpp ProperlySeededRandomGeneratorCheck.cpp SetLongJmpCheck.cpp diff --git a/clang-tools-extra/clang-tidy/cert/NotTrivialTypesLibcMemoryCallsCheck.h b/clang-tools-extra/clang-tidy/cert/NotTrivialTypesLibcMemoryCallsCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/cert/NotTrivialTypesLibcMemoryCallsCheck.h @@ -0,0 +1,40 @@ +//===--- NotTrivialTypesLibcMemoryCallsCheck.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_NOTTRIVIALTYPESLIBCMEMORYCALLSCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_NOTTRIVIALTYPESLIBCMEMORYCALLSCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace cert { + +/// This check flags use of the c standard library functions 'memset', 'memcpy', +/// 'memmove', 'strcpy', 'memcmp' and 'strcmp' on non trivial types. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/cert-oop57-cpp.html +class NotTrivialTypesLibcMemoryCallsCheck : public ClangTidyCheck { +public: + NotTrivialTypesLibcMemoryCallsCheck(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/NotTrivialTypesLibcMemoryCallsCheck.cpp b/clang-tools-extra/clang-tidy/cert/NotTrivialTypesLibcMemoryCallsCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/cert/NotTrivialTypesLibcMemoryCallsCheck.cpp @@ -0,0 +1,145 @@ +//===--- NotTrivialTypesLibcMemoryCallsCheck.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 "NotTrivialTypesLibcMemoryCallsCheck.h" +#include "../utils/OptionsUtils.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.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(); +} +} // namespace + +static const char BuiltinMemSet[] = "::std::memset;" + "::memset;"; +static const char BuiltinMemCpy[] = "::std::memcpy;" + "::memcpy;" + "::std::memmove;" + "::memmove;" + "::std::strcpy;" + "::strcpy;"; +static const char BuiltinMemCmp[] = "::std::memcmp;" + "::memcmp;" + "::std::strcmp;" + "::strcmp;"; +static constexpr llvm::StringRef EqualityComparison[] = {"operator==", + "operator!="}; + +static std::string getName(const CallExpr &Caller) { + llvm::SmallVector Buffer; + llvm::raw_svector_ostream OS(Buffer); + cast(Caller.getCalleeDecl())->printQualifiedName(OS); + return OS.str().trim(':'); +} + +static std::vector toStringRefVec(const std::vector & Items){ + return std::vector(Items.begin(), Items.end()); +} + +NotTrivialTypesLibcMemoryCallsCheck::NotTrivialTypesLibcMemoryCallsCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + MemSetNames(Options.get("MemSetNames", "")), + MemCpyNames(Options.get("MemCpyNames", "")), + MemCmpNames(Options.get("MemCmpNames", "")) {} + +void NotTrivialTypesLibcMemoryCallsCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "MemSetNames", MemSetNames); + Options.store(Opts, "MemCpyNames", MemCpyNames); + Options.store(Opts, "MemCmpNames", MemCmpNames); +} + +void NotTrivialTypesLibcMemoryCallsCheck::registerMatchers( + MatchFinder *Finder) { + + if (!getLangOpts().CPlusPlus) + 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 HasCxxRecordArgs = [&](Matcher RecordConstraint, + BindableMatcher SecondArg) { + return allOf(argumentCountIs(3), + hasArgument(0, IsStructPointer(RecordConstraint, true)), + hasArgument(1, SecondArg), hasArgument(2, IsRecordSizeOf)); + }; + std::vector MemSet = utils::options::parseStringList( + (llvm::Twine(BuiltinMemSet) + MemSetNames).str()); + std::vector MemCpy = utils::options::parseStringList( + (llvm::Twine(BuiltinMemCpy) + MemCpyNames).str()); + std::vector MemCmp = utils::options::parseStringList( + (llvm::Twine(BuiltinMemCmp) + MemCmpNames).str()); + + auto IsIntZero = expr(integerLiteral(equals(0))); + + Finder->addMatcher( + callExpr(callee(namedDecl(hasAnyName(toStringRefVec(MemSet)))), + HasCxxRecordArgs(unless(isTriviallyDefaultConstructible()), + IsIntZero)) + .bind("lazyConstruct"), + this); + Finder->addMatcher(callExpr(callee(namedDecl(hasAnyName(toStringRefVec(MemCpy)))), + HasCxxRecordArgs(unless(isTriviallyCopyable()), + IsStructPointer())) + .bind("lazyCopy"), + this); + Finder->addMatcher( + callExpr(callee(namedDecl(hasAnyName(toStringRefVec(MemCmp)))), + HasCxxRecordArgs(hasMethod(hasAnyName(EqualityComparison)), + IsStructPointer())) + .bind("lazyCompare"), + this); +} + +void NotTrivialTypesLibcMemoryCallsCheck::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") + << getName(*Caller); + } + if (const auto *Caller = Result.Nodes.getNodeAs("lazyCopy")) { + diag(Caller->getBeginLoc(), + "Calling '%0' on a non trivially copyable class is undefined") + << getName(*Caller); + } + if (const auto *Caller = Result.Nodes.getNodeAs("lazyCompare")) { + diag(Caller->getBeginLoc(), + "consider using comparison operators instead of calling '%0'") + << getName(*Caller); + } +} + +} // 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 @@ -106,6 +106,12 @@ Checks if an object of type with extended alignment is allocated by using the default ``operator new``. +- New :doc:`cert-oop57-cpp + ` check. + + Flags use of the c standard library functions ``memset``, ``memcpy``, + ``memmove``, ``strcpy``, ``memcmp`` and ``strcmp`` on non trivial types. + - New :doc:`cert-oop58-cpp ` check. 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,33 @@ +.. title:: clang-tidy - cert-oop57-cpp + +cert-oop57-cpp +============== + +Flags use of the c standard library functions ``memset``, ``memcpy``, +``memmove``, ``strcpy``, ``memcmp`` and ``strcmp`` on non trivial types. + +Options +------- + +.. option:: MemSetNames + + Specify extra functions to flag on that act similarily to memset. + Default is an empty string + The check will always detect the function: ``memset`` + +.. option:: MemCpyNames + + Specify extra functions to flag on that act similarily to memcpy. + Default is an empty string + The check will always detect the functions: ``memcpy``, ``memmove`` and ``strcpy`` + +.. option:: MemCmpNames + + Specify extra functions to flag on that act similarily to memcmp. + Default is an empty string + The check will always detect the functions: ``memcmp`` and ``strcmp`` + +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 @@ -99,6 +99,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 'std::memset' on a non trivially default constructible class is undefined + std::memset(&Data, 0, sizeof(Trivial)); + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: Calling 'std::memset' on a non trivially default constructible class is undefined + std::memcpy(&Data, &Other, sizeof(Data)); + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: Calling 'std::memcpy' on a non trivially copyable class is undefined + std::memmove(&Data, &Other, sizeof(Data)); + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: Calling 'std::memmove' on a non trivially copyable class is undefined + std::strcpy(&Data, &Other, sizeof(Data)); + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: Calling 'std::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 'std::memcmp' + std::strcmp(&Data, &Other, sizeof(Data)); + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: consider using comparison operators instead of calling 'std::strcmp' +} + +void bar(const NonTrivial &Other) { + NonTrivial Data; + std::memset(&Data, 0, sizeof(Data)); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Calling 'std::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 'std::memset' on a non trivially default constructible class is undefined + std::memcpy(&Data, &Other, sizeof(Data)); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Calling 'std::memcpy' on a non trivially copyable class is undefined + std::memmove(&Data, &Other, sizeof(Data)); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Calling 'std::memmove' on a non trivially copyable class is undefined + std::strcpy(&Data, &Other, sizeof(Data)); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Calling 'std::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 'std::memcmp' + std::strcmp(&Data, &Other, sizeof(Data)); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: consider using comparison operators instead of calling 'std::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' +}