[analyzer] Bugfix for autorelease + main run loop leak checker

[george.karpenkov]

Summary

Do not warn when the other message-send-expression is correctly wrapped in a different autorelease pool

[NoQ]

clang/lib/StaticAnalyzer/Checkers/RunLoopAutoreleaseLeakChecker.cpp

126–127 ↗

(On Diff #157708)

Can this logic be moved into the matcher, eg. hasAncestor(autoreleasePoolStmt(unless(equalsBoundNode(AutoreleasePoolBind))))? The good thing here, apart from moving more logic into matchers, would be that we won't be relying on the order in which ancestors are matched.

[george.karpenkov]

clang/lib/StaticAnalyzer/Checkers/RunLoopAutoreleaseLeakChecker.cpp

126–127 ↗

(On Diff #157708)

I'm not sure the relation I need can be encoded with existing matchers =/
(though maybe a new one can be created).

The problem is that there's no way to "fix" the hasAncestor matcher at the closest ancestor of a given type (kind of hasParent), and if it does not find a match, it would move on to further ancestors.

[NoQ]

clang/lib/StaticAnalyzer/Checkers/RunLoopAutoreleaseLeakChecker.cpp

126–127 ↗

(On Diff #157708)

Hmm, okay,

hasAncestor(autoreleasePoolStmt(
  hasAncestor(autoreleasePoolStmt(equalsBoundNode(AutoreleasePoolBind)))))

?

[NoQ]

Though i guess it's O(n²) of a matcher already. I'm fine with both approaches.

lib/StaticAnalyzer/Checkers/RunLoopAutoreleaseLeakChecker.cpp

 using namespace ast_matchers;
 
 namespace {
 
 const char * RunLoopBind = "NSRunLoopM";
 const char * RunLoopRunBind = "RunLoopRunM";
 const char * OtherMsgBind = "OtherMessageSentM";
 const char * AutoreleasePoolBind = "AutoreleasePoolM";
+const char * OtherStmtAutoreleasePoolBind = "OtherAutoreleasePoolM";
 
 class RunLoopAutoreleaseLeakChecker : public Checker<check::ASTCodeBody> {
 
 public:
   void checkASTCodeBody(const Decl *D,
                         AnalysisManager &AM,
                         BugReporter &BR) const;
 
static void emitDiagnostics(BoundNodes &Match,
 
   AnalysisDeclContext *ADC = AM.getAnalysisDeclContext(D);
 
   const auto *ME = Match.getNodeAs<ObjCMessageExpr>(OtherMsgBind);
   assert(ME);
 
   const auto *AP =
       Match.getNodeAs<ObjCAutoreleasePoolStmt>(AutoreleasePoolBind);
+  const auto *OAP =
+      Match.getNodeAs<ObjCAutoreleasePoolStmt>(OtherStmtAutoreleasePoolBind);
   bool HasAutoreleasePool = (AP != nullptr);
 
   const auto *RL = Match.getNodeAs<ObjCMessageExpr>(RunLoopBind);
   const auto *RLR = Match.getNodeAs<Stmt>(RunLoopRunBind);
   assert(RLR && "Run loop launch not found");
-
   assert(ME != RLR);
-  if (HasAutoreleasePool && seenBefore(AP, RLR, ME))
+
+  // Launch of run loop occurs before the message-sent expression is seen.
+  if (seenBefore(DeclBody, RLR, ME))
     return;
 
-  if (!HasAutoreleasePool && seenBefore(DeclBody, RLR, ME))
+  if (HasAutoreleasePool && (OAP != AP))
     return;
 
   PathDiagnosticLocation Location = PathDiagnosticLocation::createBegin(
     ME, BR.getSourceManager(), ADC);
   SourceRange Range = ME->getSourceRange();
 
   BR.EmitBasicReport(ADC->getDecl(), Checker,
                      /*Name=*/"Memory leak inside autorelease pool",
return objcMessageExpr(unless(anyOf(equalsBoundNode(RunLoopBind),
                          Extra)
       .bind(OtherMsgBind);
 }
 
 static void
 checkTempObjectsInSamePool(const Decl *D, AnalysisManager &AM, BugReporter &BR,
                            const RunLoopAutoreleaseLeakChecker *Chkr) {
   StatementMatcher RunLoopRunM = getRunLoopRunM();
-  StatementMatcher OtherMessageSentM = getOtherMessageSentM();
+  StatementMatcher OtherMessageSentM = getOtherMessageSentM(
+    hasAncestor(autoreleasePoolStmt().bind(OtherStmtAutoreleasePoolBind)));
 
   StatementMatcher RunLoopInAutorelease =
       autoreleasePoolStmt(
         hasDescendant(RunLoopRunM),
         hasDescendant(OtherMessageSentM)).bind(AutoreleasePoolBind);
 
   DeclarationMatcher GroupM = decl(hasDescendant(RunLoopInAutorelease));
 

test/Analysis/Checkers/RunLoopAutoreleaseLeakChecker.m

 void runloop_init_before() { // Warning: object created before the loop.
   @autoreleasepool {
     NSObject *object = [[NSObject alloc] init]; // expected-warning{{Temporary objects allocated in the autorelease pool followed by the launch of main run loop may never get released; consider moving them to a separate autorelease pool}}
     (void) object;
     [[NSRunLoop mainRunLoop] run]; 
   }
 }
 
+void runloop_init_before_separate_pool() { // No warning: separate autorelease pool.
+  @autoreleasepool {
+    NSObject *object;
+    @autoreleasepool {
+      object = [[NSObject alloc] init]; // no-warning
+    }
+    (void) object;
+    [[NSRunLoop mainRunLoop] run]; 
+  }
+}
+
 void xpcmain_init_before() { // Warning: object created before the loop.
   @autoreleasepool {
     NSObject *object = [[NSObject alloc] init]; // expected-warning{{Temporary objects allocated in the autorelease pool followed by the launch of xpc_main may never get released; consider moving them to a separate autorelease pool}}
     (void) object;
     xpc_main(); 
   }
 }