[PATCH] D100626: [analyzer][NFC] Remove duplicated work from retain count leak report

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko created this revision.
vsavchenko added a reviewer: NoQ.
Herald added subscribers: steakhal, ASDenysPetrov, martong, Charusso, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, kristof.beyls, xazax.hun.
vsavchenko requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Allocation site is the key location for the leak checker.  It is a
uniqueing location for the report and a source of information for
the warning's message.

Before this patch, we calculated and used it twice in bug report and
in bug report visitor.  Such duplication is not only harmful
performance-wise (not much, but still), but also design-wise.  Because
changing something about the end piece of the report should've been
repeated for description as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100626

Files:
  clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp


Index: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
@@ -337,11 +337,15 @@
 
 class RefLeakReportVisitor : public RefCountReportVisitor {
 public:
-  RefLeakReportVisitor(SymbolRef sym) : RefCountReportVisitor(sym) {}
+  RefLeakReportVisitor(SymbolRef Sym, const MemRegion *FirstBinding)
+      : RefCountReportVisitor(Sym), FirstBinding(FirstBinding) {}
 
   PathDiagnosticPieceRef getEndPath(BugReporterContext &BRC,
                                     const ExplodedNode *N,
                                     PathSensitiveBugReport &BR) override;
+
+private:
+  const MemRegion *FirstBinding;
 };
 
 } // end namespace retaincountchecker
@@ -729,14 +733,6 @@
   // assigned to different variables, etc.
   BR.markInteresting(Sym);
 
-  // We are reporting a leak.  Walk up the graph to get to the first node where
-  // the symbol appeared, and also get the first VarDecl that tracked object
-  // is stored to.
-  AllocationInfo AllocI = GetAllocationSite(BRC.getStateManager(), EndN, Sym);
-
-  const MemRegion* FirstBinding = AllocI.R;
-  BR.markInteresting(AllocI.InterestingMethodContext);
-
   PathDiagnosticLocation L = cast<RefLeakReport>(BR).getEndOfPath();
 
   std::string sbuf;
@@ -902,15 +898,15 @@
 }
 
 RefLeakReport::RefLeakReport(const RefCountBug &D, const LangOptions &LOpts,
-                             ExplodedNode *n, SymbolRef sym,
+                             ExplodedNode *N, SymbolRef Sym,
                              CheckerContext &Ctx)
-    : RefCountReport(D, LOpts, n, sym, /*isLeak=*/true) {
+    : RefCountReport(D, LOpts, N, Sym, /*isLeak=*/true) {
 
-  deriveAllocLocation(Ctx, sym);
+  deriveAllocLocation(Ctx, Sym);
   if (!AllocBinding)
-    deriveParamLocation(Ctx, sym);
+    deriveParamLocation(Ctx, Sym);
 
   createDescription(Ctx);
 
-  addVisitor(std::make_unique<RefLeakReportVisitor>(sym));
+  addVisitor(std::make_unique<RefLeakReportVisitor>(Sym, AllocBinding));
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D100626.338022.patch
Type: text/x-patch
Size: 2197 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210416/7a0740fa/attachment.bin>