[clang] 5e7beb0 - [analyzer] Add PlacementNewChecker
Gabor Marton via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 10 08:59:51 PST 2020
Author: Gabor Marton
Date: 2020-01-10T17:59:06+01:00
New Revision: 5e7beb0a4146267f1d65c57543e67ca158aca4aa
URL: https://github.com/llvm/llvm-project/commit/5e7beb0a4146267f1d65c57543e67ca158aca4aa
DIFF: https://github.com/llvm/llvm-project/commit/5e7beb0a4146267f1d65c57543e67ca158aca4aa.diff
LOG: [analyzer] Add PlacementNewChecker
Summary:
This checker verifies if default placement new is provided with pointers
to sufficient storage capacity.
Noncompliant Code Example:
#include <new>
void f() {
short s;
long *lp = ::new (&s) long;
}
Based on SEI CERT rule MEM54-CPP
https://wiki.sei.cmu.edu/confluence/display/cplusplus/MEM54-CPP.+Provide+placement+new+with+properly+aligned+pointe
This patch does not implement checking of the alignment.
Reviewers: NoQ, xazax.hun
Subscribers: mgorny, whisperity, xazax.hun, baloghadamsoftware, szepet,
rnkovacs, a.sidorin, mikhail.ramalho, donat
Tags: #clang
Differential Revision: https://reviews.llvm.org/D71612
Added:
clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
clang/test/Analysis/placement-new-user-defined.cpp
clang/test/Analysis/placement-new.cpp
Modified:
clang/docs/analyzer/checkers.rst
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
Removed:
################################################################################
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 34d29ba344c9..dd2365e873fd 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -292,6 +292,20 @@ Check for memory leaks. Traces memory managed by new/delete.
int *p = new int;
} // warn
+.. _cplusplus-PlacementNewChecker:
+
+cplusplus.PlacementNewChecker (C++)
+"""""""""""""""""""""""""""""""""""
+Check if default placement new is provided with pointers to sufficient storage capacity.
+
+.. code-block:: cpp
+
+ #include <new>
+
+ void f() {
+ short s;
+ long *lp = ::new (&s) long; // warn
+ }
.. _cplusplus-SelfAssignment:
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 6d68d8215597..fc1529f2ea1c 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -470,6 +470,12 @@ def NewDeleteLeaksChecker : Checker<"NewDeleteLeaks">,
Dependencies<[NewDeleteChecker]>,
Documentation<HasDocumentation>;
+def PlacementNewChecker : Checker<"PlacementNew">,
+ HelpText<"Check if default placement new is provided with pointers to "
+ "sufficient storage capacity">,
+ Dependencies<[NewDeleteChecker]>,
+ Documentation<HasDocumentation>;
+
def CXXSelfAssignmentChecker : Checker<"SelfAssignment">,
HelpText<"Checks C++ copy and move assignment operators for self assignment">,
Documentation<NotDocumented>,
diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
index f9c4757c3ca8..936a3eacc1eb 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -19,6 +19,7 @@ add_clang_library(clangStaticAnalyzerCheckers
CastValueChecker.cpp
CheckObjCDealloc.cpp
CheckObjCInstMethSignature.cpp
+ CheckPlacementNew.cpp
CheckSecuritySyntaxOnly.cpp
CheckSizeofPointer.cpp
CheckerDocumentation.cpp
diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
new file mode 100644
index 000000000000..6fe3cb63cde9
--- /dev/null
+++ b/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
@@ -0,0 +1,119 @@
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "llvm/Support/FormatVariadic.h"
+
+using namespace clang;
+using namespace ento;
+
+class PlacementNewChecker : public Checker<check::PreStmt<CXXNewExpr>> {
+public:
+ void checkPreStmt(const CXXNewExpr *NE, CheckerContext &C) const;
+
+private:
+ // Returns the size of the target in a placement new expression.
+ // E.g. in "new (&s) long" it returns the size of `long`.
+ SVal getExtentSizeOfNewTarget(const CXXNewExpr *NE, ProgramStateRef State,
+ CheckerContext &C) const;
+ // Returns the size of the place in a placement new expression.
+ // E.g. in "new (&s) long" it returns the size of `s`.
+ SVal getExtentSizeOfPlace(const Expr *NE, ProgramStateRef State,
+ CheckerContext &C) const;
+ BugType BT{this, "Insufficient storage for placement new",
+ categories::MemoryError};
+};
+
+SVal PlacementNewChecker::getExtentSizeOfPlace(const Expr *Place,
+ ProgramStateRef State,
+ CheckerContext &C) const {
+ const MemRegion *MRegion = C.getSVal(Place).getAsRegion();
+ if (!MRegion)
+ return UnknownVal();
+ RegionOffset Offset = MRegion->getAsOffset();
+ if (Offset.hasSymbolicOffset())
+ return UnknownVal();
+ const MemRegion *BaseRegion = MRegion->getBaseRegion();
+ if (!BaseRegion)
+ return UnknownVal();
+
+ SValBuilder &SvalBuilder = C.getSValBuilder();
+ NonLoc OffsetInBytes = SvalBuilder.makeArrayIndex(
+ Offset.getOffset() / C.getASTContext().getCharWidth());
+ DefinedOrUnknownSVal ExtentInBytes =
+ BaseRegion->castAs<SubRegion>()->getExtent(SvalBuilder);
+
+ return SvalBuilder.evalBinOp(State, BinaryOperator::Opcode::BO_Sub,
+ ExtentInBytes, OffsetInBytes,
+ SvalBuilder.getArrayIndexType());
+}
+
+SVal PlacementNewChecker::getExtentSizeOfNewTarget(const CXXNewExpr *NE,
+ ProgramStateRef State,
+ CheckerContext &C) const {
+ SValBuilder &SvalBuilder = C.getSValBuilder();
+ QualType ElementType = NE->getAllocatedType();
+ ASTContext &AstContext = C.getASTContext();
+ CharUnits TypeSize = AstContext.getTypeSizeInChars(ElementType);
+ if (NE->isArray()) {
+ const Expr *SizeExpr = *NE->getArraySize();
+ SVal ElementCount = C.getSVal(SizeExpr);
+ if (auto ElementCountNL = ElementCount.getAs<NonLoc>()) {
+ // size in Bytes = ElementCountNL * TypeSize
+ return SvalBuilder.evalBinOp(
+ State, BO_Mul, *ElementCountNL,
+ SvalBuilder.makeArrayIndex(TypeSize.getQuantity()),
+ SvalBuilder.getArrayIndexType());
+ }
+ } else {
+ // Create a concrete int whose size in bits and signedness is equal to
+ // ArrayIndexType.
+ llvm::APInt I(AstContext.getTypeSizeInChars(SvalBuilder.getArrayIndexType())
+ .getQuantity() *
+ C.getASTContext().getCharWidth(),
+ TypeSize.getQuantity());
+ return SvalBuilder.makeArrayIndex(I.getZExtValue());
+ }
+ return UnknownVal();
+}
+
+void PlacementNewChecker::checkPreStmt(const CXXNewExpr *NE,
+ CheckerContext &C) const {
+ // Check only the default placement new.
+ if (!NE->getOperatorNew()->isReservedGlobalPlacementOperator())
+ return;
+ if (NE->getNumPlacementArgs() == 0)
+ return;
+
+ ProgramStateRef State = C.getState();
+ SVal SizeOfTarget = getExtentSizeOfNewTarget(NE, State, C);
+ const Expr *Place = NE->getPlacementArg(0);
+ SVal SizeOfPlace = getExtentSizeOfPlace(Place, State, C);
+ const auto SizeOfTargetCI = SizeOfTarget.getAs<nonloc::ConcreteInt>();
+ if (!SizeOfTargetCI)
+ return;
+ const auto SizeOfPlaceCI = SizeOfPlace.getAs<nonloc::ConcreteInt>();
+ if (!SizeOfPlaceCI)
+ return;
+
+ if (SizeOfPlaceCI->getValue() < SizeOfTargetCI->getValue()) {
+ if (ExplodedNode *N = C.generateErrorNode(State)) {
+ std::string Msg =
+ llvm::formatv("Storage provided to placement new is only {0} bytes, "
+ "whereas the allocated type requires {1} bytes",
+ SizeOfPlaceCI->getValue(), SizeOfTargetCI->getValue());
+
+ auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N);
+ bugreporter::trackExpressionValue(N, Place, *R);
+ C.emitReport(std::move(R));
+ return;
+ }
+ }
+}
+
+void ento::registerPlacementNewChecker(CheckerManager &mgr) {
+ mgr.registerChecker<PlacementNewChecker>();
+}
+
+bool ento::shouldRegisterPlacementNewChecker(const LangOptions &LO) {
+ return true;
+}
diff --git a/clang/test/Analysis/placement-new-user-defined.cpp b/clang/test/Analysis/placement-new-user-defined.cpp
new file mode 100644
index 000000000000..b3fe47057f8a
--- /dev/null
+++ b/clang/test/Analysis/placement-new-user-defined.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_analyze_cc1 -std=c++11 %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=cplusplus.NewDelete \
+// RUN: -analyzer-checker=cplusplus.PlacementNew \
+// RUN: -analyzer-output=text -verify \
+// RUN: -triple x86_64-unknown-linux-gnu
+
+// expected-no-diagnostics
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+struct X {
+ static void *operator new(std::size_t sz, void *b) {
+ return ::operator new(sz, b);
+ }
+ long l;
+};
+void f() {
+ short buf;
+ X *p1 = new (&buf) X;
+ (void)p1;
+}
diff --git a/clang/test/Analysis/placement-new.cpp b/clang/test/Analysis/placement-new.cpp
new file mode 100644
index 000000000000..dfd057b2fa20
--- /dev/null
+++ b/clang/test/Analysis/placement-new.cpp
@@ -0,0 +1,141 @@
+// RUN: %clang_analyze_cc1 -std=c++11 %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=cplusplus.NewDelete \
+// RUN: -analyzer-checker=cplusplus.PlacementNew \
+// RUN: -analyzer-output=text -verify \
+// RUN: -triple x86_64-unknown-linux-gnu
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+void f() {
+ short s; // expected-note {{'s' declared without an initial value}}
+ long *lp = ::new (&s) long; // expected-warning{{Storage provided to placement new is only 2 bytes, whereas the allocated type requires 8 bytes}} expected-note 3 {{}}
+ (void)lp;
+}
+
+namespace testArrayNew {
+void f() {
+ short s; // expected-note {{'s' declared without an initial value}}
+ char *buf = ::new (&s) char[8]; // expected-warning{{Storage provided to placement new is only 2 bytes, whereas the allocated type requires 8 bytes}} expected-note 3 {{}}
+ (void)buf;
+}
+} // namespace testArrayNew
+
+namespace testBufferInOtherFun {
+void f(void *place) {
+ long *lp = ::new (place) long; // expected-warning{{Storage provided to placement new is only 2 bytes, whereas the allocated type requires 8 bytes}} expected-note 1 {{}}
+ (void)lp;
+}
+void g() {
+ short buf; // expected-note {{'buf' declared without an initial value}}
+ f(&buf); // expected-note 2 {{}}
+}
+} // namespace testBufferInOtherFun
+
+namespace testArrayBuffer {
+void f(void *place) {
+ long *lp = ::new (place) long; // expected-warning{{Storage provided to placement new is only 2 bytes, whereas the allocated type requires 8 bytes}} expected-note 1 {{}}
+ (void)lp;
+}
+void g() {
+ char buf[2]; // expected-note {{'buf' initialized here}}
+ f(&buf); // expected-note 2 {{}}
+}
+} // namespace testArrayBuffer
+
+namespace testGlobalPtrAsPlace {
+void *gptr = nullptr;
+short gs;
+void f() {
+ gptr = &gs; // expected-note {{Value assigned to 'gptr'}}
+}
+void g() {
+ f(); // expected-note 2 {{}}
+ long *lp = ::new (gptr) long; // expected-warning{{Storage provided to placement new is only 2 bytes, whereas the allocated type requires 8 bytes}} expected-note 1 {{}}
+ (void)lp;
+}
+} // namespace testGlobalPtrAsPlace
+
+namespace testRvalue {
+short gs;
+void *f() {
+ return &gs;
+}
+void g() {
+ long *lp = ::new (f()) long; // expected-warning{{Storage provided to placement new is only 2 bytes, whereas the allocated type requires 8 bytes}} expected-note 1 {{}}
+ (void)lp;
+}
+} // namespace testRvalue
+
+namespace testNoWarning {
+void *f();
+void g() {
+ long *lp = ::new (f()) long;
+ (void)lp;
+}
+} // namespace testNoWarning
+
+namespace testPtrToArrayAsPlace {
+void f() {
+ //char *st = new char [8];
+ char buf[3]; // expected-note {{'buf' initialized here}}
+ void *st = buf; // expected-note {{'st' initialized here}}
+ long *lp = ::new (st) long; // expected-warning{{Storage provided to placement new is only 3 bytes, whereas the allocated type requires 8 bytes}} expected-note 1 {{}}
+ (void)lp;
+}
+} // namespace testPtrToArrayAsPlace
+
+namespace testPtrToArrayWithOffsetAsPlace {
+void f() {
+ int buf[3]; // expected-note {{'buf' initialized here}}
+ long *lp = ::new (buf + 2) long; // expected-warning{{Storage provided to placement new is only 4 bytes, whereas the allocated type requires 8 bytes}} expected-note 1 {{}}
+ (void)lp;
+}
+} // namespace testPtrToArrayWithOffsetAsPlace
+
+namespace testHeapAllocatedBuffer {
+void g2() {
+ char *buf = new char[2]; // expected-note {{'buf' initialized here}}
+ long *lp = ::new (buf) long; // expected-warning{{Storage provided to placement new is only 2 bytes, whereas the allocated type requires 8 bytes}} expected-note 1 {{}}
+ (void)lp;
+}
+} // namespace testHeapAllocatedBuffer
+
+namespace testMultiDimensionalArray {
+void f() {
+ char buf[2][3]; // expected-note {{'buf' initialized here}}
+ long *lp = ::new (buf) long; // expected-warning{{Storage provided to placement new is only 6 bytes, whereas the allocated type requires 8 bytes}} expected-note 1 {{}}
+ (void)lp;
+}
+} // namespace testMultiDimensionalArray
+
+namespace testMultiDimensionalArray2 {
+void f() {
+ char buf[2][3]; // expected-note {{'buf' initialized here}}
+ long *lp = ::new (buf + 1) long; // expected-warning{{Storage provided to placement new is only 3 bytes, whereas the allocated type requires 8 bytes}} expected-note 1 {{}}
+ (void)lp;
+}
+} // namespace testMultiDimensionalArray2
+
+namespace testMultiDimensionalArray3 {
+void f() {
+ char buf[2][3]; // expected-note {{'buf' initialized here}}
+ long *lp = ::new (&buf[1][1]) long; // expected-warning{{Storage provided to placement new is only 2 bytes, whereas the allocated type requires 8 bytes}} expected-note 1 {{}}
+ (void)lp;
+}
+} // namespace testMultiDimensionalArray3
+
+namespace testHierarchy {
+struct Base {
+ char a[2];
+};
+struct Derived : Base {
+ char x[2];
+ int y;
+};
+void f() {
+ Base b; // expected-note {{'b' initialized here}}
+ Derived *dp = ::new (&b) Derived; // expected-warning{{Storage provided to placement new is only 2 bytes, whereas the allocated type requires 8 bytes}} expected-note 1 {{}}
+ (void)dp;
+}
+} // namespace testHierarchy
More information about the cfe-commits
mailing list