From fdaa33818cf9bad8d092136e73bd2e489cb821ba Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Tue, 3 Jul 2012 22:55:57 +0000 Subject: [PATCH] [analyzer] For now, don't inline non-static member overloaded operators. Our current inlining support (specifically RegionStore::enterStackFrame) doesn't know that calls to overloaded operators may be calls to non-static member functions, and that in these cases the first argument should be treated as 'this'. This caused incorrect results and sometimes crashes. The long-term fix will be to rewrite RegionStore::enterStackFrame to use CallEvent and its subclasses, but for now we can just disable these problematic calls by classifying them under a new CallEvent, CXXMemberOperatorCall. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@159692 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../StaticAnalyzer/Core/PathSensitive/Calls.h | 30 ++++++++++- .../Checkers/RetainCountChecker.cpp | 1 + lib/StaticAnalyzer/Core/Calls.cpp | 8 +++ .../Core/ExprEngineCallAndReturn.cpp | 50 ++++++++++++++++--- test/Analysis/operator-calls.cpp | 19 ++++++- 5 files changed, 98 insertions(+), 10 deletions(-) diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/Calls.h b/include/clang/StaticAnalyzer/Core/PathSensitive/Calls.h index f2c251f86f..5ab13567cd 100644 --- a/include/clang/StaticAnalyzer/Core/PathSensitive/Calls.h +++ b/include/clang/StaticAnalyzer/Core/PathSensitive/Calls.h @@ -29,6 +29,7 @@ namespace ento { enum CallEventKind { CE_Function, CE_CXXMember, + CE_CXXMemberOperator, CE_Block, CE_BEG_SIMPLE_CALLS = CE_Function, CE_END_SIMPLE_CALLS = CE_Block, @@ -264,9 +265,36 @@ public: } }; +/// \brief Represents a C++ overloaded operator call where the operator is +/// implemented as a non-static member function. +/// +/// Example: iter + 1 +class CXXMemberOperatorCall : public SimpleCall { +protected: + void addExtraInvalidatedRegions(RegionList &Regions) const; + +public: + CXXMemberOperatorCall(const CXXOperatorCallExpr *CE, ProgramStateRef St, + const LocationContext *LCtx) + : SimpleCall(CE, St, LCtx, CE_CXXMemberOperator) {} + + const CXXOperatorCallExpr *getOriginExpr() const { + return cast(SimpleCall::getOriginExpr()); + } + + unsigned getNumArgs() const { return getOriginExpr()->getNumArgs() - 1; } + const Expr *getArgExpr(unsigned Index) const { + return getOriginExpr()->getArg(Index + 1); + } + + static bool classof(const CallEvent *CA) { + return CA->getKind() == CE_CXXMemberOperator; + } +}; + /// \brief Represents a call to a block. /// -/// Example: \c ^{ /* ... */ }() +/// Example: ^{ /* ... */ }() class BlockCall : public SimpleCall { protected: void addExtraInvalidatedRegions(RegionList &Regions) const; diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp index 5940131a5b..d0618d0ddd 100644 --- a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp @@ -945,6 +945,7 @@ RetainSummaryManager::getSummary(const CallEvent &Call, Summ = getFunctionSummary(cast(Call).getDecl()); break; case CE_CXXMember: + case CE_CXXMemberOperator: case CE_Block: case CE_CXXConstructor: case CE_CXXAllocator: diff --git a/lib/StaticAnalyzer/Core/Calls.cpp b/lib/StaticAnalyzer/Core/Calls.cpp index 35ddfc965f..24c5ab12ab 100644 --- a/lib/StaticAnalyzer/Core/Calls.cpp +++ b/lib/StaticAnalyzer/Core/Calls.cpp @@ -303,6 +303,14 @@ void CXXMemberCall::addExtraInvalidatedRegions(RegionList &Regions) const { } +void +CXXMemberOperatorCall::addExtraInvalidatedRegions(RegionList &Regions) const { + const Expr *Base = getOriginExpr()->getArg(0); + if (const MemRegion *R = getSVal(Base).getAsRegion()) + Regions.push_back(R); +} + + const BlockDataRegion *BlockCall::getBlockRegion() const { const Expr *Callee = getOriginExpr()->getCallee(); const MemRegion *DataReg = getSVal(Callee).getAsRegion(); diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index 2c749f1e0a..b2b1ab561b 100644 --- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -22,6 +22,23 @@ using namespace clang; using namespace ento; +static CallEventKind classifyCallExpr(const CallExpr *CE) { + if (isa(CE)) + return CE_CXXMember; + + const CXXOperatorCallExpr *OpCE = dyn_cast(CE); + if (OpCE) { + const FunctionDecl *DirectCallee = CE->getDirectCallee(); + if (const CXXMethodDecl *MD = dyn_cast(DirectCallee)) + if (MD->isInstance()) + return CE_CXXMemberOperator; + } else if (CE->getCallee()->getType()->isBlockPointerType()) { + return CE_Block; + } + + return CE_Function; +} + void ExprEngine::processCallEnter(CallEnter CE, ExplodedNode *Pred) { // Get the entry block in the CFG of the callee. const StackFrameContext *calleeCtx = CE.getCalleeContext(); @@ -265,6 +282,12 @@ bool ExprEngine::inlineCall(ExplodedNodeSet &Dst, case CE_CXXMember: // These are always at least possible to inline. break; + case CE_CXXMemberOperator: + // FIXME: This should be possible to inline, but + // RegionStore::enterStackFrame isn't smart enough to handle the first + // argument being 'this'. The correct solution is to use CallEvent in + // enterStackFrame as well. + return false; case CE_CXXConstructor: // Do not inline constructors until we can model destructors. // This is unfortunate, but basically necessary for smart pointers and such. @@ -336,9 +359,7 @@ void ExprEngine::VisitCallExpr(const CallExpr *CE, ExplodedNode *Pred, getCheckerManager().runCheckersForPreStmt(dstPreVisit, Pred, CE, *this); // Get the callee kind. - const CXXMemberCallExpr *MemberCE = dyn_cast(CE); - bool IsBlock = (MemberCE ? false - : CE->getCallee()->getType()->isBlockPointerType()); + CallEventKind K = classifyCallExpr(CE); // Evaluate the function call. We try each of the checkers // to see if the can evaluate the function call. @@ -349,12 +370,25 @@ void ExprEngine::VisitCallExpr(const CallExpr *CE, ExplodedNode *Pred, const LocationContext *LCtx = (*I)->getLocationContext(); // Evaluate the call. - if (MemberCE) - evalCall(dstCallEvaluated, *I, CXXMemberCall(MemberCE, State, LCtx)); - else if (IsBlock) - evalCall(dstCallEvaluated, *I, BlockCall(CE, State, LCtx)); - else + switch (K) { + case CE_Function: evalCall(dstCallEvaluated, *I, FunctionCall(CE, State, LCtx)); + break; + case CE_CXXMember: + evalCall(dstCallEvaluated, *I, CXXMemberCall(cast(CE), + State, LCtx)); + break; + case CE_CXXMemberOperator: + evalCall(dstCallEvaluated, *I, + CXXMemberOperatorCall(cast(CE), + State, LCtx)); + break; + case CE_Block: + evalCall(dstCallEvaluated, *I, BlockCall(CE, State, LCtx)); + break; + default: + llvm_unreachable("Non-CallExpr CallEventKind"); + } } // Finally, perform the post-condition check of the CallExpr and store diff --git a/test/Analysis/operator-calls.cpp b/test/Analysis/operator-calls.cpp index 73cd28ac0d..e81f428c6f 100644 --- a/test/Analysis/operator-calls.cpp +++ b/test/Analysis/operator-calls.cpp @@ -1,4 +1,6 @@ -// RUN: %clang_cc1 -analyze -analyzer-checker=core,experimental.core -verify %s +// RUN: %clang_cc1 -analyze -analyzer-checker=core,experimental.core,debug.ExprInspection -verify %s +void clang_analyzer_eval(bool); + struct X0 { }; bool operator==(const X0&, const X0&); @@ -14,3 +16,18 @@ void t2() { bool PR7287(X0 a, X0 b) { return operator==(a, b); } + + +// Inlining non-static member operators mistakenly treated 'this' as the first +// argument for a while. + +struct IntComparable { + bool operator==(int x) const { + return x == 0; + } +}; + +void testMemberOperator(IntComparable B) { + // FIXME: Change this to TRUE when we re-enable inlining. + clang_analyzer_eval(B == 0); // expected-warning{{UNKNOWN}} +} -- 2.40.0