From: Ted Kremenek Date: Fri, 8 Apr 2011 22:42:35 +0000 (+0000) Subject: Start overhauling static analyzer support for C++ constructors. The inlining support... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=5fe98728dca1f3a7a378ce1a21984a0f8a0c0b8b;p=clang Start overhauling static analyzer support for C++ constructors. The inlining support isn't complete, and needs to be reworked to model CallEnter/CallExit (just like all other calls). For now, treat constructors mostly like other function calls, making the analysis of C++ code just a little more useful. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@129166 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/StaticAnalyzer/Core/CXXExprEngine.cpp b/lib/StaticAnalyzer/Core/CXXExprEngine.cpp index b299fcc1c1..6365bce05f 100644 --- a/lib/StaticAnalyzer/Core/CXXExprEngine.cpp +++ b/lib/StaticAnalyzer/Core/CXXExprEngine.cpp @@ -124,53 +124,121 @@ void ExprEngine::CreateCXXTemporaryObject(const Expr *Ex, ExplodedNode *Pred, } void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *E, - const MemRegion *Dest, - ExplodedNode *Pred, - ExplodedNodeSet &Dst) { - if (!Dest) - Dest = svalBuilder.getRegionManager().getCXXTempObjectRegion(E, - Pred->getLocationContext()); - - if (E->isElidable()) { - VisitAggExpr(E->getArg(0), Dest, Pred, Dst); - // FIXME: this is here to force propogation if VisitAggExpr doesn't - if (Dst.empty()) - Dst.Add(Pred); - return; - } + const MemRegion *Dest, + ExplodedNode *Pred, + ExplodedNodeSet &destNodes) { const CXXConstructorDecl *CD = E->getConstructor(); assert(CD); - + +#if 0 if (!(CD->isThisDeclarationADefinition() && AMgr.shouldInlineCall())) // FIXME: invalidate the object. return; - +#endif // Evaluate other arguments. ExplodedNodeSet argsEvaluated; const FunctionProtoType *FnType = CD->getType()->getAs(); evalArguments(E->arg_begin(), E->arg_end(), FnType, Pred, argsEvaluated); - // The callee stack frame context used to create the 'this' parameter region. - const StackFrameContext *SFC = AMgr.getStackFrame(CD, - Pred->getLocationContext(), - E, Builder->getBlock(), - Builder->getIndex()); - const CXXThisRegion *ThisR =getCXXThisRegion(E->getConstructor()->getParent(), - SFC); - - CallEnter Loc(E, SFC, Pred->getLocationContext()); - for (ExplodedNodeSet::iterator NI = argsEvaluated.begin(), - NE = argsEvaluated.end(); NI != NE; ++NI) { - const GRState *state = GetState(*NI); - // Setup 'this' region, so that the ctor is evaluated on the object pointed - // by 'Dest'. - state = state->bindLoc(loc::MemRegionVal(ThisR), loc::MemRegionVal(Dest)); - ExplodedNode *N = Builder->generateNode(Loc, state, Pred); - if (N) - Dst.Add(N); +#if 0 + // Is the constructor elidable? + if (E->isElidable()) { + VisitAggExpr(E->getArg(0), destNodes, Pred, Dst); + // FIXME: this is here to force propogation if VisitAggExpr doesn't + if (destNodes.empty()) + destNodes.Add(Pred); + return; } +#endif + + // Perform the previsit of the constructor. + ExplodedNodeSet destPreVisit; + getCheckerManager().runCheckersForPreStmt(destPreVisit, argsEvaluated, E, + *this); + + // Evaluate the constructor. Currently we don't now allow checker-specific + // implementations of specific constructors (as we do with ordinary + // function calls. We can re-evaluate this in the future. + +#if 0 + // Inlining currently isn't fully implemented. + + if (AMgr.shouldInlineCall()) { + if (!Dest) + Dest = + svalBuilder.getRegionManager().getCXXTempObjectRegion(E, + Pred->getLocationContext()); + + // The callee stack frame context used to create the 'this' + // parameter region. + const StackFrameContext *SFC = + AMgr.getStackFrame(CD, Pred->getLocationContext(), + E, Builder->getBlock(), Builder->getIndex()); + + // Create the 'this' region. + const CXXThisRegion *ThisR = + getCXXThisRegion(E->getConstructor()->getParent(), SFC); + + CallEnter Loc(E, SFC, Pred->getLocationContext()); + + + for (ExplodedNodeSet::iterator NI = argsEvaluated.begin(), + NE = argsEvaluated.end(); NI != NE; ++NI) { + const GRState *state = GetState(*NI); + // Setup 'this' region, so that the ctor is evaluated on the object pointed + // by 'Dest'. + state = state->bindLoc(loc::MemRegionVal(ThisR), loc::MemRegionVal(Dest)); + if (ExplodedNode *N = Builder->generateNode(Loc, state, *NI)) + destNodes.Add(N); + } + } +#endif + + // Default semantics: invalidate all regions passed as arguments. + llvm::SmallVector regionsToInvalidate; + + // FIXME: We can have collisions on the conjured symbol if the + // expression *I also creates conjured symbols. We probably want + // to identify conjured symbols by an expression pair: the enclosing + // expression (the context) and the expression itself. This should + // disambiguate conjured symbols. + unsigned blockCount = Builder->getCurrentBlockCount(); + + // NOTE: Even if RegionsToInvalidate is empty, we must still invalidate + // global variables. + ExplodedNodeSet destCall; + + for (ExplodedNodeSet::iterator + i = destPreVisit.begin(), e = destPreVisit.end(); + i != e; ++i) + { + ExplodedNode *Pred = *i; + const GRState *state = GetState(Pred); + + // Accumulate list of regions that are invalidated. + for (CXXConstructExpr::const_arg_iterator + ai = E->arg_begin(), ae = E->arg_end(); + ai != ae; ++ai) + { + SVal val = state->getSVal(*ai); + if (const MemRegion *region = val.getAsRegion()) + regionsToInvalidate.push_back(region); + } + + // Invalidate the regions. + state = state->invalidateRegions(regionsToInvalidate.data(), + regionsToInvalidate.data() + + regionsToInvalidate.size(), + E, blockCount, 0, + /* invalidateGlobals = */ true); + + Builder->MakeNode(destCall, E, Pred, state); + } + + // Do the post visit. + getCheckerManager().runCheckersForPostStmt(destNodes, destCall, E, *this); } void ExprEngine::VisitCXXDestructor(const CXXDestructorDecl *DD, diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp index 2fd7b3b88a..0a7f1e93af 100644 --- a/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -422,7 +422,6 @@ void ExprEngine::Visit(const Stmt* S, ExplodedNode* Pred, // C++ stuff we don't support yet. case Stmt::CXXBindTemporaryExprClass: case Stmt::CXXCatchStmtClass: - case Stmt::CXXDefaultArgExprClass: case Stmt::CXXDependentScopeMemberExprClass: case Stmt::CXXNullPtrLiteralExprClass: case Stmt::CXXPseudoDestructorExprClass: @@ -448,7 +447,14 @@ void ExprEngine::Visit(const Stmt* S, ExplodedNode* Pred, Engine.addAbortedBlock(node, Builder->getBlock()); break; } - + + // We don't handle default arguments either yet, but we can fake it + // for now by just skipping them. + case Stmt::CXXDefaultArgExprClass: { + Dst.Add(Pred); + break; + } + case Stmt::ParenExprClass: llvm_unreachable("ParenExprs already handled."); // Cases that should never be evaluated simply because they shouldn't diff --git a/test/Analysis/misc-ps-region-store.cpp b/test/Analysis/misc-ps-region-store.cpp index 4ead8bb617..2b79bdbd82 100644 --- a/test/Analysis/misc-ps-region-store.cpp +++ b/test/Analysis/misc-ps-region-store.cpp @@ -278,3 +278,28 @@ const Rdar9212495_A& rdar9212495(const Rdar9212495_C* ptr) { return val; } +// Test constructors invalidating arguments. Previously this raised +// an uninitialized value warning. +extern "C" void __attribute__((noreturn)) PR9645_exit(int i); + +class PR9645_SideEffect +{ +public: + PR9645_SideEffect(int *pi); // caches pi in i_ + void Read(int *pi); // copies *pi into *i_ +private: + int *i_; +}; + +void PR9645() { + int i; + + PR9645_SideEffect se(&i); + int j = 1; + se.Read(&j); // this has a side-effect of initializing i. + + PR9645_exit(i); // no-warning +} + +PR9645_SideEffect::PR9645_SideEffect(int *pi) : i_(pi) {} +void PR9645_SideEffect::Read(int *pi) { *i_ = *pi; }