martong updated this revision to Diff 234857.
martong marked 8 inline comments as done.
martong added a comment.
- Bugtype by value
- Decompose Place to base region and offset
- Add test for type hierarchy
- Remove State check
- Return directly with the size of the type in case of non-array new
- Use Optional for ElementCount
- Update warning message
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71612/new/
https://reviews.llvm.org/D71612
Files:
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
clang/test/Analysis/placement-new-user-defined.cpp
clang/test/Analysis/placement-new.cpp
Index: clang/test/Analysis/placement-new.cpp
===================================================================
--- /dev/null
+++ 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
Index: clang/test/Analysis/placement-new-user-defined.cpp
===================================================================
--- /dev/null
+++ 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;
+}
Index: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
===================================================================
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
@@ -0,0 +1,114 @@
+#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;
+ PlacementNewChecker() : BT_Placement(this, "Insufficient storage BB") {}
+
+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(CheckerContext &C, const CXXNewExpr *NE,
+ ProgramStateRef State) 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(CheckerContext &C, const Expr *NE,
+ ProgramStateRef State) const;
+ BuiltinBug BT_Placement;
+};
+
+SVal PlacementNewChecker::getExtentSizeOfPlace(CheckerContext &C,
+ const Expr *Place,
+ ProgramStateRef State) const {
+ const MemRegion *MRegion = C.getSVal(Place).getAsRegion();
+ RegionOffset Offset = MRegion->getAsOffset();
+ const MemRegion *BaseRegion = MRegion->getBaseRegion();
+ assert(BaseRegion == Offset.getRegion());
+
+ 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(
+ CheckerContext &C, const CXXNewExpr *NE, ProgramStateRef State) 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);
+ Optional<NonLoc> ElementCountNL = ElementCount.getAs<NonLoc>();
+ if (ElementCountNL) {
+ // 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.makeIntVal(I, false);
+ }
+ return SVal();
+}
+
+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(C, NE, State);
+ const Expr *Place = NE->getPlacementArg(0);
+ SVal SizeOfPlace = getExtentSizeOfPlace(C, Place, State);
+ 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_Placement, 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;
+}
Index: clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -19,6 +19,7 @@
CastValueChecker.cpp
CheckObjCDealloc.cpp
CheckObjCInstMethSignature.cpp
+ CheckPlacementNew.cpp
CheckSecuritySyntaxOnly.cpp
CheckSizeofPointer.cpp
CheckerDocumentation.cpp
Index: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -468,6 +468,12 @@
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>,
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
