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 @@ -26,6 +26,7 @@ #include "LimitedRandomnessCheck.h" #include "MutatingCopyCheck.h" #include "NonTrivialTypesLibcMemoryCallsCheck.h" +#include "PlacementNewStorageCheck.h" #include "PostfixOperatorCheck.h" #include "ProperlySeededRandomGeneratorCheck.h" #include "SetLongJmpCheck.h" @@ -63,6 +64,7 @@ CheckFactories.registerCheck( "cert-err61-cpp"); // MEM + CheckFactories.registerCheck("cert-mem54-cpp"); CheckFactories.registerCheck( "cert-mem57-cpp"); // MSC 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 @@ -9,6 +9,7 @@ LimitedRandomnessCheck.cpp MutatingCopyCheck.cpp NonTrivialTypesLibcMemoryCallsCheck.cpp + PlacementNewStorageCheck.cpp PostfixOperatorCheck.cpp ProperlySeededRandomGeneratorCheck.cpp SetLongJmpCheck.cpp diff --git a/clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.h b/clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.h @@ -0,0 +1,35 @@ +//===--- 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 diff --git a/clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp b/clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/cert/PlacementNewStorageCheck.cpp @@ -0,0 +1,180 @@ +//===--- 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(TheStmt)) + return TheDeclRefExpr->getDecl(); + + for (const Stmt *Child : TheStmt->children()) { + if (const auto *TheDeclRefExpr = dyn_cast(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("new"); + const auto *NewExprArg0 = Result.Nodes.getNodeAs("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()) + StorageT = TheFunctionProtoType->getReturnType(); + else if (const auto *ThePointerType = StorageT->getAs()) + if (const auto *TheFunctionProtoType = + ThePointerType->getPointeeType()->getAs()) + 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 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(StorageExpr)) { + if (UnaryOperatorKind::UO_AddrOf == TheUnaryOperator->getOpcode()) { + if (const ValueDecl *TheValueDecl = + getDescendantValueDecl(TheUnaryOperator->getSubExpr())) { + CheckStorageTypeIsFine(TheValueDecl); + } + } + } else if (const auto *TheCallExpr = dyn_cast(StorageExpr)) { + if (const ValueDecl *TheValueDecl = getDescendantValueDecl(TheCallExpr)) { + CheckStorageTypeIsFine(TheValueDecl); + } + } else if (const auto *TheCastExpr = dyn_cast(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(StorageExpr)) { + CheckStorageTypeIsFine(TheDeclRefExpr->getDecl()); + } +} + +} // 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 @@ -89,6 +89,12 @@ file, which often leads to hard-to-track-down ODR violations, and diagnoses them. +- New :doc:`cert-mem54-cpp + ` check. + + Checks that placement ``new`` provided with properly aligned pointer to + sufficient storage capacity. + - New :doc:`cert-oop57-cpp ` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/cert-mem54-cpp.rst b/clang-tools-extra/docs/clang-tidy/checks/cert-mem54-cpp.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/cert-mem54-cpp.rst @@ -0,0 +1,10 @@ +.. 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 +`_. 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 @@ -105,6 +105,7 @@ `cert-err58-cpp `_, `cert-err60-cpp `_, `cert-flp30-c `_, + `cert-mem54-cpp `_, `cert-mem57-cpp `_, `cert-msc50-cpp `_, `cert-msc51-cpp `_, diff --git a/clang-tools-extra/test/clang-tidy/checkers/cert-mem54-cpp.cpp b/clang-tools-extra/test/clang-tidy/checkers/cert-mem54-cpp.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/cert-mem54-cpp.cpp @@ -0,0 +1,185 @@ +// 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(100)) long; + + ::new (reinterpret_cast(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; +}