NoQ updated this revision to Diff 131321.
NoQ marked 8 inline comments as done.
NoQ added a comment.
Address comments.
https://reviews.llvm.org/D42457
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
test/Analysis/new.cpp
Index: test/Analysis/new.cpp
===================================================================
--- test/Analysis/new.cpp
+++ test/Analysis/new.cpp
@@ -311,8 +311,7 @@
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}}
+ clang_analyzer_eval(true); // no-warning
}
// Invalidate Region even in case of default destructor
Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -622,9 +622,9 @@
CIP_DisallowedAlways
};
-static CallInlinePolicy mayInlineCallKind(const CallEvent &Call,
- const ExplodedNode *Pred,
- AnalyzerOptions &Opts) {
+static CallInlinePolicy
+mayInlineCallKind(const CallEvent &Call, const ExplodedNode *Pred,
+ AnalyzerOptions &Opts, ExprEngine::EvalCallOptions CallOpts) {
const LocationContext *CurLC = Pred->getLocationContext();
const StackFrameContext *CallerSFC = CurLC->getCurrentStackFrame();
switch (Call.getKind()) {
@@ -658,18 +658,8 @@
// initializers for array fields in default move/copy constructors.
// We still allow construction into ElementRegion targets when they don't
// represent array elements.
- const MemRegion *Target = Ctor.getCXXThisVal().getAsRegion();
- if (Target && isa<ElementRegion>(Target)) {
- if (ParentExpr)
- if (const CXXNewExpr *NewExpr = dyn_cast<CXXNewExpr>(ParentExpr))
- if (NewExpr->isArray())
- return CIP_DisallowedOnce;
-
- if (const TypedValueRegion *TR = dyn_cast<TypedValueRegion>(
- cast<SubRegion>(Target)->getSuperRegion()))
- if (TR->getValueType()->isArrayType())
- return CIP_DisallowedOnce;
- }
+ if (CallOpts.IsArrayConstructorOrDestructor)
+ return CIP_DisallowedOnce;
// Inlining constructors requires including initializers in the CFG.
const AnalysisDeclContext *ADC = CallerSFC->getAnalysisDeclContext();
@@ -688,7 +678,7 @@
// FIXME: This is a hack. We don't handle temporary destructors
// right now, so we shouldn't inline their constructors.
if (CtorExpr->getConstructionKind() == CXXConstructExpr::CK_Complete)
- if (!Target || isa<CXXTempObjectRegion>(Target))
+ if (CallOpts.IsConstructorWithImproperlyModeledTargetRegion)
return CIP_DisallowedOnce;
break;
@@ -702,11 +692,8 @@
assert(ADC->getCFGBuildOptions().AddImplicitDtors && "No CFG destructors");
(void)ADC;
- const CXXDestructorCall &Dtor = cast<CXXDestructorCall>(Call);
-
// FIXME: We don't handle constructors or destructors for arrays properly.
- const MemRegion *Target = Dtor.getCXXThisVal().getAsRegion();
- if (Target && isa<ElementRegion>(Target))
+ if (CallOpts.IsArrayConstructorOrDestructor)
return CIP_DisallowedOnce;
break;
@@ -847,7 +834,8 @@
}
bool ExprEngine::shouldInlineCall(const CallEvent &Call, const Decl *D,
- const ExplodedNode *Pred) {
+ const ExplodedNode *Pred,
+ EvalCallOptions CallOpts) {
if (!D)
return false;
@@ -894,7 +882,7 @@
// FIXME: this checks both static and dynamic properties of the call, which
// means we're redoing a bit of work that could be cached in the function
// summary.
- CallInlinePolicy CIP = mayInlineCallKind(Call, Pred, Opts);
+ CallInlinePolicy CIP = mayInlineCallKind(Call, Pred, Opts, CallOpts);
if (CIP != CIP_Allowed) {
if (CIP == CIP_DisallowedAlways) {
assert(!MayInline.hasValue() || MayInline.getValue());
@@ -946,7 +934,8 @@
}
void ExprEngine::defaultEvalCall(NodeBuilder &Bldr, ExplodedNode *Pred,
- const CallEvent &CallTemplate) {
+ const CallEvent &CallTemplate,
+ EvalCallOptions CallOpts) {
// Make sure we have the most recent state attached to the call.
ProgramStateRef State = Pred->getState();
CallEventRef<> Call = CallTemplate.cloneWithState(State);
@@ -969,7 +958,7 @@
} else {
RuntimeDefinition RD = Call->getRuntimeDefinition();
const Decl *D = RD.getDecl();
- if (shouldInlineCall(*Call, D, Pred)) {
+ if (shouldInlineCall(*Call, D, Pred, CallOpts)) {
if (RD.mayHaveOtherDefinitions()) {
AnalyzerOptions &Options = getAnalysisManager().options;
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -85,28 +85,33 @@
/// Returns a region representing the first element of a (possibly
-/// multi-dimensional) array.
+/// multi-dimensional) array, for the purposes of element construction or
+/// destruction.
///
/// On return, \p Ty will be set to the base type of the array.
///
/// If the type is not an array type at all, the original value is returned.
+/// Otherwise the "IsArrayConstructorOrDestructor" flag is set in CallOpts.
static SVal makeZeroElementRegion(ProgramStateRef State, SVal LValue,
- QualType &Ty) {
+ QualType &Ty,
+ ExprEngine::EvalCallOptions &CallOpts) {
SValBuilder &SVB = State->getStateManager().getSValBuilder();
ASTContext &Ctx = SVB.getContext();
while (const ArrayType *AT = Ctx.getAsArrayType(Ty)) {
Ty = AT->getElementType();
LValue = State->getLValue(Ty, SVB.makeZeroArrayIndex(), LValue);
+ CallOpts.IsArrayConstructorOrDestructor = true;
}
return LValue;
}
const MemRegion *
ExprEngine::getRegionForConstructedObject(const CXXConstructExpr *CE,
- ExplodedNode *Pred) {
+ ExplodedNode *Pred,
+ EvalCallOptions &CallOpts) {
const LocationContext *LCtx = Pred->getLocationContext();
ProgramStateRef State = Pred->getState();
@@ -122,9 +127,9 @@
if (const SubRegion *MR = dyn_cast_or_null<SubRegion>(
getCXXNewAllocatorValue(State, CNE, LCtx).getAsRegion())) {
if (CNE->isArray()) {
- // TODO: This code exists only to trigger the suppression for
- // array constructors. In fact, we need to call the constructor
- // for every allocated element, not just the first one!
+ // TODO: In fact, we need to call the constructor for every
+ // allocated element, not just the first one!
+ CallOpts.IsArrayConstructorOrDestructor = true;
return getStoreManager().GetElementZeroRegion(
MR, CNE->getType()->getPointeeType());
}
@@ -136,7 +141,7 @@
if (Var->getInit() && Var->getInit()->IgnoreImplicit() == CE) {
SVal LValue = State->getLValue(Var, LCtx);
QualType Ty = Var->getType();
- LValue = makeZeroElementRegion(State, LValue, Ty);
+ LValue = makeZeroElementRegion(State, LValue, Ty, CallOpts);
return LValue.getAsRegion();
}
}
@@ -162,16 +167,17 @@
}
QualType Ty = Field->getType();
- FieldVal = makeZeroElementRegion(State, FieldVal, Ty);
+ FieldVal = makeZeroElementRegion(State, FieldVal, Ty, CallOpts);
return FieldVal.getAsRegion();
}
// FIXME: This will eventually need to handle new-expressions as well.
// Don't forget to update the pre-constructor initialization code in
// ExprEngine::VisitCXXConstructExpr.
}
// If we couldn't find an existing region to construct into, assume we're
- // constructing a temporary.
+ // constructing a temporary. Notify the caller of our failure.
+ CallOpts.IsConstructorWithImproperlyModeledTargetRegion = true;
MemRegionManager &MRMgr = getSValBuilder().getRegionManager();
return MRMgr.getCXXTempObjectRegion(CE, LCtx);
}
@@ -265,9 +271,11 @@
// For now, we just run the first constructor (which should still invalidate
// the entire array).
+ EvalCallOptions CallOpts;
+
switch (CE->getConstructionKind()) {
case CXXConstructExpr::CK_Complete: {
- Target = getRegionForConstructedObject(CE, Pred);
+ Target = getRegionForConstructedObject(CE, Pred, CallOpts);
break;
}
case CXXConstructExpr::CK_VirtualBase:
@@ -304,6 +312,7 @@
if (dyn_cast_or_null<InitListExpr>(LCtx->getParentMap().getParent(CE))) {
MemRegionManager &MRMgr = getSValBuilder().getRegionManager();
Target = MRMgr.getCXXTempObjectRegion(CE, LCtx);
+ CallOpts.IsConstructorWithImproperlyModeledTargetRegion = true;
break;
}
// FALLTHROUGH
@@ -371,19 +380,18 @@
ExplodedNodeSet DstEvaluated;
StmtNodeBuilder Bldr(DstPreCall, DstEvaluated, *currBldrCtx);
- bool IsArray = isa<ElementRegion>(Target);
if (CE->getConstructor()->isTrivial() &&
CE->getConstructor()->isCopyOrMoveConstructor() &&
- !IsArray) {
+ !CallOpts.IsArrayConstructorOrDestructor) {
// FIXME: Handle other kinds of trivial constructors as well.
for (ExplodedNodeSet::iterator I = DstPreCall.begin(), E = DstPreCall.end();
I != E; ++I)
performTrivialCopy(Bldr, *I, *Call);
} else {
for (ExplodedNodeSet::iterator I = DstPreCall.begin(), E = DstPreCall.end();
I != E; ++I)
- defaultEvalCall(Bldr, *I, *Call);
+ defaultEvalCall(Bldr, *I, *Call, CallOpts);
}
// If the CFG was contructed without elements for temporary destructors
@@ -431,7 +439,10 @@
SVal DestVal = UnknownVal();
if (Dest)
DestVal = loc::MemRegionVal(Dest);
- DestVal = makeZeroElementRegion(State, DestVal, ObjectType);
+
+ EvalCallOptions CallOpts;
+ DestVal = makeZeroElementRegion(State, DestVal, ObjectType, CallOpts);
+
Dest = DestVal.getAsRegion();
const CXXRecordDecl *RecordDecl = ObjectType->getAsCXXRecordDecl();
@@ -454,7 +465,7 @@
StmtNodeBuilder Bldr(DstPreCall, DstInvalidated, *currBldrCtx);
for (ExplodedNodeSet::iterator I = DstPreCall.begin(), E = DstPreCall.end();
I != E; ++I)
- defaultEvalCall(Bldr, *I, *Call);
+ defaultEvalCall(Bldr, *I, *Call, CallOpts);
ExplodedNodeSet DstPostCall;
getCheckerManager().runCheckersForPostCall(Dst, DstInvalidated,
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===================================================================
--- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -567,14 +567,25 @@
const LocationContext *LCtx,
ProgramStateRef State);
+ struct EvalCallOptions {
+ bool IsConstructorWithImproperlyModeledTargetRegion : 1;
+ bool IsArrayConstructorOrDestructor : 1;
+
+ // Bitfield inline initialization is C++20.
+ EvalCallOptions()
+ : IsConstructorWithImproperlyModeledTargetRegion(false),
+ IsArrayConstructorOrDestructor(false) {}
+ };
+
/// Evaluate a call, running pre- and post-call checks and allowing checkers
/// to be responsible for handling the evaluation of the call itself.
void evalCall(ExplodedNodeSet &Dst, ExplodedNode *Pred,
const CallEvent &Call);
/// \brief Default implementation of call evaluation.
void defaultEvalCall(NodeBuilder &B, ExplodedNode *Pred,
- const CallEvent &Call);
+ const CallEvent &Call, EvalCallOptions CallOpts = {});
+
private:
void evalLoadCommon(ExplodedNodeSet &Dst,
const Expr *NodeEx, /* Eventually will be a CFGStmt */
@@ -600,7 +611,8 @@
/// Checks our policies and decides weither the given call should be inlined.
bool shouldInlineCall(const CallEvent &Call, const Decl *D,
- const ExplodedNode *Pred);
+ const ExplodedNode *Pred,
+ EvalCallOptions CallOpts = {});
bool inlineCall(const CallEvent &Call, const Decl *D, NodeBuilder &Bldr,
ExplodedNode *Pred, ProgramStateRef State);
@@ -650,11 +662,11 @@
/// For a given constructor, look forward in the current CFG block to
/// determine the region into which an object will be constructed by \p CE.
- /// Returns either a field or local variable region if the object will be
- /// directly constructed in an existing region or a temporary object region
- /// if not.
+ /// When the lookahead fails, a temporary region is returned, and the
+ /// IsConstructorWithImproperlyModeledTargetRegion flag is set in \p CallOpts.
const MemRegion *getRegionForConstructedObject(const CXXConstructExpr *CE,
- ExplodedNode *Pred);
+ ExplodedNode *Pred,
+ EvalCallOptions &CallOpts);
/// Store the region returned by operator new() so that the constructor
/// that follows it knew what location to initialize. The value should be
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
