[PATCH] Sema: Eliminate recursion from checkForFunctionCall()

Vedant Kumar vsk at apple.com
Wed Jul 22 13:34:41 PDT 2015


Large CFGs cause clang::Sema::checkForFunctionCall() to stack overflow.

I have a patch which breaks the recursion in checkForFunctionCall() by manually
managing the call stack. This fixes the problem. The check-clang target reports
no regressions.

I don't have commit rights, so I'd appreciate someone taking a look at the
patch and getting it into trunk :).

thanks!
vedant

================================================================================

diff --git a/lib/Sema/AnalysisBasedWarnings.cpp b/lib/Sema/AnalysisBasedWarnings.cpp
index f2ff48a..dc6dda0 100644
--- a/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/lib/Sema/AnalysisBasedWarnings.cpp
@@ -52,6 +52,8 @@
#include <deque>
#include <iterator>
#include <vector>
+#include <functional>
+#include <utility>

using namespace clang;

@@ -175,59 +177,69 @@ static void checkForFunctionCall(Sema &S, const FunctionDecl *FD,
                                 CFGBlock &Block, unsigned ExitID,
                                 llvm::SmallVectorImpl<RecursiveState> &States,
                                 RecursiveState State) {
-  unsigned ID = Block.getBlockID();
+  std::vector<std::pair<std::reference_wrapper<CFGBlock>, RecursiveState>>
+      Stack;
+  Stack.emplace_back(std::make_pair(std::ref(Block), State));

-  // A block's state can only move to a higher state.
-  if (States[ID] >= State)
-    return;
+  while (!Stack.empty()) {
+    CFGBlock &CurBlock = Stack.back().first.get();
+    RecursiveState CurState = Stack.back().second;
+    Stack.pop_back();

-  States[ID] = State;
+    unsigned ID = CurBlock.getBlockID();

-  // Found a path to the exit node without a recursive call.
-  if (ID == ExitID && State == FoundPathWithNoRecursiveCall)
-    return;
+    // A block's state can only move to a higher state.
+    if (States[ID] >= CurState)
+      continue;

-  if (State == FoundPathWithNoRecursiveCall) {
-    // If the current state is FoundPathWithNoRecursiveCall, the successors
-    // will be either FoundPathWithNoRecursiveCall or FoundPath.  To determine
-    // which, process all the Stmt's in this block to find any recursive calls.
-    for (const auto &B : Block) {
-      if (B.getKind() != CFGElement::Statement)
-        continue;
+    States[ID] = CurState;
+
+    // Found a path to the exit node without a recursive call.
+    if (ID == ExitID && CurState == FoundPathWithNoRecursiveCall)
+      continue;
+
+    if (CurState == FoundPathWithNoRecursiveCall) {
+      // If the current state is FoundPathWithNoRecursiveCall, the successors
+      // will be either FoundPathWithNoRecursiveCall or FoundPath.  To determine
+      // which, process all the Stmt's in this block to find any recursive
+      // calls.
+      for (const auto &B : CurBlock) {
+        if (B.getKind() != CFGElement::Statement)
+          continue;

-      const CallExpr *CE = dyn_cast<CallExpr>(B.getAs<CFGStmt>()->getStmt());
-      if (CE && CE->getCalleeDecl() &&
-          CE->getCalleeDecl()->getCanonicalDecl() == FD) {
-
-        // Skip function calls which are qualified with a templated class.
-        if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(
-                CE->getCallee()->IgnoreParenImpCasts())) {
-          if (NestedNameSpecifier *NNS = DRE->getQualifier()) {
-            if (NNS->getKind() == NestedNameSpecifier::TypeSpec &&
-                isa<TemplateSpecializationType>(NNS->getAsType())) {
-               continue;
+        const CallExpr *CE = dyn_cast<CallExpr>(B.getAs<CFGStmt>()->getStmt());
+        if (CE && CE->getCalleeDecl() &&
+            CE->getCalleeDecl()->getCanonicalDecl() == FD) {
+
+          // Skip function calls which are qualified with a templated class.
+          if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(
+                  CE->getCallee()->IgnoreParenImpCasts())) {
+            if (NestedNameSpecifier *NNS = DRE->getQualifier()) {
+              if (NNS->getKind() == NestedNameSpecifier::TypeSpec &&
+                  isa<TemplateSpecializationType>(NNS->getAsType())) {
+                continue;
+              }
            }
          }
-        }

-        if (const CXXMemberCallExpr *MCE = dyn_cast<CXXMemberCallExpr>(CE)) {
-          if (isa<CXXThisExpr>(MCE->getImplicitObjectArgument()) ||
-              !MCE->getMethodDecl()->isVirtual()) {
-            State = FoundPath;
+          if (const CXXMemberCallExpr *MCE = dyn_cast<CXXMemberCallExpr>(CE)) {
+            if (isa<CXXThisExpr>(MCE->getImplicitObjectArgument()) ||
+                !MCE->getMethodDecl()->isVirtual()) {
+              CurState = FoundPath;
+              break;
+            }
+          } else {
+            CurState = FoundPath;
            break;
          }
-        } else {
-          State = FoundPath;
-          break;
        }
      }
    }
-  }

-  for (CFGBlock::succ_iterator I = Block.succ_begin(), E = Block.succ_end();
-       I != E; ++I)
-    if (*I)
-      checkForFunctionCall(S, FD, **I, ExitID, States, State);
+    for (auto I = CurBlock.succ_rbegin(), E = CurBlock.succ_rend(); I != E; ++I)
+      if (*I)
+        Stack.emplace_back(std::make_pair(std::ref(**I), CurState));
+  }
}

static void checkRecursiveFunction(Sema &S, const FunctionDecl *FD,



More information about the cfe-commits mailing list