[analyzer] Make it possible to analyze functions even if they were inlined

[a.sidorin]

Summary

The analyzer uses heuristic that assumes that functions that were analyzed in some context will not be called out of some context. While it helps to suppress false positives, it also may lead to loss of some TPs. So, I think it is a good idea to allow user enabling of analysis of all functions. This behaviour is enabled via a specific option, default behaviour remains unchanged.

[jan]

Thanks. This is exactly what I needed. Very useful.
(If approved, needs a description in IPA.txt)

[zaks.anna]

Thanks! I'll take a look.

Please, include context in the patch as described in http://llvm.org/docs/Phabricator.html.

[zaks.anna]

We already do something similar for ObjC methods by default (we re-analyze them, but in a more shallow mode) but decided not to reanalyze the other functions as top level. Do you have any data on how many more issues are found with this option on on real codebases?

The better option name (and term to use) is "analyze externally visible functions as top-level" because they are analyzed but just not in all possible contexts or not as top-level.

lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

444

Could you wrap this in a helper just like the rest of the options are done in AnalyzerOptions.cpp?

[zaks.anna]

Ping?

lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

 }
 
 static bool shouldSkipFunction(const Decl *D,
+                               AnalyzerOptionsRef Opts,
                                const SetOfConstDecls &Visited,
                                const SetOfConstDecls &VisitedAsTopLevel) {
   if (VisitedAsTopLevel.count(D))

       return false;
   }
 
-  // Otherwise, if we visited the function before, do not reanalyze it.
-  return Visited.count(D);
+  // Otherwise, if we visited the function before, do not reanalyze it
+  // unless we don't want to analyze all functions.
+  if (Opts->getBooleanOption("skip-inlined-functions", true))
+    return Visited.count(D);
+
+  // If "skip-inlined-functions" option was set to false,
+  // we want to reanalyze every function that can be visible externally
+  // (but we exclude C static functions and private methods).
+  if (D->getAccess() == AS_private)
+    return true;
+
+  const FunctionDecl *FD = dyn_cast<FunctionDecl>(D);
+  if (FD && !FD->hasExternalFormalLinkage())
+    return true;
+
+  return false;
 }
 
 ExprEngine::InliningModes

 
     // Skip the functions which have been processed already or previously
     // inlined.
-    if (shouldSkipFunction(D, Visited, VisitedAsTopLevel))
+    if (shouldSkipFunction(D, Opts, Visited, VisitedAsTopLevel))
       continue;
 
     // Analyze the function.

test/Analysis/analyzer-config.c

 // CHECK-NEXT: min-cfg-size-treat-functions-as-large = 14
 // CHECK-NEXT: mode = deep
 // CHECK-NEXT: region-store-small-struct-limit = 2
+// CHECK-NEXT: skip-inlined-functions = true
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 15
+// CHECK-NEXT: num-entries = 16
 

test/Analysis/analyzer-config.cpp

 // CHECK-NEXT: min-cfg-size-treat-functions-as-large = 14
 // CHECK-NEXT: mode = deep
 // CHECK-NEXT: region-store-small-struct-limit = 2
+// CHECK-NEXT: skip-inlined-functions = true
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 20
+// CHECK-NEXT: num-entries = 21

test/Analysis/inlining/analyze-all-visible.cpp

+// RUN: %clang_cc1 -analyze -analyzer-checker=debug.ExprInspection -analyzer-config skip-inlined-functions=false -analyzer-display-progress %s 2>&1 | FileCheck %s
+
+namespace c_like {
+
+// static functions are not analyzed again if they are inlined
+static int static_func() {
+  return 0;
+}
+
+// We analyze callee() again despite the fact it was inlined
+// because it is externally visible
+int callee(int arg) {
+  if (arg == 1)
+    return 1; // expected-warning{{REACHABLE}}
+  return 0;   // expected-warning{{REACHABLE}}
+}
+
+void caller() {
+  callee(1);
+  static_func();
+}
+
+}
+
+namespace private_methods {
+
+class Test {
+public:
+  void test() {
+    callee(1);
+  }
+private:
+  // We don't analyze private methods if they were inlined
+  int callee(int arg) {
+    if (arg == 1)
+      return 1; // expected-warning{{REACHABLE}}
+    return 0;   // no-warning
+  }
+};
+
+}
+
+// CHECK: analyze-all-visible.cpp c_like::static_func()
+// CHECK-NEXT: analyze-all-visible.cpp c_like::callee(int)
+// CHECK-NEXT: analyze-all-visible.cpp c_like::caller()
+// CHECK-NEXT: analyze-all-visible.cpp private_methods::Test::test()
+// CHECK-NEXT: analyze-all-visible.cpp private_methods::Test::callee(int)
+// CHECK-NEXT: analyze-all-visible.cpp private_methods::Test::test()
+// CHECK-NEXT: analyze-all-visible.cpp c_like::caller()
+// CHECK-NEXT: analyze-all-visible.cpp c_like::callee(int)