f00kat updated this revision to Diff 253408.
f00kat added a comment.
lint fixes
Repository:
rG LLVM Github Monorepo
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,109 @@
(void)dp;
}
} // namespace testHierarchy
+
+namespace testConcretePointers {
+void f1() {
+ // ok (200 % 8 == 0).
+ ::new ((size_t*)200) long;
+}
+void f2() {
+ // bad (100 % 8 == 4).
+ ::new ((size_t*)100) long; // expected-warning{{Address is aligned to 4 bytes instead of 8 bytes}} expected-note 1 {{}}
+}
+
+void f3() {
+ ::new (reinterpret_cast<size_t*>(200)) long;
+}
+
+void f4() {
+ ::new (reinterpret_cast<size_t*>(100)) long; // expected-warning{{Address is aligned to 4 bytes instead of 8 bytes}} expected-note 1 {{}}
+}
+
+void f5() {
+ ::new ((size_t*)(200 + 0)) long;
+}
+
+void f6() {
+ ::new ((size_t*)(100 + 2)) long; // expected-warning{{Address is aligned to 6 bytes instead of 8 bytes}} expected-note 1 {{}}
+}
+} // namespace testConcretePointers
+
+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{{Possibly not enough 68 bytes 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 A {
+ char a[9];
+ } Ai; // expected-note {{'Ai' initialized here}}
+
+ // bad (struct A is aligned to char).
+ ::new (&Ai.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 B {
+ char a;
+ long b;
+ } Bi;
+
+ // ok (struct B is aligned to long).
+ ::new (&Bi.a) long;
+}
+
+void f3() {
+ struct C {
+ char a[8];
+ alignas(2) char b;
+ } Ci; // expected-note {{'Ci' initialized here}}
+
+ // bad (struct C is aligned to 2).
+ ::new (&Ci.a) long; // expected-warning{{Storage type is aligned to 2 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
+}
+
+void f4() {
+ struct D {
+ char a;
+ char b;
+ struct {
+ long c;
+ };
+ } Di;
+
+ // ok (struct D is aligned to long).
+ ::new (&Di.a) long;
+}
+
+void f5() {
+ struct alignas(alignof(long)) E {
+ char a;
+ char b;
+ } Ei;
+
+ // ok (struct E is aligned to long).
+ ::new (&Ei.a) long;
+}
+} // 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,14 @@
}
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);
- if (NE->isArray()) {
+ if (IsArray = NE->isArray()) {
const Expr *SizeExpr = *NE->getArraySize();
SVal ElementCount = C.getSVal(SizeExpr);
if (auto ElementCountNL = ElementCount.getAs<NonLoc>()) {
@@ -91,38 +101,133 @@
return UnknownVal();
}
+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 true;
+ const auto SizeOfPlaceCI = SizeOfPlace.getAs<nonloc::ConcreteInt>();
+ if (!SizeOfPlaceCI)
+ 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(
+ "Possibly not enough {0} bytes 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;
+ }
+ }
+
+ return true;
+}
+
+bool PlacementNewChecker::checkPlaceIsAlignedProperly(const CXXNewExpr *NE,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ const Expr *Place = NE->getPlacementArg(0);
+ auto PlaceVal = C.getSVal(Place);
+
+ QualType AllocatedT = NE->getAllocatedType();
+ unsigned AllocatedTAlign = C.getASTContext().getTypeAlign(AllocatedT) /
+ C.getASTContext().getCharWidth();
+
+ if (const MemRegion *MRegion = PlaceVal.getAsRegion()) {
+ if (const FieldRegion *TheFieldRegion = MRegion->getAs<FieldRegion>())
+ MRegion = TheFieldRegion->getBaseRegion();
+
+ if (!MRegion)
+ return true;
+
+ if (const VarRegion *TheVarRegion = MRegion->getAs<VarRegion>()) {
+ const VarDecl *TheVarDecl = TheVarRegion->getDecl();
+ QualType PlaceType = TheVarDecl->getType();
+
+ unsigned StorageTAlign = C.getASTContext().getTypeAlign(PlaceType);
+ if (unsigned SpecifiedAlignment = TheVarDecl->getMaxAlignment())
+ StorageTAlign = SpecifiedAlignment;
+
+ StorageTAlign = StorageTAlign / C.getASTContext().getCharWidth();
+
+ if (AllocatedTAlign > StorageTAlign) {
+ if (ExplodedNode *N = C.generateErrorNode(State)) {
+ 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>(ABT, Msg, N);
+ bugreporter::trackExpressionValue(N, Place, *R);
+ C.emitReport(std::move(R));
+
+ return false;
+ }
+ }
+ }
+ } else if (Optional<loc::ConcreteInt> TheConcreteInt =
+ PlaceVal.getAs<loc::ConcreteInt>()) {
+ uint64_t PlaceAlign = *TheConcreteInt.getValue().getValue().getRawData();
+ unsigned AddressAlign = PlaceAlign % AllocatedTAlign;
+ if (AddressAlign != 0) {
+ if (ExplodedNode *N = C.generateErrorNode(State)) {
+ std::string Msg(llvm::formatv(
+ "Address is aligned to {0} bytes instead of {1} bytes",
+ AddressAlign, AllocatedTAlign));
+
+ auto R = std::make_unique<PathSensitiveBugReport>(ABT, Msg, N);
+ bugreporter::trackExpressionValue(N, Place, *R);
+ C.emitReport(std::move(R));
+
+ 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;
- 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)
+ if (!checkPlaceCapacityIsSufficient(NE, C))
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 = 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>(BT, Msg, N);
- bugreporter::trackExpressionValue(N, Place, *R);
- C.emitReport(std::move(R));
- return;
- }
- }
+ checkPlaceIsAlignedProperly(NE, C);
}
void ento::registerPlacementNewChecker(CheckerManager &mgr) {
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
