[analyzer] Change inlining policy to inline small functions when reanalyzing ObjC methods as top level.

This allows us to better reason about(inline) small wrapper functions.

llvm-svn: 178063

clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h

@@ -48,12 +48,12 @@ class CXXConstructorCall;
 
 class ExprEngine : public SubEngine {
 public:
-  /// The modes of inlining.
+  /// The modes of inlining, which override the default analysis-wide settings.
   enum InliningModes {
-    /// Do not inline any of the callees.
-    Inline_None = 0,
-    /// Inline all callees.
-    Inline_All = 0x1
+    /// Follow the default settings for inlining callees.
+    Inline_Regular = 0,
+    /// Do minimal inlining of callees.
+    Inline_Minimal = 0x1
   };
 
 private:

clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

@@ -683,7 +683,7 @@ bool ExprEngine::shouldInlineCall(const CallEvent &Call, const Decl *D,
   if (CalleeADC->isBodyAutosynthesized())
     return true;
 
-  if (HowToInline == Inline_None)
+  if (!AMgr.shouldInlineCall())
     return false;
 
   // Check if we should inline a call based on its kind.
@@ -745,6 +745,12 @@ bool ExprEngine::shouldInlineCall(const CallEvent &Call, const Decl *D,
     NumReachedInlineCountMax++;
     return false;
   }
+
+  if (HowToInline == Inline_Minimal &&
+      (CalleeCFG->getNumBlockIDs() > Opts.getAlwaysInlineSize()
+      || IsRecursive))
+    return false;
+
   Engine.FunctionSummaries->bumpNumTimesInlined(D);
 
   return true;

clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

@@ -234,11 +234,11 @@ public:
       else if (Mode == AM_Path) {
         llvm::errs() << " (Path, ";
         switch (IMode) {
-          case ExprEngine::Inline_None:
-            llvm::errs() << " Inline_None";
+          case ExprEngine::Inline_Minimal:
+            llvm::errs() << " Inline_Minimal";
             break;
-          case ExprEngine::Inline_All:
-            llvm::errs() << " Inline_All";
+          case ExprEngine::Inline_Regular:
+            llvm::errs() << " Inline_Regular";
             break;
         }
         llvm::errs() << ")";
@@ -284,7 +284,8 @@ public:
   virtual void HandleTranslationUnit(ASTContext &C);
 
   /// \brief Determine which inlining mode should be used when this function is
-  /// analyzed. For example, determines if the callees should be inlined.
+  /// analyzed. This allows to redefine the default inlining policies when
+  /// analyzing a given function.
   ExprEngine::InliningModes
   getInliningModeForFunction(const Decl *D, SetOfConstDecls Visited);
 
@@ -299,7 +300,7 @@ public:
   /// set of functions which should be considered analyzed after analyzing the
   /// given root function.
   void HandleCode(Decl *D, AnalysisMode Mode,
-                  ExprEngine::InliningModes IMode = ExprEngine::Inline_None,
+                  ExprEngine::InliningModes IMode = ExprEngine::Inline_Minimal,
                   SetOfConstDecls *VisitedCallees = 0);
 
   void RunPathSensitiveChecks(Decl *D,
@@ -410,22 +411,18 @@ static bool shouldSkipFunction(const Decl *D,
 ExprEngine::InliningModes
 AnalysisConsumer::getInliningModeForFunction(const Decl *D,
                                              SetOfConstDecls Visited) {
-  ExprEngine::InliningModes HowToInline =
-      (Mgr->shouldInlineCall()) ? ExprEngine::Inline_All :
-                                  ExprEngine::Inline_None;
-
   // We want to reanalyze all ObjC methods as top level to report Retain
-  // Count naming convention errors more aggressively. But we can turn off
+  // Count naming convention errors more aggressively. But we should tune down
   // inlining when reanalyzing an already inlined function.
   if (Visited.count(D)) {
     assert(isa<ObjCMethodDecl>(D) &&
            "We are only reanalyzing ObjCMethods.");
     const ObjCMethodDecl *ObjCM = cast<ObjCMethodDecl>(D);
     if (ObjCM->getMethodFamily() != OMF_init)
-      HowToInline = ExprEngine::Inline_None;
+      return ExprEngine::Inline_Minimal;
   }
 
-  return HowToInline;
+  return ExprEngine::Inline_Regular;
 }
 
 void AnalysisConsumer::HandleDeclsCallGraph(const unsigned LocalTUDeclsSize) {
@@ -595,7 +592,7 @@ void AnalysisConsumer::HandleCode(Decl *D, AnalysisMode Mode,
     checkerMgr->runCheckersOnASTBody(D, *Mgr, BR);
   if ((Mode & AM_Path) && checkerMgr->hasPathSensitiveCheckers()) {
     RunPathSensitiveChecks(D, IMode, VisitedCallees);
-    if (IMode != ExprEngine::Inline_None)
+    if (IMode != ExprEngine::Inline_Minimal)
       NumFunctionsAnalyzed++;
   }
 }

clang/test/Analysis/retain-release-inline.m

@@ -361,3 +361,30 @@ CFStringRef testCovariantReturnType() {
   }
   return Str;
 }
+
+// Test that we reanalyze ObjC methods which have been inlined. When reanalyzing
+// them, make sure we inline very small functions.
+
+@interface MyClass : NSObject
+- (id)test_return_retained_NS;
+- (void)test_return_retained;
+@end
+
+id returnInputParam(id x) {
+  return x;
+}
+@implementation MyClass
+- (id)test_return_retained_NS {
+  // This method does not follow naming conventions, so a warning will be
+  // reported when it is reanalyzed at top level.
+  return returnInputParam([[NSString alloc] init]); // expected-warning {{leak}}
+}
+
+- (void)test_return_retained {
+  id x = test_return_retained_NS(); // expected-warning {{leak}}
+  [x retain];
+  [x release];
+}
+
+@end
+