[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<std::string>
everywhere.

llvm-svn: 282281
This commit is contained in:
Vedant Kumar 2016-09-23 18:57:32 +00:00
parent 224ef8d73b
commit bc6479850e
10 changed files with 43 additions and 33 deletions

View File

@ -2,8 +2,18 @@ RUN: mkdir -p %t/a/b/
RUN: echo "" > %t/a/b/c.tmp RUN: echo "" > %t/a/b/c.tmp
RUN: echo "" > %t/a/d.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 | 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 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 REAL-DAG: {{.*}}c.tmp
CHECK-DAG: {{.*}}d.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

View File

@ -64,7 +64,8 @@ private:
/// \brief Print the warning message to the error output stream. /// \brief Print the warning message to the error output stream.
void warning(const Twine &Message, StringRef Whence = ""); 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); void addCollectedPath(const std::string &Path);
/// \brief If \p Path is a regular file, collect the path. If it's a /// \brief If \p Path is a regular file, collect the path. If it's a
@ -116,7 +117,7 @@ private:
std::string PGOFilename; std::string PGOFilename;
/// A list of input source files. /// A list of input source files.
std::vector<StringRef> SourceFiles; std::vector<std::string> SourceFiles;
/// Whether or not we're in -filename-equivalence mode. /// Whether or not we're in -filename-equivalence mode.
bool CompareFilenamesOnly; bool CompareFilenamesOnly;
@ -131,9 +132,6 @@ private:
/// A cache for demangled symbol names. /// A cache for demangled symbol names.
StringMap<std::string> DemangledNames; StringMap<std::string> DemangledNames;
/// File paths (absolute, or otherwise) to input source files.
std::vector<std::string> CollectedPaths;
/// Errors and warnings which have not been printed. /// Errors and warnings which have not been printed.
std::mutex ErrsLock; std::mutex ErrsLock;
@ -168,7 +166,7 @@ void CodeCoverageTool::warning(const Twine &Message, StringRef Whence) {
void CodeCoverageTool::addCollectedPath(const std::string &Path) { void CodeCoverageTool::addCollectedPath(const std::string &Path) {
if (CompareFilenamesOnly) { if (CompareFilenamesOnly) {
CollectedPaths.push_back(Path); SourceFiles.emplace_back(Path);
} else { } else {
SmallString<128> EffectivePath(Path); SmallString<128> EffectivePath(Path);
if (std::error_code EC = sys::fs::make_absolute(EffectivePath)) { if (std::error_code EC = sys::fs::make_absolute(EffectivePath)) {
@ -176,10 +174,8 @@ void CodeCoverageTool::addCollectedPath(const std::string &Path) {
return; return;
} }
sys::path::remove_dots(EffectivePath, /*remove_dot_dots=*/true); 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) { void CodeCoverageTool::collectPaths(const std::string &Path) {
@ -597,7 +593,7 @@ int CodeCoverageTool::run(Command Cmd, int argc, const char **argv) {
collectPaths(File); collectPaths(File);
if (DebugDumpCollectedPaths) { if (DebugDumpCollectedPaths) {
for (StringRef SF : SourceFiles) for (const std::string &SF : SourceFiles)
outs() << SF << '\n'; outs() << SF << '\n';
::exit(0); ::exit(0);
} }
@ -751,11 +747,11 @@ int CodeCoverageTool::show(int argc, const char **argv,
ThreadCount = std::thread::hardware_concurrency(); ThreadCount = std::thread::hardware_concurrency();
ThreadPool Pool(ThreadCount); ThreadPool Pool(ThreadCount);
for (StringRef SourceFile : SourceFiles) { for (const std::string &SourceFile : SourceFiles) {
Pool.async([this, SourceFile, &Coverage, &Printer, ShowFilenames] { Pool.async([this, &SourceFile, &Coverage, &Printer, ShowFilenames] {
auto View = createSourceFileView(SourceFile, *Coverage); auto View = createSourceFileView(SourceFile, *Coverage);
if (!View) { if (!View) {
warning("The file '" + SourceFile.str() + "' isn't covered."); warning("The file '" + SourceFile + "' isn't covered.");
return; return;
} }

View File

@ -175,7 +175,9 @@ class CoverageExporterJson {
emitDictKey("files"); emitDictKey("files");
FileCoverageSummary Totals = FileCoverageSummary("Totals"); FileCoverageSummary Totals = FileCoverageSummary("Totals");
std::vector<StringRef> SourceFiles = Coverage.getUniqueSourceFiles(); std::vector<std::string> SourceFiles;
for (StringRef SF : Coverage.getUniqueSourceFiles())
SourceFiles.emplace_back(SF);
auto FileReports = auto FileReports =
CoverageReport::prepareFileReports(Coverage, Totals, SourceFiles); CoverageReport::prepareFileReports(Coverage, Totals, SourceFiles);
renderFiles(SourceFiles, FileReports); renderFiles(SourceFiles, FileReports);
@ -235,7 +237,7 @@ class CoverageExporterJson {
} }
/// \brief Render an array of all the source files, also pass back a Summary. /// \brief Render an array of all the source files, also pass back a Summary.
void renderFiles(ArrayRef<StringRef> SourceFiles, void renderFiles(ArrayRef<std::string> SourceFiles,
ArrayRef<FileCoverageSummary> FileReports) { ArrayRef<FileCoverageSummary> FileReports) {
// Start List of Files. // Start List of Files.
emitArrayStart(); emitArrayStart();

View File

@ -120,7 +120,7 @@ raw_ostream::Colors determineCoveragePercentageColor(const T &Info) {
/// \brief Determine the length of the longest common prefix of the strings in /// \brief Determine the length of the longest common prefix of the strings in
/// \p Strings. /// \p Strings.
unsigned getLongestCommonPrefixLen(ArrayRef<StringRef> Strings) { unsigned getLongestCommonPrefixLen(ArrayRef<std::string> Strings) {
unsigned LCP = Strings[0].size(); unsigned LCP = Strings[0].size();
for (unsigned I = 1, E = Strings.size(); LCP > 0 && I < E; ++I) { for (unsigned I = 1, E = Strings.size(); LCP > 0 && I < E; ++I) {
auto Mismatch = auto Mismatch =
@ -215,7 +215,7 @@ void CoverageReport::render(const FunctionCoverageSummary &Function,
OS << "\n"; OS << "\n";
} }
void CoverageReport::renderFunctionReports(ArrayRef<StringRef> Files, void CoverageReport::renderFunctionReports(ArrayRef<std::string> Files,
raw_ostream &OS) { raw_ostream &OS) {
bool isFirst = true; bool isFirst = true;
for (StringRef Filename : Files) { for (StringRef Filename : Files) {
@ -261,7 +261,7 @@ void CoverageReport::renderFunctionReports(ArrayRef<StringRef> Files,
std::vector<FileCoverageSummary> std::vector<FileCoverageSummary>
CoverageReport::prepareFileReports(const coverage::CoverageMapping &Coverage, CoverageReport::prepareFileReports(const coverage::CoverageMapping &Coverage,
FileCoverageSummary &Totals, FileCoverageSummary &Totals,
ArrayRef<StringRef> Files) { ArrayRef<std::string> Files) {
std::vector<FileCoverageSummary> FileReports; std::vector<FileCoverageSummary> FileReports;
unsigned LCP = 0; unsigned LCP = 0;
if (Files.size() > 1) if (Files.size() > 1)
@ -298,12 +298,14 @@ CoverageReport::prepareFileReports(const coverage::CoverageMapping &Coverage,
} }
void CoverageReport::renderFileReports(raw_ostream &OS) const { void CoverageReport::renderFileReports(raw_ostream &OS) const {
std::vector<StringRef> UniqueSourceFiles = Coverage.getUniqueSourceFiles(); std::vector<std::string> UniqueSourceFiles;
for (StringRef SF : Coverage.getUniqueSourceFiles())
UniqueSourceFiles.emplace_back(SF.str());
renderFileReports(OS, UniqueSourceFiles); renderFileReports(OS, UniqueSourceFiles);
} }
void CoverageReport::renderFileReports(raw_ostream &OS, void CoverageReport::renderFileReports(raw_ostream &OS,
ArrayRef<StringRef> Files) const { ArrayRef<std::string> Files) const {
FileCoverageSummary Totals("TOTAL"); FileCoverageSummary Totals("TOTAL");
auto FileReports = prepareFileReports(Coverage, Totals, Files); auto FileReports = prepareFileReports(Coverage, Totals, Files);

View File

@ -32,18 +32,18 @@ public:
const coverage::CoverageMapping &Coverage) const coverage::CoverageMapping &Coverage)
: Options(Options), Coverage(Coverage) {} : Options(Options), Coverage(Coverage) {}
void renderFunctionReports(ArrayRef<StringRef> Files, raw_ostream &OS); void renderFunctionReports(ArrayRef<std::string> Files, raw_ostream &OS);
/// Prepare file reports for the files specified in \p Files. /// Prepare file reports for the files specified in \p Files.
static std::vector<FileCoverageSummary> static std::vector<FileCoverageSummary>
prepareFileReports(const coverage::CoverageMapping &Coverage, prepareFileReports(const coverage::CoverageMapping &Coverage,
FileCoverageSummary &Totals, ArrayRef<StringRef> Files); FileCoverageSummary &Totals, ArrayRef<std::string> Files);
/// Render file reports for every unique file in the coverage mapping. /// Render file reports for every unique file in the coverage mapping.
void renderFileReports(raw_ostream &OS) const; void renderFileReports(raw_ostream &OS) const;
/// Render file reports for the files specified in \p Files. /// Render file reports for the files specified in \p Files.
void renderFileReports(raw_ostream &OS, ArrayRef<StringRef> Files) const; void renderFileReports(raw_ostream &OS, ArrayRef<std::string> Files) const;
}; };
} // end namespace llvm } // end namespace llvm

View File

@ -142,7 +142,7 @@ public:
virtual void closeViewFile(OwnedStream OS) = 0; virtual void closeViewFile(OwnedStream OS) = 0;
/// \brief Create an index which lists reports for the given source files. /// \brief Create an index which lists reports for the given source files.
virtual Error createIndexFile(ArrayRef<StringRef> SourceFiles, virtual Error createIndexFile(ArrayRef<std::string> SourceFiles,
const coverage::CoverageMapping &Coverage) = 0; const coverage::CoverageMapping &Coverage) = 0;
/// @} /// @}

View File

@ -347,7 +347,7 @@ void CoveragePrinterHTML::emitFileSummary(raw_ostream &OS, StringRef SF,
} }
Error CoveragePrinterHTML::createIndexFile( Error CoveragePrinterHTML::createIndexFile(
ArrayRef<StringRef> SourceFiles, ArrayRef<std::string> SourceFiles,
const coverage::CoverageMapping &Coverage) { const coverage::CoverageMapping &Coverage) {
// Emit the default stylesheet. // Emit the default stylesheet.
auto CSSOrErr = createOutputStream("style", "css", /*InToplevel=*/true); auto CSSOrErr = createOutputStream("style", "css", /*InToplevel=*/true);

View File

@ -28,7 +28,7 @@ public:
void closeViewFile(OwnedStream OS) override; void closeViewFile(OwnedStream OS) override;
Error createIndexFile(ArrayRef<StringRef> SourceFiles, Error createIndexFile(ArrayRef<std::string> SourceFiles,
const coverage::CoverageMapping &Coverage) override; const coverage::CoverageMapping &Coverage) override;
CoveragePrinterHTML(const CoverageViewOptions &Opts) CoveragePrinterHTML(const CoverageViewOptions &Opts)

View File

@ -29,7 +29,7 @@ void CoveragePrinterText::closeViewFile(OwnedStream OS) {
} }
Error CoveragePrinterText::createIndexFile( Error CoveragePrinterText::createIndexFile(
ArrayRef<StringRef> SourceFiles, ArrayRef<std::string> SourceFiles,
const coverage::CoverageMapping &Coverage) { const coverage::CoverageMapping &Coverage) {
auto OSOrErr = createOutputStream("index", "txt", /*InToplevel=*/true); auto OSOrErr = createOutputStream("index", "txt", /*InToplevel=*/true);
if (Error E = OSOrErr.takeError()) if (Error E = OSOrErr.takeError())
@ -38,7 +38,7 @@ Error CoveragePrinterText::createIndexFile(
raw_ostream &OSRef = *OS.get(); raw_ostream &OSRef = *OS.get();
CoverageReport Report(Opts, Coverage); CoverageReport Report(Opts, Coverage);
Report.renderFileReports(OSRef); Report.renderFileReports(OSRef, SourceFiles);
Opts.colored_ostream(OSRef, raw_ostream::CYAN) << "\n" Opts.colored_ostream(OSRef, raw_ostream::CYAN) << "\n"
<< Opts.getLLVMVersionString(); << Opts.getLLVMVersionString();

View File

@ -26,7 +26,7 @@ public:
void closeViewFile(OwnedStream OS) override; void closeViewFile(OwnedStream OS) override;
Error createIndexFile(ArrayRef<StringRef> SourceFiles, Error createIndexFile(ArrayRef<std::string> SourceFiles,
const coverage::CoverageMapping &Coverage) override; const coverage::CoverageMapping &Coverage) override;
CoveragePrinterText(const CoverageViewOptions &Opts) CoveragePrinterText(const CoverageViewOptions &Opts)