[PATCH] Sema: Eliminate recursion from checkForFunctionCall()

[Richard Trieu]

On Wed, Jul 22, 2015 at 1:34 PM, Vedant Kumar <vsk at apple.com> wrote:

> Large CFGs cause clang::Sema::checkForFunctionCall() to stack overflow.
>
Is the code that produces the large CFG available?  It may be useful to
test that other users of the CFG can cope with processing the code.

>
> 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,
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150722/185825a8/attachment.html>