llvm-cov: Simplify coverage reports, fixing PR22575 in the process

PR22575 occurred because we were unsafely storing references into a
std::vector. If the vector moved because it grew, we'd be left
iterating through garbage memory. This avoids the issue by simplifying
the logic to gather coverage information as we go, rather than storing
it and iterating over it.

I'm relying on the existing tests showing that this is semantically
NFC, since it's difficult to hit the issue this fixes without
relatively large covered programs.

llvm-svn: 229215

tools/llvm-cov/CMakeLists.txt

@@ -6,7 +6,6 @@ add_llvm_tool(llvm-cov
   CodeCoverage.cpp
   CoverageFilters.cpp
   CoverageReport.cpp
-  CoverageSummary.cpp
   CoverageSummaryInfo.cpp
   SourceCoverageView.cpp
   TestingSupport.cpp

tools/llvm-cov/CodeCoverage.cpp

@@ -16,7 +16,6 @@
 #include "RenderingSupport.h"
 #include "CoverageFilters.h"
 #include "CoverageReport.h"
-#include "CoverageSummary.h"
 #include "CoverageViewOptions.h"
 #include "SourceCoverageView.h"
 #include "llvm/ADT/SmallString.h"
@@ -459,9 +458,7 @@ int CodeCoverageTool::report(int argc, const char **argv,
   if (!Coverage)
     return 1;
 
-  CoverageSummary Summarizer;
-  Summarizer.createSummaries(*Coverage);
-  CoverageReport Report(ViewOpts, Summarizer);
+  CoverageReport Report(ViewOpts, std::move(Coverage));
   if (SourceFiles.empty() && Filters.empty()) {
     Report.renderFileReports(llvm::outs());
     return 0;

tools/llvm-cov/CoverageReport.cpp

@@ -12,7 +12,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "CoverageReport.h"
-#include "CoverageSummary.h"
 #include "RenderingSupport.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Format.h"
@@ -158,12 +157,12 @@ void CoverageReport::render(const FunctionCoverageSummary &Function,
 
 void CoverageReport::renderFunctionReports(raw_ostream &OS) {
   bool isFirst = true;
-  for (const auto &File : Summary.getFileSummaries()) {
+  for (StringRef Filename : Coverage->getUniqueSourceFiles()) {
     if (isFirst)
       isFirst = false;
     else
       OS << "\n";
-    OS << "File '" << File.Name << "':\n";
+    OS << "File '" << Filename << "':\n";
     OS << column("Name", FunctionReportColumns[0])
        << column("Regions", FunctionReportColumns[1], Column::RightAlignment)
        << column("Miss", FunctionReportColumns[2], Column::RightAlignment)
@@ -174,13 +173,19 @@ void CoverageReport::renderFunctionReports(raw_ostream &OS) {
     OS << "\n";
     renderDivider(FunctionReportColumns, OS);
     OS << "\n";
-    for (const auto &Function : File.FunctionSummaries)
+    FunctionCoverageSummary Totals("TOTAL");
+    for (const auto &F : Coverage->getCoveredFunctions(Filename)) {
+      FunctionCoverageSummary Function = FunctionCoverageSummary::get(F);
+      ++Totals.ExecutionCount;
+      Totals.RegionCoverage += Function.RegionCoverage;
+      Totals.LineCoverage += Function.LineCoverage;
       render(Function, OS);
-    renderDivider(FunctionReportColumns, OS);
-    OS << "\n";
-    render(FunctionCoverageSummary("TOTAL", /*ExecutionCount=*/0,
-                                   File.RegionCoverage, File.LineCoverage),
-           OS);
+    }
+    if (Totals.ExecutionCount) {
+      renderDivider(FunctionReportColumns, OS);
+      OS << "\n";
+      render(Totals, OS);
+    }
   }
 }
 
@@ -194,9 +199,17 @@ void CoverageReport::renderFileReports(raw_ostream &OS) {
      << "\n";
   renderDivider(FileReportColumns, OS);
   OS << "\n";
-  for (const auto &File : Summary.getFileSummaries())
-    render(File, OS);
+  FileCoverageSummary Totals("TOTAL");
+  for (StringRef Filename : Coverage->getUniqueSourceFiles()) {
+    FileCoverageSummary Summary(Filename);
+    for (const auto &F : Coverage->getCoveredFunctions(Filename)) {
+      FunctionCoverageSummary Function = FunctionCoverageSummary::get(F);
+      Summary.addFunction(Function);
+      Totals.addFunction(Function);
+    }
+    render(Summary, OS);
+  }
   renderDivider(FileReportColumns, OS);
   OS << "\n";
-  render(Summary.getCombinedFileSummaries(), OS);
+  render(Totals, OS);
 }

tools/llvm-cov/CoverageReport.h

@@ -14,7 +14,7 @@
 #ifndef LLVM_COV_COVERAGEREPORT_H
 #define LLVM_COV_COVERAGEREPORT_H
 
-#include "CoverageSummary.h"
+#include "CoverageSummaryInfo.h"
 #include "CoverageViewOptions.h"
 
 namespace llvm {
@@ -22,14 +22,15 @@ namespace llvm {
 /// \brief Displays the code coverage report.
 class CoverageReport {
   const CoverageViewOptions &Options;
-  CoverageSummary &Summary;
+  std::unique_ptr<coverage::CoverageMapping> Coverage;
 
   void render(const FileCoverageSummary &File, raw_ostream &OS);
   void render(const FunctionCoverageSummary &Function, raw_ostream &OS);
 
 public:
-  CoverageReport(const CoverageViewOptions &Options, CoverageSummary &Summary)
-      : Options(Options), Summary(Summary) {}
+  CoverageReport(const CoverageViewOptions &Options,
+                 std::unique_ptr<coverage::CoverageMapping> Coverage)
+      : Options(Options), Coverage(std::move(Coverage)) {}
 
   void renderFunctionReports(raw_ostream &OS);
 

tools/llvm-cov/CoverageSummary.cpp

@@ -1,64 +0,0 @@
-//===- CoverageSummary.cpp - Code coverage summary ------------------------===//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-//
-// This class implements data management and rendering for the code coverage
-// summaries of all files and functions.
-//
-//===----------------------------------------------------------------------===//
-
-#include "CoverageSummary.h"
-#include "llvm/Support/FileSystem.h"
-#include "llvm/Support/Format.h"
-
-using namespace llvm;
-
-unsigned CoverageSummary::getFileID(StringRef Filename) {
-  for (unsigned I = 0, E = Filenames.size(); I < E; ++I) {
-    if (sys::fs::equivalent(Filenames[I], Filename))
-      return I;
-  }
-  Filenames.push_back(Filename);
-  return Filenames.size() - 1;
-}
-
-void
-CoverageSummary::createSummaries(const coverage::CoverageMapping &Coverage) {
-  for (StringRef Filename : Coverage.getUniqueSourceFiles()) {
-    size_t PrevSize = FunctionSummaries.size();
-    for (const auto &F : Coverage.getCoveredFunctions(Filename))
-      FunctionSummaries.push_back(FunctionCoverageSummary::get(F));
-    size_t Count = FunctionSummaries.size() - PrevSize;
-    if (Count == 0)
-      continue;
-    FileSummaries.push_back(FileCoverageSummary::get(
-        Filename, makeArrayRef(FunctionSummaries.data() + PrevSize, Count)));
-  }
-}
-
-FileCoverageSummary CoverageSummary::getCombinedFileSummaries() {
-  size_t NumRegions = 0, CoveredRegions = 0;
-  size_t NumLines = 0, NonCodeLines = 0, CoveredLines = 0;
-  size_t NumFunctionsExecuted = 0, NumFunctions = 0;
-  for (const auto &File : FileSummaries) {
-    NumRegions += File.RegionCoverage.NumRegions;
-    CoveredRegions += File.RegionCoverage.Covered;
-
-    NumLines += File.LineCoverage.NumLines;
-    NonCodeLines += File.LineCoverage.NonCodeLines;
-    CoveredLines += File.LineCoverage.Covered;
-
-    NumFunctionsExecuted += File.FunctionCoverage.Executed;
-    NumFunctions += File.FunctionCoverage.NumFunctions;
-  }
-  return FileCoverageSummary(
-      "TOTAL", RegionCoverageInfo(CoveredRegions, NumRegions),
-      LineCoverageInfo(CoveredLines, NonCodeLines, NumLines),
-      FunctionCoverageInfo(NumFunctionsExecuted, NumFunctions),
-      None);
-}

tools/llvm-cov/CoverageSummary.h

@@ -1,45 +0,0 @@
-//===- CoverageSummary.h - Code coverage summary --------------------------===//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-//
-// This class implements data management and rendering for the code coverage
-// summaries of all files and functions.
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_COV_COVERAGESUMMARY_H
-#define LLVM_COV_COVERAGESUMMARY_H
-
-#include "CoverageSummaryInfo.h"
-#include <vector>
-
-namespace llvm {
-
-/// \brief Manager for the function and file code coverage summaries.
-class CoverageSummary {
-  std::vector<StringRef> Filenames;
-  std::vector<FunctionCoverageSummary> FunctionSummaries;
-  std::vector<std::pair<unsigned, unsigned>> FunctionSummariesFileIDs;
-  std::vector<FileCoverageSummary> FileSummaries;
-
-  unsigned getFileID(StringRef Filename);
-
-public:
-  void createSummaries(const coverage::CoverageMapping &Coverage);
-
-  ArrayRef<FileCoverageSummary> getFileSummaries() { return FileSummaries; }
-
-  FileCoverageSummary getCombinedFileSummaries();
-
-  void render(const FunctionCoverageSummary &Summary, raw_ostream &OS);
-
-  void render(raw_ostream &OS);
-};
-}
-
-#endif // LLVM_COV_COVERAGESUMMARY_H

tools/llvm-cov/CoverageSummaryInfo.cpp

@@ -69,28 +69,3 @@ FunctionCoverageSummary::get(const coverage::FunctionRecord &Function) {
       RegionCoverageInfo(CoveredRegions, NumCodeRegions),
       LineCoverageInfo(CoveredLines, 0, NumLines));
 }
-
-FileCoverageSummary
-FileCoverageSummary::get(StringRef Name,
-                         ArrayRef<FunctionCoverageSummary> FunctionSummaries) {
-  size_t NumRegions = 0, CoveredRegions = 0;
-  size_t NumLines = 0, NonCodeLines = 0, CoveredLines = 0;
-  size_t NumFunctionsExecuted = 0;
-  for (const auto &Func : FunctionSummaries) {
-    CoveredRegions += Func.RegionCoverage.Covered;
-    NumRegions += Func.RegionCoverage.NumRegions;
-
-    CoveredLines += Func.LineCoverage.Covered;
-    NonCodeLines += Func.LineCoverage.NonCodeLines;
-    NumLines += Func.LineCoverage.NumLines;
-
-    if (Func.ExecutionCount != 0)
-      ++NumFunctionsExecuted;
-  }
-
-  return FileCoverageSummary(
-      Name, RegionCoverageInfo(CoveredRegions, NumRegions),
-      LineCoverageInfo(CoveredLines, NonCodeLines, NumLines),
-      FunctionCoverageInfo(NumFunctionsExecuted, FunctionSummaries.size()),
-      FunctionSummaries);
-}

tools/llvm-cov/CoverageSummaryInfo.h

@@ -31,10 +31,19 @@ struct RegionCoverageInfo {
   /// \brief The total number of regions in a function/file.
   size_t NumRegions;
 
+  RegionCoverageInfo() : Covered(0), NotCovered(0), NumRegions(0) {}
+
   RegionCoverageInfo(size_t Covered, size_t NumRegions)
       : Covered(Covered), NotCovered(NumRegions - Covered),
         NumRegions(NumRegions) {}
 
+  RegionCoverageInfo &operator+=(const RegionCoverageInfo &RHS) {
+    Covered += RHS.Covered;
+    NotCovered += RHS.NotCovered;
+    NumRegions += RHS.NumRegions;
+    return *this;
+  }
+
   bool isFullyCovered() const { return Covered == NumRegions; }
 
   double getPercentCovered() const {
@@ -56,10 +65,21 @@ struct LineCoverageInfo {
   /// \brief The total number of lines in a function/file.
   size_t NumLines;
 
+  LineCoverageInfo()
+      : Covered(0), NotCovered(0), NonCodeLines(0), NumLines(0) {}
+
   LineCoverageInfo(size_t Covered, size_t NumNonCodeLines, size_t NumLines)
       : Covered(Covered), NotCovered(NumLines - NumNonCodeLines - Covered),
         NonCodeLines(NumNonCodeLines), NumLines(NumLines) {}
 
+  LineCoverageInfo &operator+=(const LineCoverageInfo &RHS) {
+    Covered += RHS.Covered;
+    NotCovered += RHS.NotCovered;
+    NonCodeLines += RHS.NonCodeLines;
+    NumLines += RHS.NumLines;
+    return *this;
+  }
+
   bool isFullyCovered() const { return Covered == (NumLines - NonCodeLines); }
 
   double getPercentCovered() const {
@@ -75,9 +95,17 @@ struct FunctionCoverageInfo {
   /// \brief The total number of functions in this file.
   size_t NumFunctions;
 
+  FunctionCoverageInfo() : Executed(0), NumFunctions(0) {}
+
   FunctionCoverageInfo(size_t Executed, size_t NumFunctions)
       : Executed(Executed), NumFunctions(NumFunctions) {}
 
+  void addFunction(bool Covered) {
+    if (Covered)
+      ++Executed;
+    ++NumFunctions;
+  }
+
   bool isFullyCovered() const { return Executed == NumFunctions; }
 
   double getPercentCovered() const {
@@ -92,6 +120,8 @@ struct FunctionCoverageSummary {
   RegionCoverageInfo RegionCoverage;
   LineCoverageInfo LineCoverage;
 
+  FunctionCoverageSummary(StringRef Name) : Name(Name), ExecutionCount(0) {}
+
   FunctionCoverageSummary(StringRef Name, uint64_t ExecutionCount,
                           const RegionCoverageInfo &RegionCoverage,
                           const LineCoverageInfo &LineCoverage)
@@ -111,21 +141,14 @@ struct FileCoverageSummary {
   RegionCoverageInfo RegionCoverage;
   LineCoverageInfo LineCoverage;
   FunctionCoverageInfo FunctionCoverage;
-  /// \brief The summary of every function
-  /// in this file.
-  ArrayRef<FunctionCoverageSummary> FunctionSummaries;
 
-  FileCoverageSummary(StringRef Name, const RegionCoverageInfo &RegionCoverage,
-                      const LineCoverageInfo &LineCoverage,
-                      const FunctionCoverageInfo &FunctionCoverage,
-                      ArrayRef<FunctionCoverageSummary> FunctionSummaries)
-      : Name(Name), RegionCoverage(RegionCoverage), LineCoverage(LineCoverage),
-        FunctionCoverage(FunctionCoverage),
-        FunctionSummaries(FunctionSummaries) {}
+  FileCoverageSummary(StringRef Name) : Name(Name) {}
 
-  /// \brief Compute the code coverage summary for a file.
-  static FileCoverageSummary
-  get(StringRef Name, ArrayRef<FunctionCoverageSummary> FunctionSummaries);
+  void addFunction(const FunctionCoverageSummary &Function) {
+    RegionCoverage += Function.RegionCoverage;
+    LineCoverage += Function.LineCoverage;
+    FunctionCoverage.addFunction(/*Covered=*/Function.ExecutionCount > 0);
+  }
 };
 
 } // namespace llvm