f00kat updated this revision to Diff 255039.
f00kat added a comment.
1. Maybe build fix
2. Added tests for nested arrays of structures
3. Fixed bugs in implementation for ElementRegion cases
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76996/new/
https://reviews.llvm.org/D76996
Files:
clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
clang/test/Analysis/placement-new.cpp
Index: clang/test/Analysis/placement-new.cpp
===================================================================
--- clang/test/Analysis/placement-new.cpp
+++ clang/test/Analysis/placement-new.cpp
@@ -155,3 +155,312 @@
(void)dp;
}
} // namespace testHierarchy
+
+namespace testArrayTypesAllocation {
+void f1() {
+ struct S {
+ short a;
+ };
+
+ // bad (not enough space).
+ const unsigned N = 32;
+ alignas(S) unsigned char buffer1[sizeof(S) * N]; // expected-note {{'buffer1' initialized here}}
+ ::new (buffer1) S[N]; // expected-warning{{Storage provided to placement new is only 64 bytes, whereas the allocated array type requires more space for internal needs}} expected-note 1 {{}}
+}
+
+void f2() {
+ struct S {
+ short a;
+ };
+
+ // maybe ok but we need to warn.
+ const unsigned N = 32;
+ alignas(S) unsigned char buffer2[sizeof(S) * N + sizeof(int)]; // expected-note {{'buffer2' initialized here}}
+ ::new (buffer2) S[N]; // expected-warning{{68 bytes is possibly not enough for array allocation which requires 64 bytes. Current overhead requires the size of 4 bytes}} expected-note 1 {{}}
+}
+} // namespace testArrayTypesAllocation
+
+namespace testStructAlign {
+void f1() {
+ struct X {
+ char a[9];
+ } Xi; // expected-note {{'Xi' initialized here}}
+
+ // bad (struct X is aligned to char).
+ ::new (&Xi.a) long; // expected-warning{{Storage type is aligned to 1 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f2() {
+ struct X {
+ char a;
+ char b;
+ long c;
+ } Xi;
+
+ // ok (struct X is aligned to long).
+ ::new (&Xi.a) long;
+}
+
+void f3() {
+ struct X {
+ char a;
+ char b;
+ long c;
+ } Xi; // expected-note {{'Xi' initialized here}}
+
+ // bad (struct X is aligned to long but field 'b' is aligned to 1 because of its offset)
+ ::new (&Xi.b) long; // expected-warning{{Storage type is aligned to 1 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f4() {
+ struct X {
+ char a;
+ struct alignas(alignof(short)) Y {
+ char b;
+ char c;
+ } y;
+ long d;
+ } Xi; // expected-note {{'Xi' initialized here}}
+
+ // bad. 'b' is aligned to short
+ ::new (&Xi.y.b) long; // expected-warning{{Storage type is aligned to 2 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f5() {
+ short b[10]; // expected-note {{'b' initialized here}}
+
+ ::new (&b) long; // expected-warning{{Storage type is aligned to 2 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f6() {
+ short b[10]; // expected-note {{'b' initialized here}}
+
+ // bad (same as previous but checks ElementRegion case)
+ ::new (&b[0]) long; // expected-warning{{Storage type is aligned to 2 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f7() {
+ short b[10];
+
+ // ok. 2(short align) + 3*2(index '3' offset)
+ ::new (&b[3]) long;
+}
+
+void f8() {
+ short b[10]; // expected-note {{'b' initialized here}}
+
+ // bad. 2(short align) + 2*2(index '2' offset)
+ ::new (&b[2]) long; // expected-warning{{Storage type is aligned to 6 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f9() {
+ struct X {
+ char a;
+ alignas(alignof(short)) char b[20];
+ } Xi; // expected-note {{'Xi' initialized here}}
+
+ // ok 2(custom align) + 6*1(index '6' offset)
+ ::new (&Xi.b[6]) long;
+
+ // bad 2(custom align) + 1*1(index '1' offset)
+ ::new (&Xi.b[1]) long; // expected-warning{{Storage type is aligned to 3 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f10() {
+ struct X {
+ char a[8];
+ alignas(2) char b;
+ } Xi; // expected-note {{'Xi' initialized here}}
+
+ // bad (struct X is aligned to 2).
+ ::new (&Xi.a) long; // expected-warning{{Storage type is aligned to 2 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f11() {
+ struct X {
+ char a;
+ char b;
+ struct Y {
+ long c;
+ } d;
+ } Xi;
+
+ // ok (struct X is aligned to long).
+ ::new (&Xi.a) long;
+}
+
+void f12() {
+ struct alignas(alignof(long)) X {
+ char a;
+ char b;
+ } Xi;
+
+ // ok (struct X is aligned to long).
+ ::new (&Xi.a) long;
+}
+
+void test13() {
+ struct Y {
+ char a[10];
+ };
+
+ struct X {
+ Y b[10];
+ } Xi; // expected-note {{'Xi' initialized here}}
+
+ // bad. X,A are aligned to 'char'
+ ::new (&Xi.b[0].a) long; // expected-warning{{Storage type is aligned to 1 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void test14() {
+ struct Y {
+ char a[10];
+ };
+
+ struct alignas(alignof(long)) X {
+ Y b[10];
+ } Xi;
+
+ // ok. X is aligned to 'long' and field 'a' goes with zero offset
+ ::new (&Xi.b[0].a) long;
+}
+
+void test15() {
+ struct alignas(alignof(long)) Y {
+ char a[10];
+ };
+
+ struct X {
+ Y b[10];
+ } Xi;
+
+ // ok. X is aligned to 'long' because it contains struct 'Y' which is aligned to 'long'
+ ::new (&Xi.b[0].a) long;
+}
+
+void test16() {
+ struct alignas(alignof(long)) Y {
+ char p;
+ char a[10];
+ };
+
+ struct X {
+ Y b[10];
+ } Xi; // expected-note {{'Xi' initialized here}}
+
+ // bad 8(custom Y align) + 1(field 'a' offset)
+ ::new (&Xi.b[0].a) long; // expected-warning{{Storage type is aligned to 1 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void test17() {
+ struct alignas(alignof(long)) Y {
+ char p;
+ char a[10];
+ };
+
+ struct X {
+ Y b[10];
+ } Xi;
+
+ // ok 8(custom Y align) + 1(field 'a' offset) + 7(index '7' offset)
+ ::new (&Xi.b[0].a[7]) long;
+}
+
+void test18() {
+ struct Y {
+ char p;
+ alignas(alignof(short)) char a[10];
+ };
+
+ struct X {
+ Y b[10];
+ } Xi; // expected-note {{'Xi' initialized here}}
+
+ // ok 2(custom Y align) + 6*1(index '6' offset)
+ // we ignore 'p' field offset because field 'a' has its own align specifier
+ ::new (&Xi.b[0].a[6]) long;
+
+ // bad 2(custom align) + 1*1(index '1' offset)
+ ::new (&Xi.b[1].a[1]) long; // expected-warning{{Storage type is aligned to 3 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void test19() {
+ struct Z {
+ char p;
+ char c[10];
+ };
+
+ struct Y {
+ char p;
+ Z b[10];
+ };
+
+ struct X {
+ Y a[10];
+ } Xi; // expected-note {{'Xi' initialized here}}
+
+ // bad. all structures X,Y,Z are aligned to char
+ ::new (&Xi.a[1].b[1].c) long; // expected-warning{{Storage type is aligned to 1 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void test20() {
+ struct Z {
+ char p;
+ alignas(alignof(long)) char c[10];
+ };
+
+ struct Y {
+ char p;
+ Z b[10];
+ };
+
+ struct X {
+ Y a[10];
+ } Xi;
+
+ // ok. field 'c' is aligned to 'long'
+ ::new (&Xi.a[1].b[1].c) long;
+}
+
+void test21() {
+ struct Z {
+ char p;
+ char c[10];
+ };
+
+ struct Y {
+ char p;
+ Z b[10];
+ };
+
+ struct X {
+ Y a[10];
+ } Xi; // expected-note {{'Xi' initialized here}}
+
+ // bad 1(field 'Y.p' offset) + 1(field 'Z.p' offset) + 7*1(index '7' offset)
+ ::new (&Xi.a[0].b[0].c[7]) long; // expected-warning{{Storage type is aligned to 1 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void test22() {
+ struct alignas(alignof(short)) Z {
+ char p;
+ char c[10];
+ };
+
+ struct Y {
+ char p;
+ Z b[10];
+ };
+
+ struct X {
+ Y a[10];
+ } Xi; // expected-note {{'Xi' initialized here}}
+
+ // ok. 2(X align) + 1 (offset Y.p) + 1(align Z.p to 'short') + 1(offset Z.p) + 3(index)
+ ::new (&Xi.a[0].b[0].c[3]) long;
+
+ // bad. 2(X align) + 1 (offset Y.p) + 1(align Z.p to 'short') + 1(offset Z.p) + 4(index)
+ ::new (&Xi.a[0].b[0].c[4]) long; // expected-warning{{Storage type is aligned to 1 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+} // namespace testStructAlign
Index: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
@@ -25,22 +25,31 @@
void checkPreStmt(const CXXNewExpr *NE, CheckerContext &C) const;
private:
+ bool checkPlaceCapacityIsSufficient(const CXXNewExpr *NE,
+ CheckerContext &C) const;
+
+ bool checkPlaceIsAlignedProperly(const CXXNewExpr *NE,
+ CheckerContext &C) const;
+
// 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;
+ SVal getExtentSizeOfNewTarget(const CXXNewExpr *NE, CheckerContext &C,
+ bool &IsArray) 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 getExtentSizeOfPlace(const CXXNewExpr *NE, CheckerContext &C) const;
+ BugType SBT{this, "Insufficient storage for placement new",
+ categories::MemoryError};
+ BugType ABT{this, "Bad align storage for placement new",
+ categories::MemoryError};
};
} // namespace
-SVal PlacementNewChecker::getExtentSizeOfPlace(const Expr *Place,
- ProgramStateRef State,
+SVal PlacementNewChecker::getExtentSizeOfPlace(const CXXNewExpr *NE,
CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ const Expr *Place = NE->getPlacementArg(0);
+
const MemRegion *MRegion = C.getSVal(Place).getAsRegion();
if (!MRegion)
return UnknownVal();
@@ -63,13 +72,16 @@
}
SVal PlacementNewChecker::getExtentSizeOfNewTarget(const CXXNewExpr *NE,
- ProgramStateRef State,
- CheckerContext &C) const {
+ CheckerContext &C,
+ bool &IsArray) const {
+ ProgramStateRef State = C.getState();
SValBuilder &SvalBuilder = C.getSValBuilder();
QualType ElementType = NE->getAllocatedType();
ASTContext &AstContext = C.getASTContext();
CharUnits TypeSize = AstContext.getTypeSizeInChars(ElementType);
+ IsArray = false;
if (NE->isArray()) {
+ IsArray = true;
const Expr *SizeExpr = *NE->getArraySize();
SVal ElementCount = C.getSVal(SizeExpr);
if (auto ElementCountNL = ElementCount.getAs<NonLoc>()) {
@@ -91,38 +103,189 @@
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);
+bool PlacementNewChecker::checkPlaceCapacityIsSufficient(
+ const CXXNewExpr *NE, CheckerContext &C) const {
+ bool IsArrayTypeAllocated;
+ SVal SizeOfTarget = getExtentSizeOfNewTarget(NE, C, IsArrayTypeAllocated);
+ SVal SizeOfPlace = getExtentSizeOfPlace(NE, C);
const auto SizeOfTargetCI = SizeOfTarget.getAs<nonloc::ConcreteInt>();
if (!SizeOfTargetCI)
- return;
+ return true;
const auto SizeOfPlaceCI = SizeOfPlace.getAs<nonloc::ConcreteInt>();
if (!SizeOfPlaceCI)
- return;
+ return true;
+
+ if ((SizeOfPlaceCI->getValue() < SizeOfTargetCI->getValue()) ||
+ (IsArrayTypeAllocated &&
+ SizeOfPlaceCI->getValue() >= SizeOfTargetCI->getValue())) {
+ if (ExplodedNode *N = C.generateErrorNode(C.getState())) {
+ std::string Msg;
+ // TODO: use clang constant
+ if (IsArrayTypeAllocated &&
+ SizeOfPlaceCI->getValue() > SizeOfTargetCI->getValue())
+ Msg = std::string(llvm::formatv(
+ "{0} bytes is possibly not enough for array allocation which "
+ "requires {1} bytes. Current overhead requires the size of {2} "
+ "bytes",
+ SizeOfPlaceCI->getValue(), SizeOfTargetCI->getValue(),
+ SizeOfPlaceCI->getValue() - SizeOfTargetCI->getValue()));
+ else if (IsArrayTypeAllocated &&
+ SizeOfPlaceCI->getValue() == SizeOfTargetCI->getValue())
+ Msg = std::string(llvm::formatv(
+ "Storage provided to placement new is only {0} bytes, "
+ "whereas the allocated array type requires more space for "
+ "internal needs",
+ SizeOfPlaceCI->getValue(), SizeOfTargetCI->getValue()));
+ else
+ Msg = std::string(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>(SBT, Msg, N);
+ bugreporter::trackExpressionValue(N, NE->getPlacementArg(0), *R);
+ C.emitReport(std::move(R));
+
+ return false;
+ }
+ }
- if (SizeOfPlaceCI->getValue() < SizeOfTargetCI->getValue()) {
+ return true;
+}
+
+bool PlacementNewChecker::checkPlaceIsAlignedProperly(const CXXNewExpr *NE,
+ CheckerContext &C) const {
+ const Expr *Place = NE->getPlacementArg(0);
+
+ QualType AllocatedT = NE->getAllocatedType();
+ unsigned AllocatedTAlign = C.getASTContext().getTypeAlign(AllocatedT) /
+ C.getASTContext().getCharWidth();
+
+ auto EmitBadAlignReport = [Place, &C, AllocatedTAlign,
+ this](unsigned StorageTAlign) -> void {
+ ProgramStateRef State = C.getState();
if (ExplodedNode *N = C.generateErrorNode(State)) {
- std::string Msg = std::string(
- llvm::formatv("Storage provided to placement new is only {0} bytes, "
- "whereas the allocated type requires {1} bytes",
- SizeOfPlaceCI->getValue(), SizeOfTargetCI->getValue()));
+ std::string Msg(llvm::formatv("Storage type is aligned to {0} bytes but "
+ "allocated type is aligned to {1} bytes",
+ StorageTAlign, AllocatedTAlign));
- auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N);
+ auto R = std::make_unique<PathSensitiveBugReport>(ABT, Msg, N);
bugreporter::trackExpressionValue(N, Place, *R);
C.emitReport(std::move(R));
- return;
+ }
+ };
+
+ auto GetStorageAlign = [&C](const ValueDecl *TheValueDecl) -> unsigned {
+ unsigned StorageTAlign =
+ C.getASTContext().getTypeAlign(TheValueDecl->getType());
+ if (unsigned SpecifiedAlignment = TheValueDecl->getMaxAlignment())
+ StorageTAlign = SpecifiedAlignment;
+
+ return StorageTAlign / C.getASTContext().getCharWidth();
+ };
+
+ SVal PlaceVal = C.getSVal(Place);
+ if (const MemRegion *MRegion = PlaceVal.getAsRegion()) {
+ if (const ElementRegion *TheElementRegion =
+ MRegion->getAs<ElementRegion>()) {
+ RegionRawOffset Offset = TheElementRegion->getAsArrayOffset();
+ if (const MemRegion *OffsetRegion = Offset.getRegion()) {
+ int64_t FieldOffsetValue = 0;
+ if (const FieldRegion *TheFieldRegion =
+ OffsetRegion->getAs<FieldRegion>()) {
+ RegionOffset Offset = TheFieldRegion->getAsOffset();
+ if (Offset.hasSymbolicOffset())
+ return true;
+
+ // We ignore field offset if the field has its own align specifier.
+ if (!TheFieldRegion->getDecl()->getMaxAlignment())
+ FieldOffsetValue =
+ Offset.getOffset() / C.getASTContext().getCharWidth();
+
+ MRegion = TheFieldRegion->getBaseRegion();
+ } else
+ MRegion = OffsetRegion;
+
+ if (const DeclRegion *TheDeclRegion = MRegion->getAs<DeclRegion>()) {
+ unsigned StorageTAlign = GetStorageAlign(TheDeclRegion->getDecl());
+ CharUnits::QuantityType ElementOffsetValue =
+ Offset.getOffset().getQuantity();
+
+ // We ignore StorageType align if StorageType is aligned to one byte.
+ if (StorageTAlign == 1)
+ StorageTAlign = 0;
+
+ long long FinalStorageTAlign =
+ StorageTAlign + FieldOffsetValue + ElementOffsetValue;
+ unsigned AddressAlign = FinalStorageTAlign % AllocatedTAlign;
+ if (AddressAlign != 0) {
+ EmitBadAlignReport(AddressAlign);
+
+ return false;
+ }
+ }
+ }
+ } else if (const FieldRegion *TheFieldRegion =
+ MRegion->getAs<FieldRegion>()) {
+ MRegion = TheFieldRegion->getBaseRegion();
+
+ if (!MRegion)
+ return false;
+
+ if (const VarRegion *TheVarRegion = MRegion->getAs<VarRegion>()) {
+ const VarDecl *TheVarDecl = TheVarRegion->getDecl();
+
+ unsigned StorageTAlign = GetStorageAlign(TheVarDecl);
+ if (AllocatedTAlign > StorageTAlign) {
+ EmitBadAlignReport(StorageTAlign);
+
+ return false;
+ }
+
+ // We've checked type align but, unless FieldRegion offset is zero, we
+ // also need to check its own align.
+ RegionOffset Offset = TheFieldRegion->getAsOffset();
+ if (Offset.hasSymbolicOffset())
+ return true;
+
+ int64_t OffsetValue =
+ Offset.getOffset() / C.getASTContext().getCharWidth();
+ if (OffsetValue > 0) {
+ unsigned AddressAlign = OffsetValue % AllocatedTAlign;
+ if (AddressAlign != 0) {
+ EmitBadAlignReport(AddressAlign);
+
+ return false;
+ }
+ }
+ }
+ } else if (const VarRegion *TheVarRegion = MRegion->getAs<VarRegion>()) {
+ const VarDecl *TheVarDecl = TheVarRegion->getDecl();
+ unsigned StorageTAlign = GetStorageAlign(TheVarDecl);
+ if (AllocatedTAlign > StorageTAlign) {
+ EmitBadAlignReport(StorageTAlign);
+
+ return false;
+ }
}
}
+
+ return true;
+}
+
+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;
+
+ if (!checkPlaceCapacityIsSufficient(NE, C))
+ return;
+
+ checkPlaceIsAlignedProperly(NE, C);
}
void ento::registerPlacementNewChecker(CheckerManager &mgr) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
