diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp --- a/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp +++ b/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()) { @@ -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(); + if (!SizeOfTargetCI) + return true; + const auto SizeOfPlaceCI = SizeOfPlace.getAs(); + 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(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()) + MRegion = TheFieldRegion->getBaseRegion(); + + if (!MRegion) + return true; + + if (const VarRegion *TheVarRegion = MRegion->getAs()) { + 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(ABT, Msg, N); + bugreporter::trackExpressionValue(N, Place, *R); + C.emitReport(std::move(R)); + + return false; + } + } + } + } else if (Optional TheConcreteInt = + PlaceVal.getAs()) { + 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(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(); - if (!SizeOfTargetCI) + if (!checkPlaceCapacityIsSufficient(NE, C)) return; - const auto SizeOfPlaceCI = SizeOfPlace.getAs(); - 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(BT, Msg, N); - bugreporter::trackExpressionValue(N, Place, *R); - C.emitReport(std::move(R)); - 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 --- a/clang/test/Analysis/placement-new.cpp +++ b/clang/test/Analysis/placement-new.cpp @@ -155,3 +155,127 @@ (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(200)) long; +} + +void f4() +{ + ::new (reinterpret_cast(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 +