diff options
9 files changed, 193 insertions, 5 deletions
diff --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp index 23820446cd1..3130dad68ff 100644 --- a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp @@ -19,6 +19,7 @@ #include "../performance/MoveConstructorInitCheck.h" #include "../readability/UppercaseLiteralSuffixCheck.h" #include "CommandProcessorCheck.h" +#include "DefaultOperatorNewAlignmentCheck.h" #include "DontModifyStdNamespaceCheck.h" #include "FloatLoopCounter.h" #include "LimitedRandomnessCheck.h" @@ -48,11 +49,6 @@ public: "cert-dcl58-cpp"); CheckFactories.registerCheck<google::build::UnnamedNamespaceInHeaderCheck>( "cert-dcl59-cpp"); - // OOP - CheckFactories.registerCheck<performance::MoveConstructorInitCheck>( - "cert-oop11-cpp"); - CheckFactories.registerCheck<bugprone::UnhandledSelfAssignmentCheck>( - "cert-oop54-cpp"); // ERR CheckFactories.registerCheck<misc::ThrowByValueCatchByReferenceCheck>( "cert-err09-cpp"); @@ -61,10 +57,18 @@ public: CheckFactories.registerCheck<ThrownExceptionTypeCheck>("cert-err60-cpp"); CheckFactories.registerCheck<misc::ThrowByValueCatchByReferenceCheck>( "cert-err61-cpp"); + // MEM + CheckFactories.registerCheck<DefaultOperatorNewAlignmentCheck>( + "cert-mem57-cpp"); // MSC CheckFactories.registerCheck<LimitedRandomnessCheck>("cert-msc50-cpp"); CheckFactories.registerCheck<ProperlySeededRandomGeneratorCheck>( "cert-msc51-cpp"); + // OOP + CheckFactories.registerCheck<performance::MoveConstructorInitCheck>( + "cert-oop11-cpp"); + CheckFactories.registerCheck<bugprone::UnhandledSelfAssignmentCheck>( + "cert-oop54-cpp"); // C checkers // DCL diff --git a/clang-tools-extra/clang-tidy/cert/CMakeLists.txt b/clang-tools-extra/clang-tidy/cert/CMakeLists.txt index 474d9356adf..0363db7cf02 100644 --- a/clang-tools-extra/clang-tidy/cert/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/cert/CMakeLists.txt @@ -3,6 +3,7 @@ set(LLVM_LINK_COMPONENTS support) add_clang_library(clangTidyCERTModule CERTTidyModule.cpp CommandProcessorCheck.cpp + DefaultOperatorNewAlignmentCheck.cpp DontModifyStdNamespaceCheck.cpp FloatLoopCounter.cpp LimitedRandomnessCheck.cpp diff --git a/clang-tools-extra/clang-tidy/cert/DefaultOperatorNewAlignmentCheck.cpp b/clang-tools-extra/clang-tidy/cert/DefaultOperatorNewAlignmentCheck.cpp new file mode 100644 index 00000000000..48eee8e6949 --- /dev/null +++ b/clang-tools-extra/clang-tidy/cert/DefaultOperatorNewAlignmentCheck.cpp @@ -0,0 +1,74 @@ +//===--- DefaultOperatorNewCheck.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 "DefaultOperatorNewAlignmentCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace cert { + +AST_MATCHER(CXXNewExpr, isPlacementNew) { + return Node.getNumPlacementArgs() > 0; +} + +void DefaultOperatorNewAlignmentCheck::registerMatchers(MatchFinder *Finder) { + // Check not applicable in C++17 (or newer). + if (getLangOpts().CPlusPlus17) + return; + + Finder->addMatcher(cxxNewExpr(unless(isPlacementNew())).bind("new"), this); +} + +void DefaultOperatorNewAlignmentCheck::check( + const MatchFinder::MatchResult &Result) { + // Get the found 'new' expression. + const auto *NewExpr = Result.Nodes.getNodeAs<CXXNewExpr>("new"); + + QualType T = NewExpr->getAllocatedType(); + // Dependent types do not have fixed alignment. + if (T->isDependentType()) + return; + const TagDecl *D = T->getAsTagDecl(); + // Alignment can not be obtained for undefined type. + if (!D || !D->getDefinition() || !D->isCompleteDefinition()) + return; + + ASTContext &Context = D->getASTContext(); + + // Check if no alignment was specified for the type. + if (!Context.isAlignmentRequired(T)) + return; + + // The user-specified alignment (in bits). + unsigned SpecifiedAlignment = D->getMaxAlignment(); + // Double-check if no alignment was specified. + if (!SpecifiedAlignment) + return; + // The alignment used by default 'operator new' (in bits). + unsigned DefaultNewAlignment = Context.getTargetInfo().getNewAlign(); + + bool OverAligned = SpecifiedAlignment > DefaultNewAlignment; + bool HasDefaultOperatorNew = + !NewExpr->getOperatorNew() || NewExpr->getOperatorNew()->isImplicit(); + + unsigned CharWidth = Context.getTargetInfo().getCharWidth(); + if (HasDefaultOperatorNew && OverAligned) + diag(NewExpr->getBeginLoc(), + "allocation function returns a pointer with alignment %0 but the " + "over-aligned type being allocated requires alignment %1") + << (DefaultNewAlignment / CharWidth) + << (SpecifiedAlignment / CharWidth); +} + +} // namespace cert +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/cert/DefaultOperatorNewAlignmentCheck.h b/clang-tools-extra/clang-tidy/cert/DefaultOperatorNewAlignmentCheck.h new file mode 100644 index 00000000000..12729874411 --- /dev/null +++ b/clang-tools-extra/clang-tidy/cert/DefaultOperatorNewAlignmentCheck.h @@ -0,0 +1,35 @@ +//===--- DefaultOperatorNewCheck.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_DEFAULTOPERATORNEWALIGNMENTCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_DEFAULTOPERATORNEWALIGNMENTCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace cert { + +/// Checks if an object of type with extended alignment is allocated by using +/// the default operator new. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/cert-mem57-cpp.html +class DefaultOperatorNewAlignmentCheck : public ClangTidyCheck { +public: + DefaultOperatorNewAlignmentCheck(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_DEFAULTOPERATORNEWALIGNMENTCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 6d1aee4c95d..b9feee29d13 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -94,6 +94,12 @@ Improvements to clang-tidy Without the null terminator it can result in undefined behaviour when the string is read. +- New :doc:`cert-mem57-cpp + <clang-tidy/checks/cert-mem57-cpp>` check. + + Checks if an object of type with extended alignment is allocated by using + the default ``operator new``. + - New alias :doc:`cert-pos44-c <clang-tidy/checks/cert-pos44-c>` to :doc:`bugprone-bad-signal-to-kill-thread diff --git a/clang-tools-extra/docs/clang-tidy/checks/cert-mem57-cpp.rst b/clang-tools-extra/docs/clang-tidy/checks/cert-mem57-cpp.rst new file mode 100644 index 00000000000..5a5c343e50b --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/cert-mem57-cpp.rst @@ -0,0 +1,16 @@ +.. title:: clang-tidy - cert-mem57-cpp + +cert-mem57-cpp +============== + +This check flags uses of default ``operator new`` where the type has extended +alignment (an alignment greater than the fundamental alignment). (The default +``operator new`` is guaranteed to provide the correct alignmment if the +requested alignment is less or equal to the fundamental alignment). +Only cases are detected (by design) where the ``operator new`` is not +user-defined and is not a placement new (the reason is that in these cases we +assume that the user provided the correct memory allocation). + +This check corresponds to the CERT C++ Coding Standard rule +`MEM57-CPP. Avoid using default operator new for over-aligned types +<https://wiki.sei.cmu.edu/confluence/display/cplusplus/MEM57-CPP.+Avoid+using+default+operator+new+for+over-aligned+types>`_. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 9e6e3472d06..4a120684e3d 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -100,6 +100,7 @@ Clang-Tidy Checks cert-err61-cpp (redirects to misc-throw-by-value-catch-by-reference) <cert-err61-cpp> cert-fio38-c (redirects to misc-non-copyable-objects) <cert-fio38-c> cert-flp30-c + cert-mem57-cpp cert-msc30-c (redirects to cert-msc50-cpp) <cert-msc30-c> cert-msc32-c (redirects to cert-msc51-cpp) <cert-msc32-c> cert-msc50-cpp diff --git a/clang-tools-extra/test/clang-tidy/cert-mem57-cpp-cpp17.cpp b/clang-tools-extra/test/clang-tidy/cert-mem57-cpp-cpp17.cpp new file mode 100644 index 00000000000..676cb642d4a --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/cert-mem57-cpp-cpp17.cpp @@ -0,0 +1,12 @@ +// RUN: %check_clang_tidy %s -std=c++14 cert-mem57-cpp %t +// RUN: clang-tidy --extra-arg='-std=c++17' -checks='-*,cert-mem57-cpp' --warnings-as-errors='*' %s +// RUN: clang-tidy --extra-arg='-std=c++2a' -checks='-*,cert-mem57-cpp' --warnings-as-errors='*' %s + +struct alignas(32) Vector { + char Elems[32]; +}; + +void f() { + auto *V1 = new Vector; // CHECK-MESSAGES: warning: allocation function returns a pointer with alignment 16 but the over-aligned type being allocated requires alignment 32 [cert-mem57-cpp] + auto *V1_Arr = new Vector[2]; // CHECK-MESSAGES: warning: allocation function returns a pointer with alignment 16 but the over-aligned type being allocated requires alignment 32 [cert-mem57-cpp] +} diff --git a/clang-tools-extra/test/clang-tidy/cert-mem57-cpp.cpp b/clang-tools-extra/test/clang-tidy/cert-mem57-cpp.cpp new file mode 100644 index 00000000000..81429035327 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/cert-mem57-cpp.cpp @@ -0,0 +1,39 @@ +// RUN: %check_clang_tidy %s -std=c++14 cert-mem57-cpp %t + +namespace std { +typedef __typeof(sizeof(int)) size_t; +void *aligned_alloc(size_t, size_t); +void free(void *); +} // namespace std + +struct alignas(32) Vector1 { + char elems[32]; +}; + +struct Vector2 { + char elems[32]; +}; + +struct alignas(32) Vector3 { + char elems[32]; + static void *operator new(std::size_t nbytes) noexcept(true) { + return std::aligned_alloc(alignof(Vector3), nbytes); + } + static void operator delete(void *p) { + std::free(p); + } +}; + +struct alignas(8) Vector4 { + char elems[32]; +}; + +void f() { + auto *V1 = new Vector1; + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: allocation function returns a pointer with alignment 16 but the over-aligned type being allocated requires alignment 32 [cert-mem57-cpp] + auto *V2 = new Vector2; + auto *V3 = new Vector3; + auto *V4 = new Vector4; + auto *V1_Arr = new Vector1[2]; + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: allocation function returns a pointer with alignment 16 but the over-aligned type being allocated requires alignment 32 [cert-mem57-cpp] +} |
