[clang-tidy] Added PlacementNewStorageCheck

[f00kat]

Summary

Added new checker 'cert-mem54-cpp' that checks for CERT rule MEM54-CPP.

I have troubles writing English descriptions for the release notes, so I need all the help I can get :)

[Eugene.Zelenko]

clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp

20

Please use static instead of anonymous namespaces for functions. See LLVM Coding Guidelines.

22

const auto *. Same in other places.

48

Belongs to isLanguageVersionSupported() now.

70

const auto *. Same below.

clang-tools-extra/docs/ReleaseNotes.rst

95

Please highlight new with double back-ticks.

clang-tools-extra/docs/clang-tidy/checks/cert-mem54-cpp.rst

7

Please highlight new with double back-ticks.

[f00kat]

1. Static functions instead of anonymous namespaces.
2. const auto*.
3. Added double back-ticks.

[f00kat]

Fix 'const auto*' in function getDescendantValueDecl

[Eugene.Zelenko]

clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp

43

Unnecessary empty line.

[njames93]

Also fix the test case, premerge found a failure

clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp

23

For better or worse children can sometimes contain nullptr, best to check for that first

52

Not gonna say this is blocking anything, but this is quite a large function that could be made smaller (more readable) by moving some of those lambdas out of the function

[aaron.ballman]

Have you considered making this a static analyzer check as opposed to a clang-tidy check? I think using dataflow analysis will really reduce the false positives because it will be able to track the allocation and alignment data across control flow.

[lebedev.ri]

This seems to be already handled by clang static analyzer? (clang-analyzer-cplusplus.PlacementNew)

<source>:19:5: warning: Storage provided to placement new is only 2 bytes, whereas the allocated type requires 8 bytes [clang-analyzer-cplusplus.PlacementNew]
    ::new (&s1) long;
    ^
<source>:19:5: note: Storage provided to placement new is only 2 bytes, whereas the allocated type requires 8 bytes
<source>:64:3: warning: Storage provided to placement new is only 2 bytes, whereas the allocated type requires 8 bytes [clang-analyzer-cplusplus.PlacementNew]
  ::new (buffer3) long;
  ^

https://godbolt.org/z/9VX5WW

[f00kat]

In D76229#1925268, @aaron.ballman wrote:

Have you considered making this a static analyzer check as opposed to a clang-tidy check? I think using dataflow analysis will really reduce the false positives because it will be able to track the allocation and alignment data across control flow.

I have never try to write static analyzer before. Okay, I will look at examples of how to work with dataflow.

[f00kat]

In D76229#1925360, @lebedev.ri wrote:

This seems to be already handled by clang static analyzer? (clang-analyzer-cplusplus.PlacementNew)

<source>:19:5: warning: Storage provided to placement new is only 2 bytes, whereas the allocated type requires 8 bytes [clang-analyzer-cplusplus.PlacementNew]
    ::new (&s1) long;
    ^
<source>:19:5: note: Storage provided to placement new is only 2 bytes, whereas the allocated type requires 8 bytes
<source>:64:3: warning: Storage provided to placement new is only 2 bytes, whereas the allocated type requires 8 bytes [clang-analyzer-cplusplus.PlacementNew]
  ::new (buffer3) long;
  ^

https://godbolt.org/z/9VX5WW

Oh no..It is sad :)

[f00kat]

In D76229#1925360, @lebedev.ri wrote:

This seems to be already handled by clang static analyzer? (clang-analyzer-cplusplus.PlacementNew)

<source>:19:5: warning: Storage provided to placement new is only 2 bytes, whereas the allocated type requires 8 bytes [clang-analyzer-cplusplus.PlacementNew]
    ::new (&s1) long;
    ^
<source>:19:5: note: Storage provided to placement new is only 2 bytes, whereas the allocated type requires 8 bytes
<source>:64:3: warning: Storage provided to placement new is only 2 bytes, whereas the allocated type requires 8 bytes [clang-analyzer-cplusplus.PlacementNew]
  ::new (buffer3) long;
  ^

https://godbolt.org/z/9VX5WW

But it seems like not all of my tests pass on static analyzer?

[lebedev.ri]

In D76229#1925371, @f00kat wrote:

In D76229#1925360, @lebedev.ri wrote:

This seems to be already handled by clang static analyzer? (clang-analyzer-cplusplus.PlacementNew)

<source>:19:5: warning: Storage provided to placement new is only 2 bytes, whereas the allocated type requires 8 bytes [clang-analyzer-cplusplus.PlacementNew]
    ::new (&s1) long;
    ^
<source>:19:5: note: Storage provided to placement new is only 2 bytes, whereas the allocated type requires 8 bytes
<source>:64:3: warning: Storage provided to placement new is only 2 bytes, whereas the allocated type requires 8 bytes [clang-analyzer-cplusplus.PlacementNew]
  ::new (buffer3) long;
  ^

https://godbolt.org/z/9VX5WW

But it seems like not all of my tests pass on static analyzer?

I have not really worked with static analyzer code, but assuming that those cases
that are no longer diagnosed as compared to this clang-tidy checks *should* be diagnosed
(i.e. diagnosing them isn't false-positive), then i'd think that static analyzer check
simply needs some work?

[f00kat]

In D76229#1925948, @lebedev.ri wrote:

In D76229#1925371, @f00kat wrote:

In D76229#1925360, @lebedev.ri wrote:

This seems to be already handled by clang static analyzer? (clang-analyzer-cplusplus.PlacementNew)

<source>:19:5: warning: Storage provided to placement new is only 2 bytes, whereas the allocated type requires 8 bytes [clang-analyzer-cplusplus.PlacementNew]
    ::new (&s1) long;
    ^
<source>:19:5: note: Storage provided to placement new is only 2 bytes, whereas the allocated type requires 8 bytes
<source>:64:3: warning: Storage provided to placement new is only 2 bytes, whereas the allocated type requires 8 bytes [clang-analyzer-cplusplus.PlacementNew]
  ::new (buffer3) long;
  ^

https://godbolt.org/z/9VX5WW

But it seems like not all of my tests pass on static analyzer?

I have not really worked with static analyzer code, but assuming that those cases
that are no longer diagnosed as compared to this clang-tidy checks *should* be diagnosed
(i.e. diagnosing them isn't false-positive), then i'd think that static analyzer check
simply needs some work?

Yeah. You are right. I will try to improve exist checker.
What we gonna do with this patch? Remove it?

[NoQ]

First of all, the static analyzer doesn't warn on everything at once; it stops the analysis after a fatal error (such as UB) is found, so it doesn't explore the execution path further. This is the reason why some tests look like they don't warn, but they will if you comment out the previous tests in the same function.

// bad (short is aligned to 2).
short* s6 = nullptr;
::new (s6) long;
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 's6' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]

Null pointer is obviously well-aligned, so this is a false positive / incorrect test (though for a syntactic check that focuses on a single statement it's a perfectly normal test).

// bad (just check for pointer to pointer).
short **s7;
::new (*s7) long;
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 's7' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]

There's another checker that warns here: warning: Dereference of undefined pointer value. So the analysis is interrupted before new is reached.

// bad (buffer is aligned to 'short' instead of 'long').
alignas(short) unsigned char buffer2[sizeof(long)];
::new (buffer2) long;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'buffer2' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]

Hmm, i think we don't have an insufficient alignment check yet, only an insufficient storage check. Same goes for all of the test3(). These shouldn't be too hard to add so that, at least, to pass the missing tests.

short* test4_f1()
{
    return nullptr;
}

void test4()
{
    ::new (test4_f1()) long;
    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'test4_f1' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]

Again, the static analyzer is inter-procedural so it knows that it's a null pointer.

short* (*p1)();
p1 = test4_f1;
::new (p1()) long;
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'p1' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]

We can even see through your obfuscation via function pointer! It's still null.

struct S
{
    short a;
};

// bad (not enough space).
const unsigned N = 32;
alignas(S) unsigned char buffer1[sizeof(S) * N];
::new (buffer1) S[N];
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 64 bytes are not enough for array allocation which requires 64 bytes [cert-mem54-cpp]

// maybe ok but we need to warn.
alignas(S) unsigned char buffer2[sizeof(S) * N + sizeof(int)];
::new (buffer2) S[N];
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: possibly not enough 68 bytes for array allocation which requires 64 bytes. current overhead requires the size of 4 bytes [cert-mem54-cpp]

Ah yes, placement array-new, we're still missing it but it shouldn't be hard to add.

[f00kat]

NoQ, I don`t want to say that existing static alalyzer is bad written or something. I just noticed that it does not yet support arrays and alignment.
Anyway thank you for your detailed feedback.

[NoQ]

Sure np, was just curious^^

[Charusso]

In D76229#1925363, @f00kat wrote:

In D76229#1925268, @aaron.ballman wrote:

Have you considered making this a static analyzer check as opposed to a clang-tidy check? I think using dataflow analysis will really reduce the false positives because it will be able to track the allocation and alignment data across control flow.

I have never try to write static analyzer before. Okay, I will look at examples of how to work with dataflow.

To getting started with the Analyzer please visit @NoQ's (tiny bit outdated) booklet: https://github.com/haoNoQ/clang-analyzer-guide

Advertisement:
If D69726 lands we will have an arsenal of APIs for helping dynamic size based checkers.
If D73521 lands it should be easier to getting started with the Analyzer, but now you could visit what is our current API.

[f00kat]

In D76229#1926818, @Charusso wrote:

In D76229#1925363, @f00kat wrote:

In D76229#1925268, @aaron.ballman wrote:

Have you considered making this a static analyzer check as opposed to a clang-tidy check? I think using dataflow analysis will really reduce the false positives because it will be able to track the allocation and alignment data across control flow.

I have never try to write static analyzer before. Okay, I will look at examples of how to work with dataflow.

To getting started with the Analyzer please visit @NoQ's (tiny bit outdated) booklet: https://github.com/haoNoQ/clang-analyzer-guide

Advertisement:
If D69726 lands we will have an arsenal of APIs for helping dynamic size based checkers.
If D73521 lands it should be easier to getting started with the Analyzer, but now you could visit what is our current API.

Thank you!

[martong]

Please note about the alignment requirements that they are not checked in the static analyzer for a reason. To track the alignment data across the data flow, probably we should modify the analyzer's core structure (e.g. MemRegion) and some modeling checkers like the MallocChecker. That is not an impossible task but also far from easy IMHO (would require several patches built on top of each other). Just as an example for the subtleties: think about the nuances of operator news default alignment and specific C++17 rules (https://reviews.llvm.org/D67545).

[NoQ]

In D76229#1929453, @martong wrote:

Please note about the alignment requirements that they are not checked in the static analyzer for a reason. To track the alignment data across the data flow, probably we should modify the analyzer's core structure (e.g. MemRegion) and some modeling checkers like the MallocChecker. That is not an impossible task but also far from easy IMHO (would require several patches built on top of each other). Just as an example for the subtleties: think about the nuances of operator news default alignment and specific C++17 rules (https://reviews.llvm.org/D67545).

This checker's tests only cover VarRegion-based regions. It should be easy to pick up the alignment from the AST for the VarDecl it corresponds to.

[lebedev.ri]

This review seems to be stuck/dead, consider abandoning if no longer relevant.

clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp

 #include "../readability/UppercaseLiteralSuffixCheck.h"
 #include "CommandProcessorCheck.h"
 #include "DefaultOperatorNewAlignmentCheck.h"
 #include "DontModifyStdNamespaceCheck.h"
 #include "FloatLoopCounter.h"
 #include "LimitedRandomnessCheck.h"
 #include "MutatingCopyCheck.h"
 #include "NonTrivialTypesLibcMemoryCallsCheck.h"
+#include "PlacementNewStorageCheck.h"
 #include "PostfixOperatorCheck.h"
 #include "ProperlySeededRandomGeneratorCheck.h"
 #include "SetLongJmpCheck.h"
 #include "StaticObjectExceptionCheck.h"
 #include "StrToNumCheck.h"
 #include "ThrownExceptionTypeCheck.h"
 #include "VariadicFunctionDefCheck.h"
 
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<misc::ThrowByValueCatchByReferenceCheck>(
         "cert-err09-cpp");
     CheckFactories.registerCheck<SetLongJmpCheck>("cert-err52-cpp");
     CheckFactories.registerCheck<StaticObjectExceptionCheck>("cert-err58-cpp");
     CheckFactories.registerCheck<ThrownExceptionTypeCheck>("cert-err60-cpp");
     CheckFactories.registerCheck<misc::ThrowByValueCatchByReferenceCheck>(
         "cert-err61-cpp");
     // MEM
+    CheckFactories.registerCheck<PlacementNewStorageCheck>("cert-mem54-cpp");
     CheckFactories.registerCheck<DefaultOperatorNewAlignmentCheck>(
         "cert-mem57-cpp");
     // MSC
     CheckFactories.registerCheck<LimitedRandomnessCheck>("cert-msc50-cpp");
     CheckFactories.registerCheck<ProperlySeededRandomGeneratorCheck>(
         "cert-msc51-cpp");
     // OOP
     CheckFactories.registerCheck<performance::MoveConstructorInitCheck>(

clang-tools-extra/clang-tidy/cert/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyCERTModule
   CERTTidyModule.cpp
   CommandProcessorCheck.cpp
   DefaultOperatorNewAlignmentCheck.cpp
   DontModifyStdNamespaceCheck.cpp
   FloatLoopCounter.cpp
   LimitedRandomnessCheck.cpp
   MutatingCopyCheck.cpp
   NonTrivialTypesLibcMemoryCallsCheck.cpp
+  PlacementNewStorageCheck.cpp
   PostfixOperatorCheck.cpp
   ProperlySeededRandomGeneratorCheck.cpp
   SetLongJmpCheck.cpp
   StaticObjectExceptionCheck.cpp
   StrToNumCheck.cpp
   ThrownExceptionTypeCheck.cpp
   VariadicFunctionDefCheck.cpp
 

clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.h

+//===--- PlacementNewStorageCheck.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_PLACEMENTNEWSTORAGECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_PLACEMENTNEWSTORAGECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace cert {
+
+/// Checks that placement new provided with properly aligned pointer to
+/// sufficient storage capacity
+class PlacementNewStorageCheck : public ClangTidyCheck {
+public:
+  PlacementNewStorageCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus;
+  }
+  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_PLACEMENTNEWSTORAGECHECK_H

clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp

+//===--- PlacementNewStorageCheck.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 "PlacementNewStorageCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cert {
+
+static const ValueDecl *getDescendantValueDecl(const Stmt *TheStmt) {
+  if (const auto *TheDeclRefExpr = dyn_cast<DeclRefExpr>(TheStmt))
+    return TheDeclRefExpr->getDecl();
+
+  for (const Stmt *Child : TheStmt->children()) {
+    if (const auto *TheDeclRefExpr = dyn_cast<DeclRefExpr>(Child))
+      return TheDeclRefExpr->getDecl();
+
+    if (const ValueDecl *TheValueDeclExpr = getDescendantValueDecl(Child))
+      return TheValueDeclExpr;
+  }
+
+  return nullptr;
+}
+
+static void unwindPointeeType(QualType &Type) {
+  if (Type->isPointerType())
+    Type = Type->getPointeeType();
+  else if (Type->isReferenceType())
+    Type = Type->getPointeeType();
+
+  if (Type->isPointerType() || Type->isReferenceType())
+    unwindPointeeType(Type);
+}
+
+
+void PlacementNewStorageCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+      cxxNewExpr(hasPlacementArg(0, implicitCastExpr(expr().bind("arg0"))))
+          .bind("new"),
+      this);
+}
+
+void PlacementNewStorageCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *NewExpr = Result.Nodes.getNodeAs<CXXNewExpr>("new");
+  const auto *NewExprArg0 = Result.Nodes.getNodeAs<ImplicitCastExpr>("arg0");
+
+  const ASTContext *Context = Result.Context;
+  const SourceManager *SourceManager = Result.SourceManager;
+  QualType AllocatedT = NewExpr->getAllocatedType();
+
+  auto CheckStorageTypeIsFine =
+      [this, &AllocatedT, NewExpr, Context,
+       SourceManager](const ValueDecl *StorageVD) -> void {
+    QualType StorageT = StorageVD->getType();
+
+    if (const auto *TheFunctionProtoType = StorageT->getAs<FunctionProtoType>())
+      StorageT = TheFunctionProtoType->getReturnType();
+    else if (const auto *ThePointerType = StorageT->getAs<PointerType>())
+      if (const auto *TheFunctionProtoType =
+              ThePointerType->getPointeeType()->getAs<FunctionProtoType>())
+        StorageT = TheFunctionProtoType->getReturnType();
+
+    unwindPointeeType(StorageT);
+
+    if (StorageT->isVoidPointerType() || StorageT->isVoidType())
+      return;
+
+    const std::size_t BitsPerByteCount = 8;
+
+    unsigned StorageTAlign = Context->getTypeAlign(StorageT);
+    if (unsigned SpecifiedAlignment = StorageVD->getMaxAlignment())
+      StorageTAlign = SpecifiedAlignment;
+
+    StorageTAlign = StorageTAlign / BitsPerByteCount;
+    unsigned AllocatedTAlign =
+        Context->getTypeAlign(AllocatedT) / BitsPerByteCount;
+
+    if (AllocatedTAlign > StorageTAlign) {
+      diag(NewExpr->getBeginLoc(), "%0 is aligned to %1 bytes but "
+                                   "allocated type %2 is aligned to %3 bytes")
+          << StorageVD << StorageTAlign << AllocatedT << AllocatedTAlign;
+
+      return;
+    }
+
+    uint64_t StorageTSize = Context->getTypeSize(StorageT) / BitsPerByteCount;
+
+    if (llvm::Optional<const Expr *> TheArraySizeExpr =
+            NewExpr->getArraySize()) {
+      Expr::EvalResult EvaluatedArraySizeValue;
+      if (!TheArraySizeExpr.getValue()->EvaluateAsInt(EvaluatedArraySizeValue,
+                                                      *Context) &&
+          !EvaluatedArraySizeValue.Val.isInt()) {
+        return;
+      }
+
+      uint64_t AllocatedSize =
+          (Context->getTypeSize(AllocatedT) *
+           (*EvaluatedArraySizeValue.Val.getInt().getRawData())) /
+          BitsPerByteCount;
+
+      if (AllocatedSize >= StorageTSize)
+        diag(NewExpr->getBeginLoc(), "%0 bytes are not enough for array "
+                                     "allocation which requires %1 bytes")
+            << (unsigned)AllocatedSize << (unsigned)StorageTSize;
+      else
+        diag(NewExpr->getBeginLoc(),
+             "possibly not enough %0 bytes for array allocation which requires "
+             "%1 bytes. current overhead requires the size of %2 bytes")
+            << (unsigned)StorageTSize << (unsigned)AllocatedSize
+            << (unsigned)(StorageTSize - AllocatedSize);
+    } else if (StorageT->isArrayType()) {
+      uint64_t AllocatedSize =
+          Context->getTypeSize(AllocatedT) / BitsPerByteCount;
+
+      if (AllocatedSize > StorageTSize)
+        diag(NewExpr->getBeginLoc(), "%0 bytes are not enough for allocation "
+                                     "type %1 which requires %2 bytes")
+            << (unsigned)StorageTSize << AllocatedT << (unsigned)AllocatedSize;
+    }
+  };
+
+  auto IsPointerAlignedProperly =
+      [this, &AllocatedT, NewExpr, Context,
+       SourceManager](const Expr *ThePointerAddressExpr) -> void {
+    Expr::EvalResult EvaluatedValue;
+    if (ThePointerAddressExpr->EvaluateAsInt(EvaluatedValue, *Context) &&
+        EvaluatedValue.Val.isInt()) {
+      unsigned AllocatedTAlign = Context->getTypeAlign(AllocatedT) / 8;
+      unsigned AddressAlign =
+          *EvaluatedValue.Val.getInt().getRawData() % AllocatedTAlign;
+      if (AddressAlign != 0) {
+        diag(NewExpr->getBeginLoc(),
+             "address is aligned to %0 bytes instead of %1 bytes")
+            << AddressAlign << AllocatedTAlign;
+      }
+    }
+  };
+
+  const Expr *StorageExpr = NewExprArg0->getSubExpr();
+
+  if (const auto *TheUnaryOperator = dyn_cast<UnaryOperator>(StorageExpr)) {
+    if (UnaryOperatorKind::UO_AddrOf == TheUnaryOperator->getOpcode()) {
+      if (const ValueDecl *TheValueDecl =
+              getDescendantValueDecl(TheUnaryOperator->getSubExpr())) {
+        CheckStorageTypeIsFine(TheValueDecl);
+      }
+    }
+  } else if (const auto *TheCallExpr = dyn_cast<CallExpr>(StorageExpr)) {
+    if (const ValueDecl *TheValueDecl = getDescendantValueDecl(TheCallExpr)) {
+      CheckStorageTypeIsFine(TheValueDecl);
+    }
+  } else if (const auto *TheCastExpr = dyn_cast<CastExpr>(StorageExpr)) {
+    const CastKind TheCastKind = TheCastExpr->getCastKind();
+    if (CastKind::CK_LValueToRValue == TheCastKind ||
+        CastKind::CK_ArrayToPointerDecay == TheCastKind) {
+      if (const ValueDecl *TheValueDecl =
+              getDescendantValueDecl(TheCastExpr->getSubExpr())) {
+        CheckStorageTypeIsFine(TheValueDecl);
+      }
+    } else if (CastKind::CK_IntegralToPointer == TheCastKind) {
+      IsPointerAlignedProperly(TheCastExpr->getSubExpr()->IgnoreParenCasts());
+    }
+  } else if (const auto *TheDeclRefExpr = dyn_cast<DeclRefExpr>(StorageExpr)) {
+    CheckStorageTypeIsFine(TheDeclRefExpr->getDecl());
+  }
+}
+
+} // namespace cert
+} // namespace tidy
+} // namespace clang

clang-tools-extra/docs/ReleaseNotes.rst

 
 - New :doc:`bugprone-suspicious-include
   <clang-tidy/checks/bugprone-suspicious-include>` check.
 
   Finds cases where an include refers to what appears to be an implementation
   file, which often leads to hard-to-track-down ODR violations, and diagnoses
   them.
 
+- New :doc:`cert-mem54-cpp
+  <clang-tidy/checks/cert-mem54-cpp>` check.
+
+  Checks that placement ``new`` provided with properly aligned pointer to
+  sufficient storage capacity.
+
 - New :doc:`cert-oop57-cpp
   <clang-tidy/checks/cert-oop57-cpp>` check.
 
   Flags use of the `C` standard library functions ``memset``, ``memcpy`` and
   ``memcmp`` and similar derivatives on non-trivial types.
 
 - New :doc:`objc-dealloc-in-category
   <clang-tidy/checks/objc-dealloc-in-category>` check.

clang-tools-extra/docs/clang-tidy/checks/cert-mem54-cpp.rst

+.. title:: clang-tidy - cert-mem54-cpp
+
+cert-mem54-cpp
+==============
+
+Checks that placement ``new`` provided with properly aligned pointer to sufficient storage capacity.
+
+This check corresponds to the CERT C++ Coding Standard rule
+`MEM54-CPP. Provide placement new with properly aligned pointers to sufficient storage capacity
+<https://wiki.sei.cmu.edu/confluence/display/cplusplus/MEM54-CPP.+Provide+placement+new+with+properly+aligned+pointers+to+sufficient+storage+capacity>`_.

clang-tools-extra/docs/clang-tidy/checks/list.rst

.. csv-table::
    `cert-dcl50-cpp <cert-dcl50-cpp.html>`_,
    `cert-dcl58-cpp <cert-dcl58-cpp.html>`_,
    `cert-env33-c <cert-env33-c.html>`_,
    `cert-err34-c <cert-err34-c.html>`_,
    `cert-err52-cpp <cert-err52-cpp.html>`_,
    `cert-err58-cpp <cert-err58-cpp.html>`_,
    `cert-err60-cpp <cert-err60-cpp.html>`_,
    `cert-flp30-c <cert-flp30-c.html>`_,
+   `cert-mem54-cpp <cert-mem54-cpp.html>`_,
    `cert-mem57-cpp <cert-mem57-cpp.html>`_,
    `cert-msc50-cpp <cert-msc50-cpp.html>`_,
    `cert-msc51-cpp <cert-msc51-cpp.html>`_,
    `cert-oop57-cpp <cert-oop57-cpp.html>`_,
    `cert-oop58-cpp <cert-oop58-cpp.html>`_,
    `clang-analyzer-core.DynamicTypePropagation <clang-analyzer-core.DynamicTypePropagation.html>`_,
    `clang-analyzer-core.uninitialized.CapturedBlockVariable <clang-analyzer-core.uninitialized.CapturedBlockVariable.html>`_,
    `clang-analyzer-cplusplus.InnerPointer <clang-analyzer-cplusplus.InnerPointer.html>`_,

clang-tools-extra/test/clang-tidy/checkers/cert-mem54-cpp.cpp

+// RUN: %check_clang_tidy %s cert-mem54-cpp %t
+
+inline void *operator new(size_t s, void *p) noexcept {
+  (void)s;
+  return p;
+}
+
+inline void *operator new[](size_t s, void *p) noexcept {
+  (void)s;
+  return p;
+}
+
+void test1()
+{
+    // bad (short is aligned to 2).
+    short s1;
+    ::new (&s1) long;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 's1' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+    // ok (same alignment because types are the same).
+    long s2;
+    ::new (&s2) long;
+
+    // ok (long long is greater then long).
+    long long s3;
+    ::new (&s3) long;
+
+    // bad (alias for type 'short' which is aligned to 2).
+    using s4t = short;
+    s4t s4;
+    ::new (&s4) long;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 's4' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+    // ok (skip void pointer).
+    void* s5 = nullptr;
+    ::new (s5) long;    
+
+    // bad (short is aligned to 2).
+    short* s6 = nullptr;
+    ::new (s6) long;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 's6' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+    
+    // bad (just check for pointer to pointer).
+    short **s7;
+    ::new (*s7) long;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 's7' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+}
+
+void test2()
+{
+  // ok.
+  alignas(long) unsigned char buffer1[sizeof(long)];
+  ::new (buffer1) long;
+
+  // bad (buffer is aligned to 'short' instead of 'long').
+  alignas(short) unsigned char buffer2[sizeof(long)];
+  ::new (buffer2) long;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'buffer2' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+  // bad (not enough space).
+  alignas(long) unsigned char buffer3[sizeof(short)];
+  ::new (buffer3) long;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 2 bytes are not enough for allocation type 'long' which requires 4 bytes [cert-mem54-cpp]
+
+  // bad (still not enough space).
+  alignas(long) unsigned char buffer4[sizeof(short) + 1];
+  ::new (buffer4) long;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 3 bytes are not enough for allocation type 'long' which requires 4 bytes [cert-mem54-cpp]
+}
+
+void test3()
+{
+    // ok (100 % 4 == 0).
+    ::new ((size_t*)100) long;
+
+    // bad (100 % 4 == 2).
+    ::new ((size_t*)102) long;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: address is aligned to 2 bytes instead of 4 bytes [cert-mem54-cpp]
+
+    ::new (reinterpret_cast<size_t*>(100)) long;
+
+    ::new (reinterpret_cast<size_t*>(102)) long;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: address is aligned to 2 bytes instead of 4 bytes [cert-mem54-cpp]
+
+    ::new ((size_t*)(100 + 0)) long;
+
+    ::new ((size_t*)(100 + 2)) long;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: address is aligned to 2 bytes instead of 4 bytes [cert-mem54-cpp]
+}
+
+short* test4_f1()
+{
+    return nullptr;
+}
+
+short& test4_f2()
+{
+    static short v;
+    return v;
+}
+
+void test4()
+{    
+    ::new (test4_f1()) long;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'test4_f1' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+    ::new (&test4_f2()) long;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'test4_f2' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+    short* (*p1)();
+    p1 = test4_f1;
+    ::new (p1()) long;
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'p1' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+}
+
+void test5()
+{
+  struct S
+  {
+      short a;
+  };
+
+  // bad (not enough space).
+  const unsigned N = 32;
+  alignas(S) unsigned char buffer1[sizeof(S) * N];
+  ::new (buffer1) S[N];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 64 bytes are not enough for array allocation which requires 64 bytes [cert-mem54-cpp]
+
+  // maybe ok but we need to warn.
+  alignas(S) unsigned char buffer2[sizeof(S) * N + sizeof(int)];
+  ::new (buffer2) S[N];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: possibly not enough 68 bytes for array allocation which requires 64 bytes. current overhead requires the size of 4 bytes [cert-mem54-cpp]
+}
+
+void test6() 
+{
+  struct A
+  {
+      char a;
+      char b;
+  } Ai;
+
+  // bad (struct A is aligned to char).
+  ::new (&Ai.a) long;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'Ai' is aligned to 1 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+  struct B
+  {
+      char a;
+      long b;
+  } Bi;
+
+  // ok (struct B is aligned to long).
+  ::new (&Bi.a) long;
+
+  struct C
+  {
+      char a;
+      alignas(2) char b;
+  } Ci;
+
+  // bad (struct C is aligned to 2).
+  ::new (&Ci.a) long;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'Ci' is aligned to 2 bytes but allocated type 'long' is aligned to 4 bytes [cert-mem54-cpp]
+
+  struct D
+  {
+      char a;
+      char b;
+      struct {
+          long c;
+      };
+  } Di;
+
+  // ok (struct D is aligned to long).
+  ::new (&Di.a) long;
+
+  struct alignas(alignof(long)) E {
+    char a;
+    char b;
+  } Ei;
+
+  // ok (struct E is aligned to long).
+  ::new (&Ei.a) long;
+}