[clang] aac73a3 - [analyzer] Process non-POD array element destructors
via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 23 16:28:43 PDT 2022
Author: isuckatcs
Date: 2022-08-24T01:28:21+02:00
New Revision: aac73a31add5e80c746a7794d832f2cdf226486c
URL: https://github.com/llvm/llvm-project/commit/aac73a31add5e80c746a7794d832f2cdf226486c
DIFF: https://github.com/llvm/llvm-project/commit/aac73a31add5e80c746a7794d832f2cdf226486c.diff
LOG: [analyzer] Process non-POD array element destructors
The constructors of non-POD array elements are evaluated under
certain conditions. This patch makes sure that in such cases
we also evaluate the destructors.
Differential Revision: https://reviews.llvm.org/D130737
Added:
clang/test/Analysis/dtor-array.cpp
Modified:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
clang/lib/Analysis/CFG.cpp
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
clang/lib/StaticAnalyzer/Core/ProgramState.cpp
clang/test/Analysis/new.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index bc5a39c91c905..aafeb5e39acb4 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -617,11 +617,16 @@ class ExprEngine {
return svalBuilder.evalBinOp(ST, Op, LHS, RHS, T);
}
- /// Retreives which element is being constructed in a non POD type array.
+ /// Retreives which element is being constructed in a non-POD type array.
static Optional<unsigned>
getIndexOfElementToConstruct(ProgramStateRef State, const CXXConstructExpr *E,
const LocationContext *LCtx);
+ /// Retreives which element is being destructed in a non-POD type array.
+ static Optional<unsigned>
+ getPendingArrayDestruction(ProgramStateRef State,
+ const LocationContext *LCtx);
+
/// Retreives the size of the array in the pending ArrayInitLoopExpr.
static Optional<unsigned> getPendingInitLoop(ProgramStateRef State,
const CXXConstructExpr *E,
@@ -825,6 +830,27 @@ class ExprEngine {
const CXXConstructExpr *CE,
const LocationContext *LCtx);
+ /// Checks whether our policies allow us to inline a non-POD type array
+ /// destruction.
+ /// \param Size The size of the array.
+ bool shouldInlineArrayDestruction(uint64_t Size);
+
+ /// Prepares the program state for array destruction. If no error happens
+ /// the function binds a 'PendingArrayDestruction' entry to the state, which
+ /// it returns along with the index. If any error happens (we fail to read
+ /// the size, the index would be -1, etc.) the function will return the
+ /// original state along with an index of 0. The actual element count of the
+ /// array can be accessed by the optional 'ElementCountVal' parameter. \param
+ /// State The program state. \param Region The memory region where the array
+ /// is stored. \param ElementTy The type an element in the array. \param LCty
+ /// The location context. \param ElementCountVal A pointer to an optional
+ /// SVal. If specified, the size of the array will be returned in it. It can
+ /// be Unknown.
+ std::pair<ProgramStateRef, uint64_t> prepareStateForArrayDestruction(
+ const ProgramStateRef State, const MemRegion *Region,
+ const QualType &ElementTy, const LocationContext *LCtx,
+ SVal *ElementCountVal = nullptr);
+
/// Checks whether we construct an array of non-POD type, and decides if the
/// constructor should be inkoved once again.
bool shouldRepeatCtorCall(ProgramStateRef State, const CXXConstructExpr *E,
@@ -924,6 +950,16 @@ class ExprEngine {
const CXXConstructExpr *E,
const LocationContext *LCtx);
+ /// Assuming we destruct an array of non-POD types, this method allows us
+ /// to store which element is to be destructed next.
+ static ProgramStateRef setPendingArrayDestruction(ProgramStateRef State,
+ const LocationContext *LCtx,
+ unsigned Idx);
+
+ static ProgramStateRef
+ removePendingArrayDestruction(ProgramStateRef State,
+ const LocationContext *LCtx);
+
/// Sets the size of the array in a pending ArrayInitLoopExpr.
static ProgramStateRef setPendingInitLoop(ProgramStateRef State,
const CXXConstructExpr *E,
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 6131256d4a0d0..5aaf2fb74318b 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -1970,9 +1970,10 @@ void CFGBuilder::addImplicitDtorsForDestructor(const CXXDestructorDecl *DD) {
for (auto *FI : RD->fields()) {
// Check for constant size array. Set type to array element type.
QualType QT = FI->getType();
- if (const ConstantArrayType *AT = Context->getAsConstantArrayType(QT)) {
+ // It may be a multidimensional array.
+ while (const ConstantArrayType *AT = Context->getAsConstantArrayType(QT)) {
if (AT->getSize() == 0)
- continue;
+ break;
QT = AT->getElementType();
}
@@ -5333,8 +5334,19 @@ CFGImplicitDtor::getDestructorDecl(ASTContext &astContext) const {
const CXXTemporary *temp = bindExpr->getTemporary();
return temp->getDestructor();
}
+ case CFGElement::MemberDtor: {
+ const FieldDecl *field = castAs<CFGMemberDtor>().getFieldDecl();
+ QualType ty = field->getType();
+
+ while (const ArrayType *arrayType = astContext.getAsArrayType(ty)) {
+ ty = arrayType->getElementType();
+ }
+
+ const CXXRecordDecl *classDecl = ty->getAsCXXRecordDecl();
+ assert(classDecl);
+ return classDecl->getDestructor();
+ }
case CFGElement::BaseDtor:
- case CFGElement::MemberDtor:
// Not yet supported.
return nullptr;
}
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index f9edcb3189e21..586e89ef2a174 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -48,6 +48,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/LoopUnrolling.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h"
@@ -204,6 +205,12 @@ typedef llvm::ImmutableMap<
std::pair<const CXXConstructExpr *, const LocationContext *>, unsigned>
PendingInitLoopMap;
REGISTER_TRAIT_WITH_PROGRAMSTATE(PendingInitLoop, PendingInitLoopMap)
+
+typedef llvm::ImmutableMap<const LocationContext *, unsigned>
+ PendingArrayDestructionMap;
+REGISTER_TRAIT_WITH_PROGRAMSTATE(PendingArrayDestruction,
+ PendingArrayDestructionMap)
+
//===----------------------------------------------------------------------===//
// Engine construction and deletion.
//===----------------------------------------------------------------------===//
@@ -517,6 +524,35 @@ ExprEngine::removeIndexOfElementToConstruct(ProgramStateRef State,
return State->remove<IndexOfElementToConstruct>(Key);
}
+Optional<unsigned>
+ExprEngine::getPendingArrayDestruction(ProgramStateRef State,
+ const LocationContext *LCtx) {
+ assert(LCtx && "LocationContext shouldn't be null!");
+
+ return Optional<unsigned>::create(
+ State->get<PendingArrayDestruction>(LCtx->getStackFrame()));
+}
+
+ProgramStateRef ExprEngine::setPendingArrayDestruction(
+ ProgramStateRef State, const LocationContext *LCtx, unsigned Idx) {
+ assert(LCtx && "LocationContext shouldn't be null!");
+
+ auto Key = LCtx->getStackFrame();
+
+ return State->set<PendingArrayDestruction>(Key, Idx);
+}
+
+ProgramStateRef
+ExprEngine::removePendingArrayDestruction(ProgramStateRef State,
+ const LocationContext *LCtx) {
+ assert(LCtx && "LocationContext shouldn't be null!");
+
+ auto Key = LCtx->getStackFrame();
+
+ assert(LCtx && State->contains<PendingArrayDestruction>(Key));
+ return State->remove<PendingArrayDestruction>(Key);
+}
+
ProgramStateRef
ExprEngine::addObjectUnderConstruction(ProgramStateRef State,
const ConstructionContextItem &Item,
@@ -1072,6 +1108,42 @@ void ExprEngine::ProcessInitializer(const CFGInitializer CFGInit,
Engine.enqueue(Dst, currBldrCtx->getBlock(), currStmtIdx);
}
+std::pair<ProgramStateRef, uint64_t>
+ExprEngine::prepareStateForArrayDestruction(const ProgramStateRef State,
+ const MemRegion *Region,
+ const QualType &ElementTy,
+ const LocationContext *LCtx,
+ SVal *ElementCountVal) {
+
+ QualType Ty = ElementTy.getDesugaredType(getContext());
+ while (const auto *NTy = dyn_cast<ArrayType>(Ty))
+ Ty = NTy->getElementType().getDesugaredType(getContext());
+
+ auto ElementCount = getDynamicElementCount(State, Region, svalBuilder, Ty);
+
+ if (ElementCountVal)
+ *ElementCountVal = ElementCount;
+
+ // Note: the destructors are called in reverse order.
+ unsigned Idx = 0;
+ if (auto OptionalIdx = getPendingArrayDestruction(State, LCtx)) {
+ Idx = *OptionalIdx;
+ } else {
+ // The element count is either unknown, or an SVal that's not an integer.
+ if (!ElementCount.isConstant())
+ return {State, 0};
+
+ Idx = ElementCount.getAsInteger()->getLimitedValue();
+ }
+
+ if (Idx == 0)
+ return {State, 0};
+
+ --Idx;
+
+ return {setPendingArrayDestruction(State, LCtx, Idx), Idx};
+}
+
void ExprEngine::ProcessImplicitDtor(const CFGImplicitDtor D,
ExplodedNode *Pred) {
ExplodedNodeSet Dst;
@@ -1121,11 +1193,14 @@ void ExprEngine::ProcessNewAllocator(const CXXNewExpr *NE,
void ExprEngine::ProcessAutomaticObjDtor(const CFGAutomaticObjDtor Dtor,
ExplodedNode *Pred,
ExplodedNodeSet &Dst) {
+ const auto *DtorDecl = Dtor.getDestructorDecl(getContext());
const VarDecl *varDecl = Dtor.getVarDecl();
QualType varType = varDecl->getType();
ProgramStateRef state = Pred->getState();
- SVal dest = state->getLValue(varDecl, Pred->getLocationContext());
+ const LocationContext *LCtx = Pred->getLocationContext();
+
+ SVal dest = state->getLValue(varDecl, LCtx);
const MemRegion *Region = dest.castAs<loc::MemRegionVal>().getRegion();
if (varType->isReferenceType()) {
@@ -1141,14 +1216,46 @@ void ExprEngine::ProcessAutomaticObjDtor(const CFGAutomaticObjDtor Dtor,
varType = cast<TypedValueRegion>(Region)->getValueType();
}
- // FIXME: We need to run the same destructor on every element of the array.
- // This workaround will just run the first destructor (which will still
- // invalidate the entire array).
+ unsigned Idx = 0;
+ if (const auto *AT = dyn_cast<ArrayType>(varType)) {
+ SVal ElementCount;
+ std::tie(state, Idx) = prepareStateForArrayDestruction(
+ state, Region, varType, LCtx, &ElementCount);
+
+ if (ElementCount.isConstant()) {
+ uint64_t ArrayLength = ElementCount.getAsInteger()->getLimitedValue();
+ assert(ArrayLength &&
+ "An automatic dtor for a 0 length array shouldn't be triggered!");
+
+ // Still handle this case if we don't have assertions enabled.
+ if (!ArrayLength) {
+ static SimpleProgramPointTag PT(
+ "ExprEngine", "Skipping automatic 0 length array destruction, "
+ "which shouldn't be in the CFG.");
+ PostImplicitCall PP(DtorDecl, varDecl->getLocation(), LCtx, &PT);
+ NodeBuilder Bldr(Pred, Dst, *currBldrCtx);
+ Bldr.generateSink(PP, Pred->getState(), Pred);
+ return;
+ }
+ }
+ }
+
EvalCallOptions CallOpts;
Region = makeElementRegion(state, loc::MemRegionVal(Region), varType,
- CallOpts.IsArrayCtorOrDtor)
+ CallOpts.IsArrayCtorOrDtor, Idx)
.getAsRegion();
+ NodeBuilder Bldr(Pred, Dst, getBuilderContext());
+
+ static SimpleProgramPointTag PT("ExprEngine",
+ "Prepare for object destruction");
+ PreImplicitCall PP(DtorDecl, varDecl->getLocation(), LCtx, &PT);
+ Pred = Bldr.generateNode(PP, state, Pred);
+ Bldr.takeNodes(Pred);
+
+ if (!Pred)
+ return;
+
VisitCXXDestructor(varType, Region, Dtor.getTriggerStmt(),
/*IsBase=*/false, Pred, Dst, CallOpts);
}
@@ -1176,20 +1283,53 @@ void ExprEngine::ProcessDeleteDtor(const CFGDeleteDtor Dtor,
return;
}
+ auto getDtorDecl = [](const QualType &DTy) {
+ const CXXRecordDecl *RD = DTy->getAsCXXRecordDecl();
+ return RD->getDestructor();
+ };
+
+ unsigned Idx = 0;
EvalCallOptions CallOpts;
const MemRegion *ArgR = ArgVal.getAsRegion();
+
if (DE->isArrayForm()) {
- // FIXME: We need to run the same destructor on every element of the array.
- // This workaround will just run the first destructor (which will still
- // invalidate the entire array).
+ SVal ElementCount;
+ std::tie(State, Idx) =
+ prepareStateForArrayDestruction(State, ArgR, DTy, LCtx, &ElementCount);
+
CallOpts.IsArrayCtorOrDtor = true;
// Yes, it may even be a multi-dimensional array.
while (const auto *AT = getContext().getAsArrayType(DTy))
DTy = AT->getElementType();
+
+ // If we're about to destruct a 0 length array, don't run any of the
+ // destructors.
+ if (ElementCount.isConstant() &&
+ ElementCount.getAsInteger()->getLimitedValue() == 0) {
+
+ static SimpleProgramPointTag PT(
+ "ExprEngine", "Skipping 0 length array delete destruction");
+ PostImplicitCall PP(getDtorDecl(DTy), DE->getBeginLoc(), LCtx, &PT);
+ NodeBuilder Bldr(Pred, Dst, *currBldrCtx);
+ Bldr.generateNode(PP, Pred->getState(), Pred);
+ return;
+ }
+
if (ArgR)
- ArgR = getStoreManager().GetElementZeroRegion(cast<SubRegion>(ArgR), DTy);
+ ArgR = State->getLValue(DTy, svalBuilder.makeArrayIndex(Idx), ArgVal)
+ .getAsRegion();
}
+ NodeBuilder Bldr(Pred, Dst, getBuilderContext());
+ static SimpleProgramPointTag PT("ExprEngine",
+ "Prepare for object destruction");
+ PreImplicitCall PP(getDtorDecl(DTy), DE->getBeginLoc(), LCtx, &PT);
+ Pred = Bldr.generateNode(PP, State, Pred);
+ Bldr.takeNodes(Pred);
+
+ if (!Pred)
+ return;
+
VisitCXXDestructor(DTy, ArgR, DE, /*IsBase=*/false, Pred, Dst, CallOpts);
}
@@ -1215,6 +1355,7 @@ void ExprEngine::ProcessBaseDtor(const CFGBaseDtor D,
void ExprEngine::ProcessMemberDtor(const CFGMemberDtor D,
ExplodedNode *Pred, ExplodedNodeSet &Dst) {
+ const auto *DtorDecl = D.getDestructorDecl(getContext());
const FieldDecl *Member = D.getFieldDecl();
QualType T = Member->getType();
ProgramStateRef State = Pred->getState();
@@ -1226,11 +1367,44 @@ void ExprEngine::ProcessMemberDtor(const CFGMemberDtor D,
Loc ThisLoc = State->getSVal(ThisStorageLoc).castAs<Loc>();
SVal FieldVal = State->getLValue(Member, ThisLoc);
- // FIXME: We need to run the same destructor on every element of the array.
- // This workaround will just run the first destructor (which will still
- // invalidate the entire array).
+ unsigned Idx = 0;
+ if (const auto *AT = dyn_cast<ArrayType>(T)) {
+ SVal ElementCount;
+ std::tie(State, Idx) = prepareStateForArrayDestruction(
+ State, FieldVal.getAsRegion(), T, LCtx, &ElementCount);
+
+ if (ElementCount.isConstant()) {
+ uint64_t ArrayLength = ElementCount.getAsInteger()->getLimitedValue();
+ assert(ArrayLength &&
+ "A member dtor for a 0 length array shouldn't be triggered!");
+
+ // Still handle this case if we don't have assertions enabled.
+ if (!ArrayLength) {
+ static SimpleProgramPointTag PT(
+ "ExprEngine", "Skipping member 0 length array destruction, which "
+ "shouldn't be in the CFG.");
+ PostImplicitCall PP(DtorDecl, Member->getLocation(), LCtx, &PT);
+ NodeBuilder Bldr(Pred, Dst, *currBldrCtx);
+ Bldr.generateSink(PP, Pred->getState(), Pred);
+ return;
+ }
+ }
+ }
+
EvalCallOptions CallOpts;
- FieldVal = makeElementRegion(State, FieldVal, T, CallOpts.IsArrayCtorOrDtor);
+ FieldVal =
+ makeElementRegion(State, FieldVal, T, CallOpts.IsArrayCtorOrDtor, Idx);
+
+ NodeBuilder Bldr(Pred, Dst, getBuilderContext());
+
+ static SimpleProgramPointTag PT("ExprEngine",
+ "Prepare for object destruction");
+ PreImplicitCall PP(DtorDecl, Member->getLocation(), LCtx, &PT);
+ Pred = Bldr.generateNode(PP, State, Pred);
+ Bldr.takeNodes(Pred);
+
+ if (!Pred)
+ return;
VisitCXXDestructor(T, FieldVal.getAsRegion(), CurDtor->getBody(),
/*IsBase=*/false, Pred, Dst, CallOpts);
@@ -1281,15 +1455,31 @@ void ExprEngine::ProcessTemporaryDtor(const CFGTemporaryDtor D,
EvalCallOptions CallOpts;
CallOpts.IsTemporaryCtorOrDtor = true;
if (!MR) {
- // If we have no MR, we still need to unwrap the array to avoid destroying
- // the whole array at once. Regardless, we'd eventually need to model array
- // destructors properly, element-by-element.
+ // FIXME: If we have no MR, we still need to unwrap the array to avoid
+ // destroying the whole array at once.
+ //
+ // For this case there is no universal solution as there is no way to
+ // directly create an array of temporary objects. There are some expressions
+ // however which can create temporary objects and have an array type.
+ //
+ // E.g.: std::initializer_list<S>{S(), S()};
+ //
+ // The expression above has a type of 'const struct S[2]' but it's a single
+ // 'std::initializer_list<>'. The destructors of the 2 temporary 'S()'
+ // objects will be called anyway, because they are 2 separate objects in 2
+ // separate clusters, i.e.: not an array.
+ //
+ // Now the 'std::initializer_list<>' is not an array either even though it
+ // has the type of an array. The point is, we only want to invoke the
+ // destructor for the initializer list once not twice or so.
while (const ArrayType *AT = getContext().getAsArrayType(T)) {
T = AT->getElementType();
- CallOpts.IsArrayCtorOrDtor = true;
+
+ // FIXME: Enable this flag once we handle this case properly.
+ // CallOpts.IsArrayCtorOrDtor = true;
}
} else {
- // We'd eventually need to makeElementRegion() trick here,
+ // FIXME: We'd eventually need to makeElementRegion() trick here,
// but for now we don't have the respective construction contexts,
// so MR would always be null in this case. Do nothing for now.
}
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index ced1932a037f3..eacacc4e1d611 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -171,13 +171,14 @@ SVal ExprEngine::computeObjectUnderConstruction(
if (const SubRegion *MR =
dyn_cast_or_null<SubRegion>(V.getAsRegion())) {
if (NE->isArray()) {
- // TODO: In fact, we need to call the constructor for every
- // allocated element, not just the first one!
CallOpts.IsArrayCtorOrDtor = true;
- auto R = MRMgr.getElementRegion(NE->getType()->getPointeeType(),
- svalBuilder.makeArrayIndex(Idx), MR,
- SVB.getContext());
+ auto Ty = NE->getType()->getPointeeType();
+ while (const auto *AT = getContext().getAsArrayType(Ty))
+ Ty = AT->getElementType();
+
+ auto R = MRMgr.getElementRegion(Ty, svalBuilder.makeArrayIndex(Idx),
+ MR, SVB.getContext());
return loc::MemRegionVal(R);
}
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 497bf8b17bc0a..953b819baa303 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -195,6 +195,33 @@ static bool wasDifferentDeclUsedForInlining(CallEventRef<> Call,
return RuntimeCallee->getCanonicalDecl() != StaticDecl->getCanonicalDecl();
}
+// Returns the number of elements in the array currently being destructed.
+// If the element count is not found 0 will be returned.
+static unsigned getElementCountOfArrayBeingDestructed(
+ const CallEvent &Call, const ProgramStateRef State, SValBuilder &SVB) {
+ assert(isa<CXXDestructorCall>(Call) &&
+ "The call event is not a destructor call!");
+
+ const auto &DtorCall = cast<CXXDestructorCall>(Call);
+
+ auto ThisVal = DtorCall.getCXXThisVal();
+
+ if (auto ThisElementRegion = dyn_cast<ElementRegion>(ThisVal.getAsRegion())) {
+ auto ArrayRegion = ThisElementRegion->getAsArrayOffset().getRegion();
+ auto ElementType = ThisElementRegion->getElementType();
+
+ auto ElementCount =
+ getDynamicElementCount(State, ArrayRegion, SVB, ElementType);
+
+ if (!ElementCount.isConstant())
+ return 0;
+
+ return ElementCount.getAsInteger()->getLimitedValue();
+ }
+
+ return 0;
+}
+
/// The call exit is simulated with a sequence of nodes, which occur between
/// CallExitBegin and CallExitEnd. The following operations occur between the
/// two program points:
@@ -234,6 +261,19 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) {
// but we want to evaluate it as many times as many elements the array has.
bool ShouldRepeatCall = false;
+ if (const auto *DtorDecl =
+ dyn_cast_or_null<CXXDestructorDecl>(Call->getDecl())) {
+ if (auto Idx = getPendingArrayDestruction(state, callerCtx)) {
+ ShouldRepeatCall = *Idx > 0;
+
+ auto ThisVal = svalBuilder.getCXXThis(DtorDecl->getParent(), calleeCtx);
+ state = state->killBinding(ThisVal);
+
+ if (!ShouldRepeatCall)
+ state = removePendingArrayDestruction(state, callerCtx);
+ }
+ }
+
// If the callee returns an expression, bind its value to CallExpr.
if (CE) {
if (const ReturnStmt *RS = dyn_cast_or_null<ReturnStmt>(LastSt)) {
@@ -818,11 +858,6 @@ ExprEngine::mayInlineCallKind(const CallEvent &Call, const ExplodedNode *Pred,
!Opts.MayInlineCXXAllocator)
return CIP_DisallowedOnce;
- // FIXME: We don't handle constructors or destructors for arrays properly.
- // Even once we do, we still need to be careful about implicitly-generated
- // initializers for array fields in default move/copy constructors.
- // We still allow construction into ElementRegion targets when they don't
- // represent array elements.
if (CallOpts.IsArrayCtorOrDtor) {
if (!shouldInlineArrayConstruction(Pred->getState(), CtorExpr, CurLC))
return CIP_DisallowedOnce;
@@ -877,9 +912,12 @@ ExprEngine::mayInlineCallKind(const CallEvent &Call, const ExplodedNode *Pred,
assert(ADC->getCFGBuildOptions().AddImplicitDtors && "No CFG destructors");
(void)ADC;
- // FIXME: We don't handle destructors for arrays properly.
- if (CallOpts.IsArrayCtorOrDtor)
- return CIP_DisallowedOnce;
+ if (CallOpts.IsArrayCtorOrDtor) {
+ if (!shouldInlineArrayDestruction(getElementCountOfArrayBeingDestructed(
+ Call, Pred->getState(), svalBuilder))) {
+ return CIP_DisallowedOnce;
+ }
+ }
// Allow disabling temporary destructor inlining with a separate option.
if (CallOpts.IsTemporaryCtorOrDtor &&
@@ -1096,13 +1134,19 @@ bool ExprEngine::shouldInlineArrayConstruction(const ProgramStateRef State,
if (!CE)
return false;
- auto Type = CE->getType();
-
// FIXME: Handle other arrays types.
- if (const auto *CAT = dyn_cast<ConstantArrayType>(Type)) {
- unsigned Size = getContext().getConstantArrayElementCount(CAT);
-
- return Size <= AMgr.options.maxBlockVisitOnPath;
+ if (const auto *CAT = dyn_cast<ConstantArrayType>(CE->getType())) {
+ unsigned ArrSize = getContext().getConstantArrayElementCount(CAT);
+
+ // This might seem conter-intuitive at first glance, but the functions are
+ // closely related. Reasoning about destructors depends only on the type
+ // of the expression that initialized the memory region, which is the
+ // CXXConstructExpr. So to avoid code repetition, the work is delegated
+ // to the function that reasons about destructor inlining. Also note that
+ // if the constructors of the array elements are inlined, the destructors
+ // can also be inlined and if the destructors can be inline, it's safe to
+ // inline the constructors.
+ return shouldInlineArrayDestruction(ArrSize);
}
// Check if we're inside an ArrayInitLoopExpr, and it's sufficiently small.
@@ -1112,6 +1156,14 @@ bool ExprEngine::shouldInlineArrayConstruction(const ProgramStateRef State,
return false;
}
+bool ExprEngine::shouldInlineArrayDestruction(uint64_t Size) {
+
+ uint64_t maxAllowedSize = AMgr.options.maxBlockVisitOnPath;
+
+ // Declaring a 0 element array is also possible.
+ return Size <= maxAllowedSize && Size > 0;
+}
+
bool ExprEngine::shouldRepeatCtorCall(ProgramStateRef State,
const CXXConstructExpr *E,
const LocationContext *LCtx) {
diff --git a/clang/lib/StaticAnalyzer/Core/ProgramState.cpp b/clang/lib/StaticAnalyzer/Core/ProgramState.cpp
index 9395b90a91064..c3c9fc8d9ae6a 100644
--- a/clang/lib/StaticAnalyzer/Core/ProgramState.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -216,8 +216,6 @@ ProgramState::invalidateRegionsImpl(ValueList Values,
}
ProgramStateRef ProgramState::killBinding(Loc LV) const {
- assert(!isa<loc::MemRegionVal>(LV) && "Use invalidateRegion instead.");
-
Store OldStore = getStore();
const StoreRef &newStore =
getStateManager().StoreMgr->killBinding(OldStore, LV);
diff --git a/clang/test/Analysis/dtor-array.cpp b/clang/test/Analysis/dtor-array.cpp
new file mode 100644
index 0000000000000..da1c80e261948
--- /dev/null
+++ b/clang/test/Analysis/dtor-array.cpp
@@ -0,0 +1,347 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-inlining=destructors -verify -std=c++11 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-inlining=destructors -verify -std=c++17 %s
+
+using size_t = __typeof(sizeof(int));
+
+void clang_analyzer_eval(bool);
+void clang_analyzer_checkInlined(bool);
+void clang_analyzer_warnIfReached();
+void clang_analyzer_explain(int);
+
+int a, b, c, d;
+
+struct InlineDtor {
+ static int cnt;
+ static int dtorCalled;
+ ~InlineDtor() {
+ switch (dtorCalled % 4) {
+ case 0:
+ a = cnt++;
+ break;
+ case 1:
+ b = cnt++;
+ break;
+ case 2:
+ c = cnt++;
+ break;
+ case 3:
+ d = cnt++;
+ break;
+ }
+
+ ++dtorCalled;
+ }
+};
+
+int InlineDtor::cnt = 0;
+int InlineDtor::dtorCalled = 0;
+
+void foo() {
+ InlineDtor::cnt = 0;
+ InlineDtor::dtorCalled = 0;
+ InlineDtor arr[4];
+}
+
+void testAutoDtor() {
+ foo();
+
+ clang_analyzer_eval(a == 0); // expected-warning {{TRUE}}
+ clang_analyzer_eval(b == 1); // expected-warning {{TRUE}}
+ clang_analyzer_eval(c == 2); // expected-warning {{TRUE}}
+ clang_analyzer_eval(d == 3); // expected-warning {{TRUE}}
+}
+
+void testDeleteDtor() {
+ InlineDtor::cnt = 10;
+ InlineDtor::dtorCalled = 0;
+
+ InlineDtor *arr = new InlineDtor[4];
+ delete[] arr;
+
+ clang_analyzer_eval(a == 10); // expected-warning {{TRUE}}
+ clang_analyzer_eval(b == 11); // expected-warning {{TRUE}}
+ clang_analyzer_eval(c == 12); // expected-warning {{TRUE}}
+ clang_analyzer_eval(d == 13); // expected-warning {{TRUE}}
+}
+
+struct MemberDtor {
+ InlineDtor arr[4];
+};
+
+void testMemberDtor() {
+ InlineDtor::cnt = 5;
+ InlineDtor::dtorCalled = 0;
+
+ MemberDtor *MD = new MemberDtor{};
+ delete MD;
+
+ clang_analyzer_eval(a == 5); // expected-warning {{TRUE}}
+ clang_analyzer_eval(b == 6); // expected-warning {{TRUE}}
+ clang_analyzer_eval(c == 7); // expected-warning {{TRUE}}
+ clang_analyzer_eval(d == 8); // expected-warning {{TRUE}}
+}
+
+struct MultipleMemberDtor
+{
+ InlineDtor arr[4];
+ InlineDtor arr2[4];
+};
+
+void testMultipleMemberDtor() {
+ InlineDtor::cnt = 30;
+ InlineDtor::dtorCalled = 0;
+
+ MultipleMemberDtor *MD = new MultipleMemberDtor{};
+ delete MD;
+
+ clang_analyzer_eval(a == 34); // expected-warning {{TRUE}}
+ clang_analyzer_eval(b == 35); // expected-warning {{TRUE}}
+ clang_analyzer_eval(c == 36); // expected-warning {{TRUE}}
+ clang_analyzer_eval(d == 37); // expected-warning {{TRUE}}
+}
+
+int EvalOrderArr[4];
+
+struct EvalOrder
+{
+ int ctor = 0;
+ static int dtorCalled;
+ static int ctorCalled;
+
+ EvalOrder() { ctor = ctorCalled++; };
+
+ ~EvalOrder() { EvalOrderArr[ctor] = dtorCalled++; }
+};
+
+int EvalOrder::ctorCalled = 0;
+int EvalOrder::dtorCalled = 0;
+
+void dtorEvaluationOrder() {
+ EvalOrder::ctorCalled = 0;
+ EvalOrder::dtorCalled = 0;
+
+ EvalOrder* eptr = new EvalOrder[4];
+ delete[] eptr;
+
+ clang_analyzer_eval(EvalOrder::dtorCalled == 4); // expected-warning {{TRUE}}
+ clang_analyzer_eval(EvalOrder::dtorCalled == EvalOrder::ctorCalled); // expected-warning {{TRUE}}
+
+ clang_analyzer_eval(EvalOrderArr[0] == 3); // expected-warning {{TRUE}}
+ clang_analyzer_eval(EvalOrderArr[1] == 2); // expected-warning {{TRUE}}
+ clang_analyzer_eval(EvalOrderArr[2] == 1); // expected-warning {{TRUE}}
+ clang_analyzer_eval(EvalOrderArr[3] == 0); // expected-warning {{TRUE}}
+}
+
+struct EmptyDtor {
+ ~EmptyDtor(){};
+};
+
+struct DefaultDtor {
+ ~DefaultDtor() = default;
+};
+
+// This function used to fail on an assertion.
+void no_crash() {
+ EmptyDtor* eptr = new EmptyDtor[4];
+ delete[] eptr;
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+
+ DefaultDtor* dptr = new DefaultDtor[4];
+ delete[] dptr;
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+// This snippet used to crash.
+namespace crash2
+{
+ template <class _Tp> class unique_ptr {
+ typedef _Tp *pointer;
+ pointer __ptr_;
+
+public:
+ unique_ptr(pointer __p) : __ptr_(__p) {}
+ ~unique_ptr() { reset(); }
+ pointer get() { return __ptr_;}
+ void reset() {}
+};
+
+struct S;
+
+S *makeS();
+int bar(S *x, S *y);
+
+void foo() {
+ unique_ptr<S> x(makeS()), y(makeS());
+ bar(x.get(), y.get());
+}
+
+void bar() {
+ foo();
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+} // namespace crash2
+
+// This snippet used to crash.
+namespace crash3
+{
+struct InlineDtor {
+ ~InlineDtor() {}
+};
+struct MultipleMemberDtor
+{
+ InlineDtor arr[4];
+ InlineDtor arr2[4];
+};
+
+void foo(){
+ auto *arr = new MultipleMemberDtor[4];
+ delete[] arr;
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+} // namespace crash3
+
+namespace crash4 {
+struct a {
+ a *b;
+};
+struct c {
+ a d;
+ c();
+ ~c() {
+ for (a e = d;; e = *e.b)
+ ;
+ }
+};
+void f() {
+ c g;
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+} // namespace crash4
+
+namespace crash5 {
+namespace std {
+template <class _Tp> class unique_ptr {
+ _Tp *__ptr_;
+public:
+ unique_ptr(_Tp *__p) : __ptr_(__p) {}
+ ~unique_ptr() {}
+};
+} // namespace std
+
+int SSL_use_certificate(int *arg) {
+ std::unique_ptr<int> free_x509(arg);
+ {
+ if (SSL_use_certificate(arg)) {
+ return 0;
+ }
+ }
+ clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+ return 1;
+}
+
+} // namespace crash5
+
+void zeroLength(){
+ InlineDtor::dtorCalled = 0;
+
+ auto *arr = new InlineDtor[0];
+ delete[] arr;
+
+ auto *arr2 = new InlineDtor[2][0][2];
+ delete[] arr2;
+
+ auto *arr3 = new InlineDtor[0][2][2];
+ delete[] arr3;
+
+ auto *arr4 = new InlineDtor[2][2][0];
+ delete[] arr4;
+
+ // FIXME: Should be TRUE once the constructors are handled properly.
+ clang_analyzer_eval(InlineDtor::dtorCalled == 0); // expected-warning {{TRUE}} expected-warning {{FALSE}}
+}
+
+
+void evalOrderPrep() {
+ EvalOrderArr[0] = 0;
+ EvalOrderArr[1] = 0;
+ EvalOrderArr[2] = 0;
+ EvalOrderArr[3] = 0;
+
+ EvalOrder::ctorCalled = 0;
+ EvalOrder::dtorCalled = 0;
+}
+
+void multidimensionalPrep(){
+ EvalOrder::ctorCalled = 0;
+ EvalOrder::dtorCalled = 0;
+
+ EvalOrder arr[2][2];
+}
+
+void multidimensional(){
+ evalOrderPrep();
+ multidimensionalPrep();
+
+ clang_analyzer_eval(EvalOrder::dtorCalled == 4); // expected-warning {{TRUE}}
+ clang_analyzer_eval(EvalOrder::dtorCalled == EvalOrder::ctorCalled); // expected-warning {{TRUE}}
+
+ clang_analyzer_eval(EvalOrderArr[0] == 3); // expected-warning {{TRUE}}
+ clang_analyzer_eval(EvalOrderArr[1] == 2); // expected-warning {{TRUE}}
+ clang_analyzer_eval(EvalOrderArr[2] == 1); // expected-warning {{TRUE}}
+ clang_analyzer_eval(EvalOrderArr[3] == 0); // expected-warning {{TRUE}}
+}
+
+void multidimensionalHeap() {
+ evalOrderPrep();
+
+ auto* eptr = new EvalOrder[2][2];
+ delete[] eptr;
+
+ clang_analyzer_eval(EvalOrder::dtorCalled == 4); // expected-warning {{TRUE}}
+ clang_analyzer_eval(EvalOrder::dtorCalled == EvalOrder::ctorCalled); // expected-warning {{TRUE}}
+
+ clang_analyzer_eval(EvalOrderArr[0] == 3); // expected-warning {{TRUE}}
+ clang_analyzer_eval(EvalOrderArr[1] == 2); // expected-warning {{TRUE}}
+ clang_analyzer_eval(EvalOrderArr[2] == 1); // expected-warning {{TRUE}}
+ clang_analyzer_eval(EvalOrderArr[3] == 0); // expected-warning {{TRUE}}
+}
+
+struct MultiWrapper{
+ EvalOrder arr[2][2];
+};
+
+void multidimensionalMember(){
+ evalOrderPrep();
+
+ auto* mptr = new MultiWrapper;
+ delete mptr;
+
+ clang_analyzer_eval(EvalOrder::dtorCalled == 4); // expected-warning {{TRUE}}
+ clang_analyzer_eval(EvalOrder::dtorCalled == EvalOrder::ctorCalled); // expected-warning {{TRUE}}
+
+ clang_analyzer_eval(EvalOrderArr[0] == 3); // expected-warning {{TRUE}}
+ clang_analyzer_eval(EvalOrderArr[1] == 2); // expected-warning {{TRUE}}
+ clang_analyzer_eval(EvalOrderArr[2] == 1); // expected-warning {{TRUE}}
+ clang_analyzer_eval(EvalOrderArr[3] == 0); // expected-warning {{TRUE}}
+}
+
+void *memset(void *, int, size_t);
+void clang_analyzer_dumpElementCount(InlineDtor *);
+
+void nonConstantRegionExtent(){
+
+ InlineDtor::dtorCalled = 0;
+
+ int x = 3;
+ memset(&x, 1, sizeof(x));
+
+ InlineDtor *arr = new InlineDtor[x];
+ clang_analyzer_dumpElementCount(arr); // expected-warning {{conj_$0}}
+ delete [] arr;
+
+ //FIXME: This should be TRUE but memset also sets this
+ // region to a conjured symbol.
+ clang_analyzer_eval(InlineDtor::dtorCalled == 0); // expected-warning {{TRUE}} expected-warning {{FALSE}}
+}
diff --git a/clang/test/Analysis/new.cpp b/clang/test/Analysis/new.cpp
index 9ebf3809cdfbf..4b93baf345642 100644
--- a/clang/test/Analysis/new.cpp
+++ b/clang/test/Analysis/new.cpp
@@ -3,6 +3,7 @@
#include "Inputs/system-header-simulator-cxx.h"
void clang_analyzer_eval(bool);
+void clang_analyzer_warnIfReached();
typedef __typeof__(sizeof(int)) size_t;
extern "C" void *malloc(size_t);
@@ -323,9 +324,8 @@ void testArrayNull() {
void testArrayDestr() {
NoReturnDtor *p = new NoReturnDtor[2];
- delete[] p; // Calls the base destructor which aborts, checked below
- // TODO: clang_analyzer_eval should not be called
- clang_analyzer_eval(true); // expected-warning{{TRUE}}
+ delete[] p;
+ clang_analyzer_warnIfReached(); // no-warning
}
// Invalidate Region even in case of default destructor
More information about the cfe-commits
mailing list