Author: Gabor Marton Date: 2020-05-14T15:50:39+02:00 New Revision: 7c3768495e8c1599dc30986f7bd47d5e91f303f2
URL: https://github.com/llvm/llvm-project/commit/7c3768495e8c1599dc30986f7bd47d5e91f303f2 DIFF: https://github.com/llvm/llvm-project/commit/7c3768495e8c1599dc30986f7bd47d5e91f303f2.diff LOG: [analyzer] Improve PlacementNewChecker Summary: 1. Added insufficient storage check for arrays 2. Added align support check Based on https://reviews.llvm.org/D76229 Reviewers: aaron.ballman, lebedev.ri, NoQ, martong Reviewed By: martong Subscribers: xazax.hun, baloghadamsoftware, szepet, rnkovacs, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, Charusso, ASDenysPetrov, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D76996 Patch by Karasev Nikita! Added: Modified: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp clang/test/Analysis/placement-new.cpp Removed: ################################################################################ diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp index 43ed0ffb238d..fec9fb59b2eb 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp @@ -25,22 +25,47 @@ class PlacementNewChecker : public Checker<check::PreStmt<CXXNewExpr>> { 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; + + void emitBadAlignReport(const Expr *P, CheckerContext &C, + unsigned AllocatedTAlign, + unsigned StorageTAlign) const; + unsigned getStorageAlign(CheckerContext &C, const ValueDecl *VD) const; + + void checkElementRegionAlign(const ElementRegion *R, CheckerContext &C, + const Expr *P, unsigned AllocatedTAlign) const; + + void checkFieldRegionAlign(const FieldRegion *R, CheckerContext &C, + const Expr *P, unsigned AllocatedTAlign) const; + + bool isVarRegionAlignedProperly(const VarRegion *R, CheckerContext &C, + const Expr *P, + unsigned AllocatedTAlign) 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 +88,16 @@ SVal PlacementNewChecker::getExtentSizeOfPlace(const Expr *Place, } 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 +119,212 @@ SVal PlacementNewChecker::getExtentSizeOfNewTarget(const CXXNewExpr *NE, 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()) { - 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())); + 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>(BT, Msg, N); - bugreporter::trackExpressionValue(N, Place, *R); + 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; +} + +void PlacementNewChecker::emitBadAlignReport(const Expr *P, CheckerContext &C, + unsigned AllocatedTAlign, + unsigned StorageTAlign) const { + ProgramStateRef State = C.getState(); + 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, P, *R); + C.emitReport(std::move(R)); + } +} + +unsigned PlacementNewChecker::getStorageAlign(CheckerContext &C, + const ValueDecl *VD) const { + unsigned StorageTAlign = C.getASTContext().getTypeAlign(VD->getType()); + if (unsigned SpecifiedAlignment = VD->getMaxAlignment()) + StorageTAlign = SpecifiedAlignment; + + return StorageTAlign / C.getASTContext().getCharWidth(); +} + +void PlacementNewChecker::checkElementRegionAlign( + const ElementRegion *R, CheckerContext &C, const Expr *P, + unsigned AllocatedTAlign) const { + auto IsBaseRegionAlignedProperly = [this, R, &C, P, + AllocatedTAlign]() -> bool { + // Unwind nested ElementRegion`s to get the type. + const MemRegion *SuperRegion = R; + while (true) { + if (SuperRegion->getKind() == MemRegion::ElementRegionKind) { + SuperRegion = cast<SubRegion>(SuperRegion)->getSuperRegion(); + continue; + } + + break; + } + + const DeclRegion *TheElementDeclRegion = SuperRegion->getAs<DeclRegion>(); + if (!TheElementDeclRegion) + return false; + + const DeclRegion *BaseDeclRegion = R->getBaseRegion()->getAs<DeclRegion>(); + if (!BaseDeclRegion) + return false; + + unsigned BaseRegionAlign = 0; + // We must use alignment TheElementDeclRegion if it has its own alignment + // specifier + if (TheElementDeclRegion->getDecl()->getMaxAlignment()) + BaseRegionAlign = getStorageAlign(C, TheElementDeclRegion->getDecl()); + else + BaseRegionAlign = getStorageAlign(C, BaseDeclRegion->getDecl()); + + if (AllocatedTAlign > BaseRegionAlign) { + emitBadAlignReport(P, C, AllocatedTAlign, BaseRegionAlign); + return false; + } + + return true; + }; + + auto CheckElementRegionOffset = [this, R, &C, P, AllocatedTAlign]() -> void { + RegionOffset TheOffsetRegion = R->getAsOffset(); + if (TheOffsetRegion.hasSymbolicOffset()) + return; + + unsigned Offset = + TheOffsetRegion.getOffset() / C.getASTContext().getCharWidth(); + unsigned AddressAlign = Offset % AllocatedTAlign; + if (AddressAlign != 0) { + emitBadAlignReport(P, C, AllocatedTAlign, AddressAlign); return; } + }; + + if (IsBaseRegionAlignedProperly()) { + CheckElementRegionOffset(); + } +} + +void PlacementNewChecker::checkFieldRegionAlign( + const FieldRegion *R, CheckerContext &C, const Expr *P, + unsigned AllocatedTAlign) const { + const MemRegion *BaseRegion = R->getBaseRegion(); + if (!BaseRegion) + return; + + if (const VarRegion *TheVarRegion = BaseRegion->getAs<VarRegion>()) { + if (isVarRegionAlignedProperly(TheVarRegion, C, P, AllocatedTAlign)) { + // We've checked type align but, unless FieldRegion + // offset is zero, we also need to check its own + // align. + RegionOffset Offset = R->getAsOffset(); + if (Offset.hasSymbolicOffset()) + return; + + int64_t OffsetValue = + Offset.getOffset() / C.getASTContext().getCharWidth(); + unsigned AddressAlign = OffsetValue % AllocatedTAlign; + if (AddressAlign != 0) + emitBadAlignReport(P, C, AllocatedTAlign, AddressAlign); + } + } +} + +bool PlacementNewChecker::isVarRegionAlignedProperly( + const VarRegion *R, CheckerContext &C, const Expr *P, + unsigned AllocatedTAlign) const { + const VarDecl *TheVarDecl = R->getDecl(); + unsigned StorageTAlign = getStorageAlign(C, TheVarDecl); + if (AllocatedTAlign > StorageTAlign) { + emitBadAlignReport(P, C, AllocatedTAlign, StorageTAlign); + + return false; + } + + 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(); + + SVal PlaceVal = C.getSVal(Place); + if (const MemRegion *MRegion = PlaceVal.getAsRegion()) { + if (const ElementRegion *TheElementRegion = MRegion->getAs<ElementRegion>()) + checkElementRegionAlign(TheElementRegion, C, Place, AllocatedTAlign); + else if (const FieldRegion *TheFieldRegion = MRegion->getAs<FieldRegion>()) + checkFieldRegionAlign(TheFieldRegion, C, Place, AllocatedTAlign); + else if (const VarRegion *TheVarRegion = MRegion->getAs<VarRegion>()) + isVarRegionAlignedProperly(TheVarRegion, C, Place, AllocatedTAlign); } + + 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) { diff --git a/clang/test/Analysis/placement-new.cpp b/clang/test/Analysis/placement-new.cpp index 37102b810d98..f3ecd7ebf2a7 100644 --- a/clang/test/Analysis/placement-new.cpp +++ b/clang/test/Analysis/placement-new.cpp @@ -155,3 +155,309 @@ void f() { (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() { + alignas(alignof(long)) short b[10]; + + // ok. aligned to long(ok). offset 4*2(ok) + ::new (&b[4]) long; +} + +void f8() { + alignas(alignof(long)) short b[10]; // expected-note {{'b' initialized here}} + + // ok. aligned to long(ok). offset 3*2(ok) + ::new (&b[3]) 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(long)) char b[20]; + } Xi; // expected-note {{'Xi' initialized here}} + + // ok. aligned to long(ok). offset 8*1(ok) + ::new (&Xi.b[8]) long; + + // bad. aligned to long(ok). offset 1*1(ok) + ::new (&Xi.b[1]) long; // expected-warning{{Storage type is aligned to 1 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. aligned to long(ok). offset 1(bad) + ::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. aligned to long(ok). offset 1+7*1(ok) + ::new (&Xi.b[0].a[7]) long; +} + +void test18() { + struct Y { + char p; + alignas(alignof(long)) char a[10]; + }; + + struct X { + Y b[10]; + } Xi; // expected-note {{'Xi' initialized here}} + + // ok. aligned to long(ok). offset 8*1(ok) + ::new (&Xi.b[0].a[8]) long; + + // bad. aligned to long(ok). offset 1(bad) + ::new (&Xi.b[0].a[1]) long; // expected-warning{{Storage type is aligned to 1 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 alignas(alignof(long)) X { + Y a[10]; + } Xi; // expected-note {{'Xi' initialized here}} + + // ok. aligned to long(ok). offset 1+7*1(ok) + ::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(long)) Y { + char p; + char a[10][10]; + }; + + struct X { + Y b[10]; + } Xi; // expected-note {{'Xi' initialized here}} + + // ok. aligned to long(ok). offset ok. 1(field 'a' offset) + 0*10(index '0' * first dimension size '10') + 7*1(index '7') + ::new (&Xi.b[0].a[0][7]) long; + + // ok. aligned to long(ok). offset ok. 1(field 'a' offset) + 1*10(index '1' * first dimension size '10') + 5*1(index '5') + ::new (&Xi.b[0].a[1][5]) long; + + // bad. aligned to long(ok). offset ok. 1(field 'a' offset) + 1*10(index '1' * first dimension size '10') + 6*1(index '5') + ::new (&Xi.b[0].a[1][6]) long; // expected-warning{{Storage type is aligned to 1 bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}} +} + +} // namespace testStructAlign _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
