From 2ff5a1edc04d447e0360c4e4ce92e0803c6b0e3a Mon Sep 17 00:00:00 2001
From: Jordan Rose <jordan_rose@apple.com>
Date: Wed, 15 Jan 2014 17:25:15 +0000
Subject: [PATCH] [analyzer] BlockCall shouldn't really be an AnyFunctionCall.

Per discussion with Anna a /long/ time ago, it was way too easy to misuse
BlockCall: because it inherited from AnyFunctionCall (through SimpleCall),
getDecl() was constrained to return a FunctionDecl, and you had to call
getBlockDecl() instead. This goes against the whole point of CallEvent
(to abstract over different ways to invoke bodies of code).

Now, BlockCall just inherits directly from CallEvent. There's a bit of
duplication in getting things out of the origin expression (which is still
known to be a CallExpr), but nothing significant.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@199321 91177308-0d34-0410-b5e6-96231b3b80d8
---
 .../StaticAnalyzer/Core/CheckerManager.h      |  1 -
 .../Core/PathSensitive/CallEvent.h            | 82 ++++++++++---------
 .../Core/PathSensitive/ExprEngine.h           |  1 -
 lib/StaticAnalyzer/Checkers/MallocChecker.cpp |  6 +-
 .../Checkers/RetainCountChecker.cpp           |  4 +-
 lib/StaticAnalyzer/Core/CallEvent.cpp         |  8 +-
 6 files changed, 52 insertions(+), 50 deletions(-)

diff --git a/include/clang/StaticAnalyzer/Core/CheckerManager.h b/include/clang/StaticAnalyzer/Core/CheckerManager.h
index b6758ae57af..63b3c3a65a0 100644
--- a/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -33,7 +33,6 @@ namespace ento {
   class AnalysisManager;
   class BugReporter;
   class CheckerContext;
-  class SimpleCall;
   class ObjCMethodCall;
   class SVal;
   class ExplodedNode;
diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h b/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
index e2baf38e0be..396a9fe3644 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -33,9 +33,6 @@ namespace ento {
 
 enum CallEventKind {
   CE_Function,
-  CE_Block,
-  CE_BEG_SIMPLE_CALLS = CE_Function,
-  CE_END_SIMPLE_CALLS = CE_Block,
   CE_CXXMember,
   CE_CXXMemberOperator,
   CE_CXXDestructor,
@@ -45,6 +42,7 @@ enum CallEventKind {
   CE_CXXAllocator,
   CE_BEG_FUNCTION_CALLS = CE_Function,
   CE_END_FUNCTION_CALLS = CE_CXXAllocator,
+  CE_Block,
   CE_ObjCMessage
 };
 
@@ -431,13 +429,21 @@ public:
   }
 };
 
-/// \brief Represents a call to a non-C++ function, written as a CallExpr.
-class SimpleCall : public AnyFunctionCall {
+/// \brief Represents a C function or static C++ member function call.
+///
+/// Example: \c fun()
+class SimpleFunctionCall : public AnyFunctionCall {
+  friend class CallEventManager;
+
 protected:
-  SimpleCall(const CallExpr *CE, ProgramStateRef St,
-             const LocationContext *LCtx)
+  SimpleFunctionCall(const CallExpr *CE, ProgramStateRef St,
+                     const LocationContext *LCtx)
     : AnyFunctionCall(CE, St, LCtx) {}
-  SimpleCall(const SimpleCall &Other) : AnyFunctionCall(Other) {}
+  SimpleFunctionCall(const SimpleFunctionCall &Other)
+    : AnyFunctionCall(Other) {}
+  virtual void cloneTo(void *Dest) const {
+    new (Dest) SimpleFunctionCall(*this);
+  }
 
 public:
   virtual const CallExpr *getOriginExpr() const {
@@ -452,27 +458,6 @@ public:
     return getOriginExpr()->getArg(Index);
   }
 
-  static bool classof(const CallEvent *CA) {
-    return CA->getKind() >= CE_BEG_SIMPLE_CALLS &&
-           CA->getKind() <= CE_END_SIMPLE_CALLS;
-  }
-};
-
-/// \brief Represents a C function or static C++ member function call.
-///
-/// Example: \c fun()
-class FunctionCall : public SimpleCall {
-  friend class CallEventManager;
-
-protected:
-  FunctionCall(const CallExpr *CE, ProgramStateRef St,
-               const LocationContext *LCtx)
-    : SimpleCall(CE, St, LCtx) {}
-
-  FunctionCall(const FunctionCall &Other) : SimpleCall(Other) {}
-  virtual void cloneTo(void *Dest) const { new (Dest) FunctionCall(*this); }
-
-public:
   virtual Kind getKind() const { return CE_Function; }
 
   static bool classof(const CallEvent *CA) {
@@ -483,30 +468,36 @@ public:
 /// \brief Represents a call to a block.
 ///
 /// Example: <tt>^{ /* ... */ }()</tt>
-class BlockCall : public SimpleCall {
+class BlockCall : public CallEvent {
   friend class CallEventManager;
 
 protected:
   BlockCall(const CallExpr *CE, ProgramStateRef St,
             const LocationContext *LCtx)
-    : SimpleCall(CE, St, LCtx) {}
+    : CallEvent(CE, St, LCtx) {}
 
-  BlockCall(const BlockCall &Other) : SimpleCall(Other) {}
+  BlockCall(const BlockCall &Other) : CallEvent(Other) {}
   virtual void cloneTo(void *Dest) const { new (Dest) BlockCall(*this); }
 
   virtual void getExtraInvalidatedValues(ValueList &Values) const;
 
 public:
+  virtual const CallExpr *getOriginExpr() const {
+    return cast<CallExpr>(CallEvent::getOriginExpr());
+  }
+
+  virtual unsigned getNumArgs() const { return getOriginExpr()->getNumArgs(); }
+
+  virtual const Expr *getArgExpr(unsigned Index) const {
+    return getOriginExpr()->getArg(Index);
+  }
+
   /// \brief Returns the region associated with this instance of the block.
   ///
   /// This may be NULL if the block's origin is unknown.
   const BlockDataRegion *getBlockRegion() const;
 
-  /// \brief Gets the declaration of the block.
-  ///
-  /// This is not an override of getDecl() because AnyFunctionCall has already
-  /// assumed that it's a FunctionDecl.
-  const BlockDecl *getBlockDecl() const {
+  virtual const BlockDecl *getDecl() const {
     const BlockDataRegion *BR = getBlockRegion();
     if (!BR)
       return 0;
@@ -514,7 +505,11 @@ public:
   }
 
   virtual RuntimeDefinition getRuntimeDefinition() const {
-    return RuntimeDefinition(getBlockDecl());
+    return RuntimeDefinition(getDecl());
+  }
+
+  virtual bool argumentsMayEscape() const {
+    return true;
   }
 
   virtual void getInitialStackFrameContents(const StackFrameContext *CalleeCtx,
@@ -913,6 +908,7 @@ class CallEventManager {
 
   llvm::BumpPtrAllocator &Alloc;
   SmallVector<void *, 8> Cache;
+  typedef SimpleFunctionCall CallEventTemplateTy;
 
   void reclaim(const void *Memory) {
     Cache.push_back(const_cast<void *>(Memory));
@@ -921,24 +917,30 @@ class CallEventManager {
   /// Returns memory that can be initialized as a CallEvent.
   void *allocate() {
     if (Cache.empty())
-      return Alloc.Allocate<FunctionCall>();
+      return Alloc.Allocate<CallEventTemplateTy>();
     else
       return Cache.pop_back_val();
   }
 
   template <typename T, typename Arg>
   T *create(Arg A, ProgramStateRef St, const LocationContext *LCtx) {
+    LLVM_STATIC_ASSERT(sizeof(T) == sizeof(CallEventTemplateTy),
+                       "CallEvent subclasses are not all the same size");
     return new (allocate()) T(A, St, LCtx);
   }
 
   template <typename T, typename Arg1, typename Arg2>
   T *create(Arg1 A1, Arg2 A2, ProgramStateRef St, const LocationContext *LCtx) {
+    LLVM_STATIC_ASSERT(sizeof(T) == sizeof(CallEventTemplateTy),
+                       "CallEvent subclasses are not all the same size");
     return new (allocate()) T(A1, A2, St, LCtx);
   }
 
   template <typename T, typename Arg1, typename Arg2, typename Arg3>
   T *create(Arg1 A1, Arg2 A2, Arg3 A3, ProgramStateRef St,
             const LocationContext *LCtx) {
+    LLVM_STATIC_ASSERT(sizeof(T) == sizeof(CallEventTemplateTy),
+                       "CallEvent subclasses are not all the same size");
     return new (allocate()) T(A1, A2, A3, St, LCtx);
   }
 
@@ -946,6 +948,8 @@ class CallEventManager {
             typename Arg4>
   T *create(Arg1 A1, Arg2 A2, Arg3 A3, Arg4 A4, ProgramStateRef St,
             const LocationContext *LCtx) {
+    LLVM_STATIC_ASSERT(sizeof(T) == sizeof(CallEventTemplateTy),
+                       "CallEvent subclasses are not all the same size");
     return new (allocate()) T(A1, A2, A3, A4, St, LCtx);
   }
 
diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index e0dbc5e583d..2699f7d0d76 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -43,7 +43,6 @@ namespace ento {
 
 class AnalysisManager;
 class CallEvent;
-class SimpleCall;
 class CXXConstructorCall;
 
 class ExprEngine : public SubEngine {
diff --git a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 92297ca3886..1e996589c1b 100644
--- a/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -1907,11 +1907,11 @@ bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModeledExplicitly(
   assert(Call);
   EscapingSymbol = 0;
   
-  // For now, assume that any C++ call can free memory.
+  // For now, assume that any C++ or block call can free memory.
   // TODO: If we want to be more optimistic here, we'll need to make sure that
   // regions escape to C++ containers. They seem to do that even now, but for
   // mysterious reasons.
-  if (!(isa<FunctionCall>(Call) || isa<ObjCMethodCall>(Call)))
+  if (!(isa<SimpleFunctionCall>(Call) || isa<ObjCMethodCall>(Call)))
     return true;
 
   // Check Objective-C messages by selector name.
@@ -1966,7 +1966,7 @@ bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModeledExplicitly(
   }
 
   // At this point the only thing left to handle is straight function calls.
-  const FunctionDecl *FD = cast<FunctionCall>(Call)->getDecl();
+  const FunctionDecl *FD = cast<SimpleFunctionCall>(Call)->getDecl();
   if (!FD)
     return true;
 
diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
index 32f762e1453..4302cc7fd29 100644
--- a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
@@ -860,7 +860,7 @@ void RetainSummaryManager::updateSummaryForCall(const RetainSummary *&S,
     // Special cases where the callback argument CANNOT free the return value.
     // This can generally only happen if we know that the callback will only be
     // called when the return value is already being deallocated.
-    if (const FunctionCall *FC = dyn_cast<FunctionCall>(&Call)) {
+    if (const SimpleFunctionCall *FC = dyn_cast<SimpleFunctionCall>(&Call)) {
       if (IdentifierInfo *Name = FC->getDecl()->getIdentifier()) {
         // When the CGBitmapContext is deallocated, the callback here will free
         // the associated data buffer.
@@ -908,7 +908,7 @@ RetainSummaryManager::getSummary(const CallEvent &Call,
   const RetainSummary *Summ;
   switch (Call.getKind()) {
   case CE_Function:
-    Summ = getFunctionSummary(cast<FunctionCall>(Call).getDecl());
+    Summ = getFunctionSummary(cast<SimpleFunctionCall>(Call).getDecl());
     break;
   case CE_CXXMember:
   case CE_CXXMemberOperator:
diff --git a/lib/StaticAnalyzer/Core/CallEvent.cpp b/lib/StaticAnalyzer/Core/CallEvent.cpp
index 71895051e2c..16bbc86dd48 100644
--- a/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ b/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -387,7 +387,7 @@ bool AnyFunctionCall::argumentsMayEscape() const {
 }
 
 
-const FunctionDecl *SimpleCall::getDecl() const {
+const FunctionDecl *SimpleFunctionCall::getDecl() const {
   const FunctionDecl *D = getOriginExpr()->getDirectCallee();
   if (D)
     return D;
@@ -549,14 +549,14 @@ const BlockDataRegion *BlockCall::getBlockRegion() const {
 }
 
 CallEvent::param_iterator BlockCall::param_begin() const {
-  const BlockDecl *D = getBlockDecl();
+  const BlockDecl *D = getDecl();
   if (!D)
     return 0;
   return D->param_begin();
 }
 
 CallEvent::param_iterator BlockCall::param_end() const {
-  const BlockDecl *D = getBlockDecl();
+  const BlockDecl *D = getDecl();
   if (!D)
     return 0;
   return D->param_end();
@@ -940,7 +940,7 @@ CallEventManager::getSimpleCall(const CallExpr *CE, ProgramStateRef State,
 
   // Otherwise, it's a normal function call, static member function call, or
   // something we can't reason about.
-  return create<FunctionCall>(CE, State, LCtx);
+  return create<SimpleFunctionCall>(CE, State, LCtx);
 }
 
 
-- 
GitLab