From fd066351a5eb34b4cd04741be998094ffad95ce9 Mon Sep 17 00:00:00 2001 From: Vedant Kumar Date: Fri, 23 Sep 2016 18:57:32 +0000 Subject: [PATCH] [llvm-cov] Get rid of all invalid filename references We used to append filenames into a vector of std::string, and then append a reference to each string into a separate vector. This made it easier to work with the getUniqueSourceFiles API. But it's buggy. std::string has a small-string optimization, so you can't expect to capture a reference to one if you're copying it into a growing vector. Add a test that triggers this invalid reference to std::string scenario, and kill the issue with fire by just using ArrayRef everywhere. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@282281 91177308-0d34-0410-b5e6-96231b3b80d8 --- test/tools/llvm-cov/scan-directory.test | 18 ++++++++++++++---- tools/llvm-cov/CodeCoverage.cpp | 22 +++++++++------------- tools/llvm-cov/CoverageExporterJson.cpp | 6 ++++-- tools/llvm-cov/CoverageReport.cpp | 12 +++++++----- tools/llvm-cov/CoverageReport.h | 6 +++--- tools/llvm-cov/SourceCoverageView.h | 2 +- tools/llvm-cov/SourceCoverageViewHTML.cpp | 2 +- tools/llvm-cov/SourceCoverageViewHTML.h | 2 +- tools/llvm-cov/SourceCoverageViewText.cpp | 4 ++-- tools/llvm-cov/SourceCoverageViewText.h | 2 +- 10 files changed, 43 insertions(+), 33 deletions(-) diff --git a/test/tools/llvm-cov/scan-directory.test b/test/tools/llvm-cov/scan-directory.test index 9bacfc209dd..b7f28584267 100644 --- a/test/tools/llvm-cov/scan-directory.test +++ b/test/tools/llvm-cov/scan-directory.test @@ -2,8 +2,18 @@ RUN: mkdir -p %t/a/b/ RUN: echo "" > %t/a/b/c.tmp RUN: echo "" > %t/a/d.tmp -RUN: llvm-cov show /dev/null -instr-profile /dev/null -dump-collected-paths %t | FileCheck %s -RUN: llvm-cov show /dev/null -instr-profile /dev/null -dump-collected-paths %t/a/b/c.tmp %t/a/d.tmp | FileCheck %s +RUN: llvm-cov show /dev/null -instr-profile /dev/null -dump-collected-paths %t | FileCheck %s --check-prefix=REAL +RUN: llvm-cov show /dev/null -instr-profile /dev/null -dump-collected-paths %t/a/b/c.tmp %t/a/d.tmp | FileCheck %s --check-prefix=REAL -CHECK-DAG: {{.*}}c.tmp -CHECK-DAG: {{.*}}d.tmp +REAL-DAG: {{.*}}c.tmp +REAL-DAG: {{.*}}d.tmp + +RUN: llvm-cov show /dev/null -instr-profile /dev/null -dump-collected-paths -filename-equivalence %t a b c d e f | FileCheck %s --check-prefix=EQUIV +EQUIV-DAG: {{.*}}c.tmp +EQUIV-DAG: {{.*}}d.tmp +EQUIV-DAG: a +EQUIV-DAG: b +EQUIV-DAG: c +EQUIV-DAG: d +EQUIV-DAG: e +EQUIV-DAG: f diff --git a/tools/llvm-cov/CodeCoverage.cpp b/tools/llvm-cov/CodeCoverage.cpp index 237d877cde1..51ac6ef3d88 100644 --- a/tools/llvm-cov/CodeCoverage.cpp +++ b/tools/llvm-cov/CodeCoverage.cpp @@ -64,7 +64,8 @@ private: /// \brief Print the warning message to the error output stream. void warning(const Twine &Message, StringRef Whence = ""); - /// \brief Copy \p Path into the list of input source files. + /// \brief Convert \p Path into an absolute path and append it to the list + /// of collected paths. void addCollectedPath(const std::string &Path); /// \brief If \p Path is a regular file, collect the path. If it's a @@ -116,7 +117,7 @@ private: std::string PGOFilename; /// A list of input source files. - std::vector SourceFiles; + std::vector SourceFiles; /// Whether or not we're in -filename-equivalence mode. bool CompareFilenamesOnly; @@ -131,9 +132,6 @@ private: /// A cache for demangled symbol names. StringMap DemangledNames; - /// File paths (absolute, or otherwise) to input source files. - std::vector CollectedPaths; - /// Errors and warnings which have not been printed. std::mutex ErrsLock; @@ -168,7 +166,7 @@ void CodeCoverageTool::warning(const Twine &Message, StringRef Whence) { void CodeCoverageTool::addCollectedPath(const std::string &Path) { if (CompareFilenamesOnly) { - CollectedPaths.push_back(Path); + SourceFiles.emplace_back(Path); } else { SmallString<128> EffectivePath(Path); if (std::error_code EC = sys::fs::make_absolute(EffectivePath)) { @@ -176,10 +174,8 @@ void CodeCoverageTool::addCollectedPath(const std::string &Path) { return; } sys::path::remove_dots(EffectivePath, /*remove_dot_dots=*/true); - CollectedPaths.push_back(EffectivePath.str()); + SourceFiles.emplace_back(EffectivePath.str()); } - - SourceFiles.emplace_back(CollectedPaths.back()); } void CodeCoverageTool::collectPaths(const std::string &Path) { @@ -597,7 +593,7 @@ int CodeCoverageTool::run(Command Cmd, int argc, const char **argv) { collectPaths(File); if (DebugDumpCollectedPaths) { - for (StringRef SF : SourceFiles) + for (const std::string &SF : SourceFiles) outs() << SF << '\n'; ::exit(0); } @@ -751,11 +747,11 @@ int CodeCoverageTool::show(int argc, const char **argv, ThreadCount = std::thread::hardware_concurrency(); ThreadPool Pool(ThreadCount); - for (StringRef SourceFile : SourceFiles) { - Pool.async([this, SourceFile, &Coverage, &Printer, ShowFilenames] { + for (const std::string &SourceFile : SourceFiles) { + Pool.async([this, &SourceFile, &Coverage, &Printer, ShowFilenames] { auto View = createSourceFileView(SourceFile, *Coverage); if (!View) { - warning("The file '" + SourceFile.str() + "' isn't covered."); + warning("The file '" + SourceFile + "' isn't covered."); return; } diff --git a/tools/llvm-cov/CoverageExporterJson.cpp b/tools/llvm-cov/CoverageExporterJson.cpp index e8dee147c00..06dc176737a 100644 --- a/tools/llvm-cov/CoverageExporterJson.cpp +++ b/tools/llvm-cov/CoverageExporterJson.cpp @@ -175,7 +175,9 @@ class CoverageExporterJson { emitDictKey("files"); FileCoverageSummary Totals = FileCoverageSummary("Totals"); - std::vector SourceFiles = Coverage.getUniqueSourceFiles(); + std::vector SourceFiles; + for (StringRef SF : Coverage.getUniqueSourceFiles()) + SourceFiles.emplace_back(SF); auto FileReports = CoverageReport::prepareFileReports(Coverage, Totals, SourceFiles); renderFiles(SourceFiles, FileReports); @@ -235,7 +237,7 @@ class CoverageExporterJson { } /// \brief Render an array of all the source files, also pass back a Summary. - void renderFiles(ArrayRef SourceFiles, + void renderFiles(ArrayRef SourceFiles, ArrayRef FileReports) { // Start List of Files. emitArrayStart(); diff --git a/tools/llvm-cov/CoverageReport.cpp b/tools/llvm-cov/CoverageReport.cpp index 0d34722eae5..24d7c661aad 100644 --- a/tools/llvm-cov/CoverageReport.cpp +++ b/tools/llvm-cov/CoverageReport.cpp @@ -120,7 +120,7 @@ raw_ostream::Colors determineCoveragePercentageColor(const T &Info) { /// \brief Determine the length of the longest common prefix of the strings in /// \p Strings. -unsigned getLongestCommonPrefixLen(ArrayRef Strings) { +unsigned getLongestCommonPrefixLen(ArrayRef Strings) { unsigned LCP = Strings[0].size(); for (unsigned I = 1, E = Strings.size(); LCP > 0 && I < E; ++I) { auto Mismatch = @@ -215,7 +215,7 @@ void CoverageReport::render(const FunctionCoverageSummary &Function, OS << "\n"; } -void CoverageReport::renderFunctionReports(ArrayRef Files, +void CoverageReport::renderFunctionReports(ArrayRef Files, raw_ostream &OS) { bool isFirst = true; for (StringRef Filename : Files) { @@ -261,7 +261,7 @@ void CoverageReport::renderFunctionReports(ArrayRef Files, std::vector CoverageReport::prepareFileReports(const coverage::CoverageMapping &Coverage, FileCoverageSummary &Totals, - ArrayRef Files) { + ArrayRef Files) { std::vector FileReports; unsigned LCP = 0; if (Files.size() > 1) @@ -298,12 +298,14 @@ CoverageReport::prepareFileReports(const coverage::CoverageMapping &Coverage, } void CoverageReport::renderFileReports(raw_ostream &OS) const { - std::vector UniqueSourceFiles = Coverage.getUniqueSourceFiles(); + std::vector UniqueSourceFiles; + for (StringRef SF : Coverage.getUniqueSourceFiles()) + UniqueSourceFiles.emplace_back(SF.str()); renderFileReports(OS, UniqueSourceFiles); } void CoverageReport::renderFileReports(raw_ostream &OS, - ArrayRef Files) const { + ArrayRef Files) const { FileCoverageSummary Totals("TOTAL"); auto FileReports = prepareFileReports(Coverage, Totals, Files); diff --git a/tools/llvm-cov/CoverageReport.h b/tools/llvm-cov/CoverageReport.h index 37bb842f13d..7a416497e25 100644 --- a/tools/llvm-cov/CoverageReport.h +++ b/tools/llvm-cov/CoverageReport.h @@ -32,18 +32,18 @@ public: const coverage::CoverageMapping &Coverage) : Options(Options), Coverage(Coverage) {} - void renderFunctionReports(ArrayRef Files, raw_ostream &OS); + void renderFunctionReports(ArrayRef Files, raw_ostream &OS); /// Prepare file reports for the files specified in \p Files. static std::vector prepareFileReports(const coverage::CoverageMapping &Coverage, - FileCoverageSummary &Totals, ArrayRef Files); + FileCoverageSummary &Totals, ArrayRef Files); /// Render file reports for every unique file in the coverage mapping. void renderFileReports(raw_ostream &OS) const; /// Render file reports for the files specified in \p Files. - void renderFileReports(raw_ostream &OS, ArrayRef Files) const; + void renderFileReports(raw_ostream &OS, ArrayRef Files) const; }; } // end namespace llvm diff --git a/tools/llvm-cov/SourceCoverageView.h b/tools/llvm-cov/SourceCoverageView.h index 129c5902070..2069fe56d8c 100644 --- a/tools/llvm-cov/SourceCoverageView.h +++ b/tools/llvm-cov/SourceCoverageView.h @@ -142,7 +142,7 @@ public: virtual void closeViewFile(OwnedStream OS) = 0; /// \brief Create an index which lists reports for the given source files. - virtual Error createIndexFile(ArrayRef SourceFiles, + virtual Error createIndexFile(ArrayRef SourceFiles, const coverage::CoverageMapping &Coverage) = 0; /// @} diff --git a/tools/llvm-cov/SourceCoverageViewHTML.cpp b/tools/llvm-cov/SourceCoverageViewHTML.cpp index 1cb283cdc1d..2707260b8ad 100644 --- a/tools/llvm-cov/SourceCoverageViewHTML.cpp +++ b/tools/llvm-cov/SourceCoverageViewHTML.cpp @@ -347,7 +347,7 @@ void CoveragePrinterHTML::emitFileSummary(raw_ostream &OS, StringRef SF, } Error CoveragePrinterHTML::createIndexFile( - ArrayRef SourceFiles, + ArrayRef SourceFiles, const coverage::CoverageMapping &Coverage) { // Emit the default stylesheet. auto CSSOrErr = createOutputStream("style", "css", /*InToplevel=*/true); diff --git a/tools/llvm-cov/SourceCoverageViewHTML.h b/tools/llvm-cov/SourceCoverageViewHTML.h index ad4b2b8ab06..94b08a5e7fc 100644 --- a/tools/llvm-cov/SourceCoverageViewHTML.h +++ b/tools/llvm-cov/SourceCoverageViewHTML.h @@ -28,7 +28,7 @@ public: void closeViewFile(OwnedStream OS) override; - Error createIndexFile(ArrayRef SourceFiles, + Error createIndexFile(ArrayRef SourceFiles, const coverage::CoverageMapping &Coverage) override; CoveragePrinterHTML(const CoverageViewOptions &Opts) diff --git a/tools/llvm-cov/SourceCoverageViewText.cpp b/tools/llvm-cov/SourceCoverageViewText.cpp index f5201d9c808..68891298a07 100644 --- a/tools/llvm-cov/SourceCoverageViewText.cpp +++ b/tools/llvm-cov/SourceCoverageViewText.cpp @@ -29,7 +29,7 @@ void CoveragePrinterText::closeViewFile(OwnedStream OS) { } Error CoveragePrinterText::createIndexFile( - ArrayRef SourceFiles, + ArrayRef SourceFiles, const coverage::CoverageMapping &Coverage) { auto OSOrErr = createOutputStream("index", "txt", /*InToplevel=*/true); if (Error E = OSOrErr.takeError()) @@ -38,7 +38,7 @@ Error CoveragePrinterText::createIndexFile( raw_ostream &OSRef = *OS.get(); CoverageReport Report(Opts, Coverage); - Report.renderFileReports(OSRef); + Report.renderFileReports(OSRef, SourceFiles); Opts.colored_ostream(OSRef, raw_ostream::CYAN) << "\n" << Opts.getLLVMVersionString(); diff --git a/tools/llvm-cov/SourceCoverageViewText.h b/tools/llvm-cov/SourceCoverageViewText.h index 3968aa70dd5..c3f20de9297 100644 --- a/tools/llvm-cov/SourceCoverageViewText.h +++ b/tools/llvm-cov/SourceCoverageViewText.h @@ -26,7 +26,7 @@ public: void closeViewFile(OwnedStream OS) override; - Error createIndexFile(ArrayRef SourceFiles, + Error createIndexFile(ArrayRef SourceFiles, const coverage::CoverageMapping &Coverage) override; CoveragePrinterText(const CoverageViewOptions &Opts) -- 2.11.0