From e8a4c74f1157017b22d1579b6ac80fb4a634bc2a Mon Sep 17 00:00:00 2001 From: =?utf8?q?Bal=C3=A1zs=20K=C3=A9ri?= <1.int32@gmail.com> Date: Mon, 18 Nov 2019 16:02:36 +0100 Subject: [PATCH] [clang-tidy] Added DefaultOperatorNewCheck. Summary: Added new checker 'cert-default-operator-new' that checks for CERT rule MEM57-CPP. Simple version. Reviewers: aaron.ballman, alexfh, JonasToth, lebedev.ri Reviewed By: aaron.ballman Subscribers: hiraditya, martong, mehdi_amini, mgorny, inglorion, xazax.hun, dkrupp, steven_wu, dexonsmith, Szelethus, gamesh411, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D67545 --- .../clang-tidy/cert/CERTTidyModule.cpp | 14 ++-- clang-tools-extra/clang-tidy/cert/CMakeLists.txt | 1 + .../cert/DefaultOperatorNewAlignmentCheck.cpp | 74 ++++++++++++++++++++++ .../cert/DefaultOperatorNewAlignmentCheck.h | 35 ++++++++++ clang-tools-extra/docs/ReleaseNotes.rst | 6 ++ .../docs/clang-tidy/checks/cert-mem57-cpp.rst | 16 +++++ clang-tools-extra/docs/clang-tidy/checks/list.rst | 1 + .../test/clang-tidy/cert-mem57-cpp-cpp17.cpp | 12 ++++ .../test/clang-tidy/cert-mem57-cpp.cpp | 39 ++++++++++++ 9 files changed, 193 insertions(+), 5 deletions(-) create mode 100644 clang-tools-extra/clang-tidy/cert/DefaultOperatorNewAlignmentCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/cert/DefaultOperatorNewAlignmentCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/cert-mem57-cpp.rst create mode 100644 clang-tools-extra/test/clang-tidy/cert-mem57-cpp-cpp17.cpp create mode 100644 clang-tools-extra/test/clang-tidy/cert-mem57-cpp.cpp diff --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp index 2382044..3130dad 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( "cert-dcl59-cpp"); - // OOP - CheckFactories.registerCheck( - "cert-oop11-cpp"); - CheckFactories.registerCheck( - "cert-oop54-cpp"); // ERR CheckFactories.registerCheck( "cert-err09-cpp"); @@ -61,10 +57,18 @@ public: CheckFactories.registerCheck("cert-err60-cpp"); CheckFactories.registerCheck( "cert-err61-cpp"); + // MEM + CheckFactories.registerCheck( + "cert-mem57-cpp"); // MSC CheckFactories.registerCheck("cert-msc50-cpp"); CheckFactories.registerCheck( "cert-msc51-cpp"); + // OOP + CheckFactories.registerCheck( + "cert-oop11-cpp"); + CheckFactories.registerCheck( + "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 474d935..0363db7 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 0000000..48eee8e --- /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("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 0000000..1272987 --- /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 6d1aee4..b9feee2 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 + ` check. + + Checks if an object of type with extended alignment is allocated by using + the default ``operator new``. + - 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-mem57-cpp.rst b/clang-tools-extra/docs/clang-tidy/checks/cert-mem57-cpp.rst new file mode 100644 index 0000000..5a5c343 --- /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 +`_. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 9e6e347..4a12068 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-fio38-c (redirects to misc-non-copyable-objects) cert-flp30-c + cert-mem57-cpp cert-msc30-c (redirects to cert-msc50-cpp) cert-msc32-c (redirects to cert-msc51-cpp) 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 0000000..676cb64 --- /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 0000000..8142903 --- /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] +} -- 2.7.4