[PATCH] Add new warning to Clang to detect when all code paths in a function has a call back to the function.

Richard Smith richard at metafoo.co.uk
Tue Dec 17 18:25:42 PST 2013


  LGTM with minor tweaks.

  I'm still not sure we've got the function template specialization case right, but I think this adds most of the value as-is, and I think you're right to blacklist an area with known false positives over not having the warning at all.


================
Comment at: lib/Lex/Pragma.cpp:934-936
@@ -929,4 +933,5 @@
+#endif
   void DebugOverflowStack() {
     DebugOverflowStack();
   }
 #ifdef _MSC_VER
----------------
Somewhat unrelated to your patch, but this looks broken (at `-O1` or above). How about replacing this with:

  static void DebugOverflowStack() {
    void (*volatile Self)() = DebugOverflowStack;
    self();
  }

That should both suppress your warning (and presumably the MSVC warning) and make this function work reliably.

================
Comment at: lib/Sema/AnalysisBasedWarnings.cpp:90-93
@@ +89,6 @@
+
+static void CheckForFunctionCall(Sema &S, const FunctionDecl *FD,
+                                 CFGBlock& Block, unsigned exitID,
+                                 llvm::SmallVectorImpl<RecursiveState> &states,
+                                 RecursiveState state) {
+  unsigned ID = Block.getBlockID();
----------------
For new code, please follow the coding style's formatting rules (`checkForFunctionCall`, `States`, `State`, `ExitID`, and `" &"` not `"& "`).

================
Comment at: lib/Sema/AnalysisBasedWarnings.cpp:96-99
@@ +95,6 @@
+
+  if (states[ID] < state)
+    states[ID] = state;
+  else
+    return;
+
----------------
Maybe

  if (States[ID] >= State)
    return;

  States[ID] = State;
  // ...


http://llvm-reviews.chandlerc.com/D1864



More information about the cfe-commits mailing list