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,35 @@ +//===--- 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) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // 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,111 @@ +//===--- 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 "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 constexpr llvm::StringRef LazyConstruct[] = {"::std::memset", + "::memset"}; +static constexpr llvm::StringRef LazyCopy[] = {"::std::memcpy", "::memcpy", + "std::memmove", "::memmove", + "::std::strcpy", "::strcpy"}; +static constexpr llvm::StringRef LazyCompare[] = {"::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(':'); +} + +void NotTrivialTypesLibcMemoryCallsCheck::registerMatchers( + MatchFinder *Finder) { + 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)); + }; + + auto IsIntZero = expr(integerLiteral(equals(0))); + + Finder->addMatcher( + callExpr(callee(namedDecl(hasAnyName(LazyConstruct))), + HasCxxRecordArgs(unless(isTriviallyDefaultConstructible()), + IsIntZero)) + .bind("lazyConstruct"), + this); + Finder->addMatcher(callExpr(callee(namedDecl(hasAnyName(LazyCopy))), + HasCxxRecordArgs(unless(isTriviallyCopyable()), + IsStructPointer())) + .bind("lazyCopy"), + this); + Finder->addMatcher( + callExpr(callee(namedDecl(hasAnyName(LazyCompare))), + 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 @@ -67,6 +67,9 @@ Improvements to clang-tidy -------------------------- +- The 'bugprone-suspicous-constructor-and-assignment' check was renamed to :doc:`cert-not-trivial-types-libc-memory-calls + ` + - New :doc:`bugprone-bad-signal-to-kill-thread ` check. @@ -106,6 +109,11 @@ Checks if an object of type with extended alignment is allocated by using the default ``operator new``. +- New :doc:`cert-oop57-cpp + ` check. + Checks for calls to memset, memcpy, memmove, strcpy, memcmp and strcmp on non + trivial types. + - New alias :doc:`cert-pos44-c ` to :doc:`bugprone-bad-signal-to-kill-thread 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,12 @@ +.. title:: clang-tidy - cert-oop57-cpp + +cert-oop57-cpp +============== + +This check flags use of the c standard library functions 'memset', 'memcpy', +'memmove', 'strcpy', 'memcmp' and 'strcmp' on non trivial types. + +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 @@ -100,6 +100,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,71 @@ +// RUN: %check_clang_tidy %s cert-oop57-cpp %t + +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' +}