[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

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/Calls.h b/include/clang/StaticAnalyzer/Core/PathSensitive/Calls.h
index f2c251f86fc11b02627b46f08cc950142918a8f9..5ab13567cd21f6c02563fa408d359ad919a84936 100644 (file)
--- 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: <tt>iter + 1</tt>
+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<CXXOperatorCallExpr>(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: <tt>^{ /* ... */ }()</tt>
 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 5940131a5b8cfe855ed0f7c5ef7a24343e493395..d0618d0dddfe0b8e369ee99cca3507b9d85c7fe7 100644 (file)
--- a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
@@ -945,6 +945,7 @@ RetainSummaryManager::getSummary(const CallEvent &Call,
     Summ = getFunctionSummary(cast<FunctionCall>(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 35ddfc965f102f0e8ab889c983d5ad2544756059..24c5ab12ab122d3a1f28cab78093e95d20b7ff4f 100644 (file)
--- 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 2c749f1e0ae11922b910884b80e4280be446411e..b2b1ab561b0f76265abca5b3b5777df5bfc09c2b 100644 (file)
--- 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<CXXMemberCallExpr>(CE))
+    return CE_CXXMember;
+
+  const CXXOperatorCallExpr *OpCE = dyn_cast<CXXOperatorCallExpr>(CE);
+  if (OpCE) {
+    const FunctionDecl *DirectCallee = CE->getDirectCallee();
+    if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(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<CXXMemberCallExpr>(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<CXXMemberCallExpr>(CE),
+                                                   State, LCtx));
+      break;
+    case CE_CXXMemberOperator:
+      evalCall(dstCallEvaluated, *I,
+               CXXMemberOperatorCall(cast<CXXOperatorCallExpr>(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 73cd28ac0d56fac47df45bc6667b86b607b9ce23..e81f428c6f50336fc140e99df41855ab019826ab 100644 (file)
--- 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}}
+}