[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-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

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

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

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

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

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>`_.

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

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]
+}

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]
+}