Add -Winfinite-recursion to Clang

This new warning detects when a function will recursively call itself on every
code path though that function.  This catches simple recursive cases such as:

void foo() {
  foo();
}

As well as more complex functions like:

void bar() {
  if (test()) {
    bar();
    return;
  } else {
    bar();
  }
  return;
}

This warning uses the CFG.  As with other CFG-based warnings, this is off
by default.  Due to false positives, this warning is also disabled for
templated functions.

llvm-svn: 197853

clang/include/clang/Basic/DiagnosticGroups.td

@@ -169,6 +169,7 @@ def OverloadedShiftOpParentheses: DiagGroup<"overloaded-shift-op-parentheses">;
 def DanglingElse: DiagGroup<"dangling-else">;
 def DanglingField : DiagGroup<"dangling-field">;
 def DistributedObjectModifiers : DiagGroup<"distributed-object-modifiers">;
+def InfiniteRecursion : DiagGroup<"infinite-recursion">;
 def GNUImaginaryConstant : DiagGroup<"gnu-imaginary-constant">;
 def IgnoredQualifiers : DiagGroup<"ignored-qualifiers">;
 def : DiagGroup<"import">;

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -30,6 +30,10 @@ def warn_duplicate_enum_values : Warning<
   "been assigned">, InGroup<DiagGroup<"duplicate-enum">>, DefaultIgnore;
 def note_duplicate_element : Note<"element %0 also has value %1">;
 
+def warn_infinite_recursive_function : Warning<
+  "all paths through this function will call itself">,
+  InGroup<InfiniteRecursion>, DefaultIgnore;
+
 // Constant expressions
 def err_expr_not_ice : Error<
   "expression is not an %select{integer|integral}0 constant expression">;

clang/lib/Sema/AnalysisBasedWarnings.cpp

@@ -77,6 +77,95 @@ static void CheckUnreachable(Sema &S, AnalysisDeclContext &AC) {
   reachable_code::FindUnreachableCode(AC, UC);
 }
 
+//===----------------------------------------------------------------------===//
+// Check for infinite self-recursion in functions
+//===----------------------------------------------------------------------===//
+
+// All blocks are in one of three states.  States are ordered so that blocks
+// can only move to higher states.
+enum RecursiveState {
+  FoundNoPath,
+  FoundPath,
+  FoundPathWithNoRecursiveCall
+};
+
+static void checkForFunctionCall(Sema &S, const FunctionDecl *FD,
+                                 CFGBlock &Block, unsigned ExitID,
+                                 llvm::SmallVectorImpl<RecursiveState> &States,
+                                 RecursiveState State) {
+  unsigned ID = Block.getBlockID();
+
+  // A block's state can only move to a higher state.
+  if (States[ID] >= State)
+    return;
+
+  States[ID] = State;
+
+  // Found a path to the exit node without a recursive call.
+  if (ID == ExitID && State == FoundPathWithNoRecursiveCall)
+    return;
+
+  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 (CFGBlock::iterator I = Block.begin(), E = Block.end(); I != E; ++I) {
+      if (I->getKind() != CFGElement::Statement)
+        continue;
+
+      const CallExpr *CE = dyn_cast<CallExpr>(I->getAs<CFGStmt>()->getStmt());
+      if (CE && CE->getCalleeDecl() &&
+          CE->getCalleeDecl()->getCanonicalDecl() == FD) {
+        if (const CXXMemberCallExpr *MCE = dyn_cast<CXXMemberCallExpr>(CE)) {
+          if (isa<CXXThisExpr>(MCE->getImplicitObjectArgument()) ||
+              !MCE->getMethodDecl()->isVirtual()) {
+            State = 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);
+}
+
+static void checkRecursiveFunction(Sema &S, const FunctionDecl *FD,
+                                   const Stmt *Body,
+                                   AnalysisDeclContext &AC) {
+  FD = FD->getCanonicalDecl();
+
+  // Only run on non-templated functions and non-templated members of
+  // templated classes.
+  if (FD->getTemplatedKind() != FunctionDecl::TK_NonTemplate &&
+      FD->getTemplatedKind() != FunctionDecl::TK_MemberSpecialization)
+    return;
+
+  CFG *cfg = AC.getCFG();
+  if (cfg == 0) return;
+
+  // If the exit block is unreachable, skip processing the function.
+  if (cfg->getExit().pred_empty())
+    return;
+
+  // Mark all nodes as FoundNoPath, then begin processing the entry block.
+  llvm::SmallVector<RecursiveState, 16> states(cfg->getNumBlockIDs(),
+                                               FoundNoPath);
+  checkForFunctionCall(S, FD, cfg->getEntry(), cfg->getExit().getBlockID(),
+                       states, FoundPathWithNoRecursiveCall);
+
+  // Check that the exit block is reachable.  This prevents triggering the
+  // warning on functions that do not terminate.
+  if (states[cfg->getExit().getBlockID()] == FoundPath)
+    S.Diag(Body->getLocStart(), diag::warn_infinite_recursive_function);
+}
+
 //===----------------------------------------------------------------------===//
 // Check for missing return value.
 //===----------------------------------------------------------------------===//
@@ -1789,6 +1878,16 @@ AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P,
                                D->getLocStart()) != DiagnosticsEngine::Ignored)
     diagnoseRepeatedUseOfWeak(S, fscope, D, AC.getParentMap());
 
+
+  // Check for infinite self-recursion in functions
+  if (Diags.getDiagnosticLevel(diag::warn_infinite_recursive_function,
+                               D->getLocStart())
+      != DiagnosticsEngine::Ignored) {
+    if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
+      checkRecursiveFunction(S, FD, Body, AC);
+    }
+  }
+
   // Collect statistics about the CFG if it was built.
   if (S.CollectStats && AC.isCFGBuilt()) {
     ++NumFunctionsAnalyzed;

clang/test/SemaCXX/warn-infinite-recursion.cpp

@@ -0,0 +1,129 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify -Winfinite-recursion
+
+void a() {  // expected-warning{{call itself}}
+  a();
+}
+
+void b(int x) {  // expected-warning{{call itself}}
+  if (x)
+    b(x);
+  else
+    b(x+1);
+}
+
+void c(int x) {
+  if (x)
+    c(5);
+}
+
+void d(int x) {  // expected-warning{{call itself}}
+  if (x)
+    ++x;
+  return d(x);
+}
+
+// Doesn't warn on mutually recursive functions
+void e();
+void f();
+
+void e() { f(); }
+void f() { e(); }
+
+// Don't warn on infinite loops
+void g() {
+  while (true)
+    g();
+
+  g();
+}
+
+void h(int x) {
+  while (x < 5) {
+    h(x+1);
+  }
+}
+
+void i(int x) {  // expected-warning{{call itself}}
+  while (x < 5) {
+    --x;
+  }
+  i(0);
+}
+
+int j() {  // expected-warning{{call itself}}
+  return 5 + j();
+}
+
+class S {
+  static void a();
+  void b();
+};
+
+void S::a() {  // expected-warning{{call itself}}
+  return a();
+}
+
+void S::b() {  // expected-warning{{call itself}}
+  int i = 0;
+  do {
+    ++i;
+    b();
+  } while (i > 5);
+}
+
+template<class member>
+struct T {
+  member m;
+  void a() { return a(); }  // expected-warning{{call itself}}
+  static void b() { return b(); }  // expected-warning{{call itself}}
+};
+
+void test_T() {
+  T<int> foo;
+  foo.a();  // expected-note{{in instantiation}}
+  foo.b();  // expected-note{{in instantiation}}
+}
+
+class U {
+  U* u;
+  void Fun() {  // expected-warning{{call itself}}
+    u->Fun();
+  }
+};
+
+// No warnings on templated functions
+// sum<0>() is instantiated, does recursively call itself, but never runs.
+template <int value>
+int sum() {
+  return value + sum<value/2>();
+}
+
+template<>
+int sum<1>() { return 1; }
+
+template<int x, int y>
+int calculate_value() {
+  if (x != y)
+    return sum<x - y>();  // This instantiates sum<0>() even if never called.
+  else
+    return 0;
+}
+
+int value = calculate_value<1,1>();
+
+void DoSomethingHere();
+
+// DoStuff<0,0>() is instantiated, but never called.
+template<int First, int Last>
+int DoStuff() {
+  if (First + 1 == Last) {
+    // This branch gets removed during <0, 0> instantiation in so CFG for this
+    // function goes straight to the else branch.
+    DoSomethingHere();
+  } else {
+    DoStuff<First, (First + Last)/2>();
+    DoStuff<(First + Last)/2, Last>();
+  }
+  return 0;
+}
+int stuff = DoStuff<0, 1>();