Revert "[Coverage] Store compilation dir separately in coverage mapping"

vedantk · vedantk · commit 703a0ea940f2 · 2021-08-30T17:03:22.000-07:00
This reverts commit 5fbd1a3. See: rdar://82543962 (Revert coverage changes from clang-1316 to make coverage format in clang-1316 identical to clang-1300) Testing: * check-profile passed before and after this revert. * check-clang reported 4 failing tests prior to this revert; that remains unchanged. Driver/clang_f_opts.c required a workaround. The coverage-compilation-dir.c test was xfailed. * check-llvm passed before and after this revert. The llvm-cov/compilation_dir.c test is now xfailed. Coverage unit tests pass with the exception of non_code_region_counters, which was disabled because https://reviews.llvm.org/D101780 was folded into this revert. Conflicts: clang/include/clang/Basic/CodeGenOptions.h clang/include/clang/Driver/Options.td clang/lib/CodeGen/CoverageMappingGen.cpp clang/lib/Driver/ToolChains/Clang.cpp clang/test/Profile/profile-prefix-map.c llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp llvm/unittests/ProfileData/CoverageMappingTest.cpp
diff --git a/clang/include/clang/Basic/CodeGenOptions.h b/clang/include/clang/Basic/CodeGenOptions.h
@@ -158,9 +158,6 @@ class CodeGenOptions : public CodeGenOptionsBase {
   /// The string to embed in debug information as the current working directory.
   std::string DebugCompilationDir;
 
-  /// The string to embed in coverage mapping as the current working directory.
-  std::string CoverageCompilationDir;
-
   /// The string to embed in the debug information for the compile unit, if
   /// non-empty.
   std::string DwarfDebugFlags;
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
@@ -1176,10 +1176,6 @@ def fdebug_compilation_dir_EQ : Joined<["-"], "fdebug-compilation-dir=">,
 def fdebug_compilation_dir : Separate<["-"], "fdebug-compilation-dir">,
     Group<f_Group>, Flags<[CC1Option, CC1AsOption, CoreOption]>,
     Alias<fdebug_compilation_dir_EQ>;
-def fcoverage_compilation_dir_EQ : Joined<["-"], "fcoverage-compilation-dir=">,
-    Group<f_Group>, Flags<[CC1Option, CC1AsOption, CoreOption]>,
-    HelpText<"The compilation directory to embed in the coverage mapping.">,
-    MarshallingInfoString<CodeGenOpts<"CoverageCompilationDir">>;
 def ffile_compilation_dir_EQ : Joined<["-"], "ffile-compilation-dir=">, Group<f_Group>,
     Flags<[CoreOption]>,
     HelpText<"The compilation directory to embed in the debug info and coverage mapping.">;
diff --git a/clang/lib/CodeGen/CoverageMappingGen.cpp b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1563,17 +1563,9 @@ CoverageMappingModuleGen::CoverageMappingModuleGen(
   CoveragePrefixMap = CGM.getCodeGenOpts().CoveragePrefixMap;
 }
 
-std::string CoverageMappingModuleGen::getCurrentDirname() {
-  if (!CGM.getCodeGenOpts().CoverageCompilationDir.empty())
-    return CGM.getCodeGenOpts().CoverageCompilationDir;
-
-  SmallString<256> CWD;
-  llvm::sys::fs::current_path(CWD);
-  return CWD.str().str();
-}
-
 std::string CoverageMappingModuleGen::normalizeFilename(StringRef Filename) {
   llvm::SmallString<256> Path(Filename);
+  llvm::sys::fs::make_absolute(Path);
   llvm::sys::path::remove_dots(Path, /*remove_dot_dot=*/true);
   for (const auto &Entry : CoveragePrefixMap) {
     if (llvm::sys::path::replace_path_prefix(Path, Entry.first, Entry.second))
@@ -1654,17 +1646,18 @@ void CoverageMappingModuleGen::addFunctionMappingRecord(
     // also processed by the CoverageMappingWriter which performs
     // additional minimization operations such as reducing the number of
     // expressions.
-    llvm::SmallVector<std::string, 16> FilenameStrs;
     std::vector<StringRef> Filenames;
     std::vector<CounterExpression> Expressions;
     std::vector<CounterMappingRegion> Regions;
-    FilenameStrs.resize(FileEntries.size() + 1);
-    FilenameStrs[0] = normalizeFilename(getCurrentDirname());
+    llvm::SmallVector<std::string, 16> FilenameStrs;
+    llvm::SmallVector<StringRef, 16> FilenameRefs;
+    FilenameStrs.resize(FileEntries.size());
+    FilenameRefs.resize(FileEntries.size());
     for (const auto &Entry : FileEntries) {
       auto I = Entry.second;
       FilenameStrs[I] = normalizeFilename(Entry.first->getName());
+      FilenameRefs[I] = FilenameStrs[I];
     }
-    ArrayRef<std::string> FilenameRefs = llvm::makeArrayRef(FilenameStrs);
     RawCoverageMappingReader Reader(CoverageMapping, FilenameRefs, Filenames,
                                     Expressions, Regions);
     if (Reader.read())
@@ -1681,18 +1674,19 @@ void CoverageMappingModuleGen::emit() {
 
   // Create the filenames and merge them with coverage mappings
   llvm::SmallVector<std::string, 16> FilenameStrs;
-  FilenameStrs.resize(FileEntries.size() + 1);
-  // The first filename is the current working directory.
-  FilenameStrs[0] = normalizeFilename(getCurrentDirname());
+  llvm::SmallVector<StringRef, 16> FilenameRefs;
+  FilenameStrs.resize(FileEntries.size());
+  FilenameRefs.resize(FileEntries.size());
   for (const auto &Entry : FileEntries) {
     auto I = Entry.second;
     FilenameStrs[I] = normalizeFilename(Entry.first->getName());
+    FilenameRefs[I] = FilenameStrs[I];
   }
 
   std::string Filenames;
   {
     llvm::raw_string_ostream OS(Filenames);
-    CoverageFilenamesSectionWriter(FilenameStrs).write(OS);
+    CoverageFilenamesSectionWriter(FilenameRefs).write(OS);
   }
   auto *FilenamesVal =
       llvm::ConstantDataArray::getString(Ctx, Filenames, false);
@@ -1750,7 +1744,7 @@ unsigned CoverageMappingModuleGen::getFileID(const FileEntry *File) {
   auto It = FileEntries.find(File);
   if (It != FileEntries.end())
     return It->second;
-  unsigned FileID = FileEntries.size() + 1;
+  unsigned FileID = FileEntries.size();
   FileEntries.insert(std::make_pair(File, FileID));
   return FileID;
 }
diff --git a/clang/lib/CodeGen/CoverageMappingGen.h b/clang/lib/CodeGen/CoverageMappingGen.h
@@ -95,7 +95,6 @@ class CoverageMappingModuleGen {
   std::vector<FunctionInfo> FunctionRecords;
   std::map<std::string, std::string> CoveragePrefixMap;
 
-  std::string getCurrentDirname();
   std::string normalizeFilename(StringRef Filename);
 
   /// Emit a function record.
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -885,18 +885,6 @@ static void addPGOAndCoverageFlags(const ToolChain &TC, Compilation &C,
     CmdArgs.push_back("-fcoverage-mapping");
   }
 
-  if (Arg *A = Args.getLastArg(options::OPT_ffile_compilation_dir_EQ,
-                               options::OPT_fcoverage_compilation_dir_EQ)) {
-    if (A->getOption().matches(options::OPT_ffile_compilation_dir_EQ))
-      CmdArgs.push_back(Args.MakeArgString(
-          Twine("-fcoverage-compilation-dir=") + A->getValue()));
-    else
-      A->render(Args, CmdArgs);
-  } else if (llvm::ErrorOr<std::string> CWD =
-                 D.getVFS().getCurrentWorkingDirectory()) {
-    CmdArgs.push_back(Args.MakeArgString("-fcoverage-compilation-dir=" + *CWD));
-  }
-
   if (Args.hasArg(options::OPT_fprofile_exclude_files_EQ)) {
     auto *Arg = Args.getLastArg(options::OPT_fprofile_exclude_files_EQ);
     if (!Args.hasArg(options::OPT_coverage))
diff --git a/clang/test/CodeGen/coverage-compilation-dir.c b/clang/test/CodeGen/coverage-compilation-dir.c
@@ -1,3 +1,6 @@
+// XFAIL: *
+// See rdar://82543962.
+//
 // RUN: mkdir -p %t.dir && cd %t.dir
 // RUN: cp %s rel.c
 // RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-compilation-dir=/nonsense -fcoverage-mapping -emit-llvm -mllvm -enable-name-compression=false rel.c -o - | FileCheck -check-prefix=CHECK-NONSENSE %s
diff --git a/clang/test/CoverageMapping/abspath.cpp b/clang/test/CoverageMapping/abspath.cpp
@@ -1,19 +1,15 @@
 // RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false -fprofile-instrument=clang -fcoverage-mapping -mllvm -enable-name-compression=false -emit-llvm -main-file-name abspath.cpp %S/Inputs/../abspath.cpp -o - | FileCheck -check-prefix=RMDOTS %s
 
-// RMDOTS: @__llvm_coverage_mapping = {{.*}}"\02
+// RMDOTS: @__llvm_coverage_mapping = {{.*}}"\01
 // RMDOTS-NOT: Inputs
 // RMDOTS: "
 
 // RUN: mkdir -p %t/test && cd %t/test
 // RUN: echo "void f1() {}" > f1.c
-// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false -fprofile-instrument=clang -fcoverage-mapping -mllvm -enable-name-compression=false -emit-llvm -main-file-name abspath.cpp %t/test/f1.c -o - | FileCheck -check-prefix=ABSPATH %s
+// RUN: %clang_cc1 -mllvm -emptyline-comment-coverage=false -fprofile-instrument=clang -fcoverage-mapping -mllvm -enable-name-compression=false -emit-llvm -main-file-name abspath.cpp ../test/f1.c -o - | FileCheck -check-prefix=RELPATH %s
 
-// RELPATH: @__llvm_coverage_mapping = {{.*}}"\02
-// RELPATH: {{..(/|\\\\)test(/|\\\\)f1}}.c
+// RELPATH: @__llvm_coverage_mapping = {{.*}}"\01
+// RELPATH: {{[/\\].*(/|\\\\)test(/|\\\\)f1}}.c
 // RELPATH: "
 
-// ABSPATH: @__llvm_coverage_mapping = {{.*}}"\02
-// ABSPATH: {{[/\\].*(/|\\\\)test(/|\\\\)f1}}.c
-// ABSPATH: "
-
 void f1() {}
diff --git a/clang/test/Driver/clang_f_opts.c b/clang/test/Driver/clang_f_opts.c
@@ -524,10 +524,11 @@
 // CHECK-DEBUG-COMPILATION-DIR: "-fdebug-compilation-dir=."
 // CHECK-DEBUG-COMPILATION-DIR-NOT: "-ffile-compilation-dir=."
 
-// RUN: %clang -### -S -fprofile-instr-generate -fcoverage-compilation-dir=. %s 2>&1 | FileCheck -check-prefix=CHECK-COVERAGE-COMPILATION-DIR %s
-// RUN: %clang -### -S -fprofile-instr-generate -ffile-compilation-dir=. %s 2>&1 | FileCheck -check-prefix=CHECK-COVERAGE-COMPILATION-DIR %s
-// CHECK-COVERAGE-COMPILATION-DIR: "-fcoverage-compilation-dir=."
-// CHECK-COVERAGE-COMPILATION-DIR-NOT: "-ffile-compilation-dir=."
+// See rdar://82543962.
+// XXX: %clang -### -S -fprofile-instr-generate -fcoverage-compilation-dir=. %s 2>&1 | FileCheck -check-prefix=CHECK-COVERAGE-COMPILATION-DIR %s
+// XXX: %clang -### -S -fprofile-instr-generate -ffile-compilation-dir=. %s 2>&1 | FileCheck -check-prefix=CHECK-COVERAGE-COMPILATION-DIR %s
+// XXX-COVERAGE-COMPILATION-DIR: "-fcoverage-compilation-dir=."
+// XXX-COVERAGE-COMPILATION-DIR-NOT: "-ffile-compilation-dir=."
 
 // RUN: %clang -### -S -fdiscard-value-names %s 2>&1 | FileCheck -check-prefix=CHECK-DISCARD-NAMES %s
 // RUN: %clang -### -S -fno-discard-value-names %s 2>&1 | FileCheck -check-prefix=CHECK-NO-DISCARD-NAMES %s
diff --git a/clang/test/Profile/coverage-prefix-map.c b/clang/test/Profile/coverage-prefix-map.c
@@ -1,3 +1,6 @@
+// XFAIL: *
+// See rdar://82543962.
+//
 // %s expands to an absolute path, so to test relative paths we need to create a
 // clean directory, put the source there, and cd into it.
 // RUN: rm -rf %t
diff --git a/clang/test/Profile/profile-prefix-map.c b/clang/test/Profile/profile-prefix-map.c
@@ -0,0 +1,17 @@
+// XFAIL: *
+// See rdar://82543962.
+//
+// %s expands to an absolute path, so to test relative paths we need to create a
+// clean directory, put the source there, and cd into it.
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/root/nested
+// RUN: echo "void f1() {}" > %t/root/nested/profile-prefix-map.c
+// RUN: cd %t/root
+
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm -mllvm -enable-name-compression=false -main-file-name profile-prefix-map.c nested/profile-prefix-map.c -o - | FileCheck --check-prefix=ABSOLUTE %s
+//
+// ABSOLUTE: @__llvm_coverage_mapping = {{.*"\\01.*root.*nested.*profile-prefix-map\.c}}
+
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -emit-llvm -mllvm -enable-name-compression=false -main-file-name profile-prefix-map.c nested/profile-prefix-map.c -fprofile-prefix-map=%/t/root=. -o - | FileCheck --check-prefix=PROFILE-PREFIX-MAP %s --implicit-check-not=root
+//
+// PROFILE-PREFIX-MAP: @__llvm_coverage_mapping = {{.*"\\01[^/]*}}.{{/|\\+}}nested{{.*profile-prefix-map\.c}}
diff --git a/compiler-rt/include/profile/InstrProfData.inc b/compiler-rt/include/profile/InstrProfData.inc
@@ -650,7 +650,7 @@ serializeValueProfDataFrom(ValueProfRecordClosure *Closure,
 /* Indexed profile format version (start from 1). */
 #define INSTR_PROF_INDEX_VERSION 7
 /* Coverage mapping format version (start from 0). */
-#define INSTR_PROF_COVMAP_VERSION 5
+#define INSTR_PROF_COVMAP_VERSION 4
 
 /* Profile version is always of type uint64_t. Reserve the upper 8 bits in the
  * version for other variants of profile. We set the lowest bit of the upper 8
diff --git a/llvm/docs/CoverageMappingFormat.rst b/llvm/docs/CoverageMappingFormat.rst
@@ -266,16 +266,7 @@ too deeply).
    [32 x i8] c"..." ; Encoded data (dissected later)
   }, section "__llvm_covmap", align 8
 
-The current version of the format is version 6.
-
-There is one difference between versions 6 and 5:
-
-* The first entry in the filename list is the compilation directory. When the
-  filename is relative, the compilation directory is combined with the relative
-  path to get an absolute path. This can reduce size by omitting the duplicate
-  prefix in filenames.
-
-There is one difference between versions 5 and 4:
+The current version of the format is version 5. There is one difference from version 4:
 
 * The notion of branch region has been introduced along with a corresponding
   region kind.  Branch regions encode two counters, one to track how many
diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
@@ -1003,10 +1003,7 @@ enum CovMapVersion {
   Version4 = 3,
   // Branch regions referring to two counters are added
   Version5 = 4,
-  // Compilation directory is stored separately and combined with relative
-  // filenames to produce an absolute file path.
-  Version6 = 5,
-  // The current version is Version6.
+  // The current version is Version5.
   CurrentVersion = INSTR_PROF_COVMAP_VERSION
 };
 
diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMappingReader.h
@@ -130,14 +130,14 @@ class RawCoverageMappingDummyChecker : public RawCoverageReader {
 
 /// Reader for the raw coverage mapping data.
 class RawCoverageMappingReader : public RawCoverageReader {
-  ArrayRef<std::string> &TranslationUnitFilenames;
+  ArrayRef<StringRef> TranslationUnitFilenames;
   std::vector<StringRef> &Filenames;
   std::vector<CounterExpression> &Expressions;
   std::vector<CounterMappingRegion> &MappingRegions;
 
 public:
   RawCoverageMappingReader(StringRef MappingData,
-                           ArrayRef<std::string> &TranslationUnitFilenames,
+                           ArrayRef<StringRef> TranslationUnitFilenames,
                            std::vector<StringRef> &Filenames,
                            std::vector<CounterExpression> &Expressions,
                            std::vector<CounterMappingRegion> &MappingRegions)
@@ -179,10 +179,12 @@ class BinaryCoverageReader : public CoverageMappingReader {
           FilenamesBegin(FilenamesBegin), FilenamesSize(FilenamesSize) {}
   };
 
+  using DecompressedData = std::vector<std::unique_ptr<SmallVector<char, 0>>>;
+
   using FuncRecordsStorage = std::unique_ptr<MemoryBuffer>;
 
 private:
-  std::vector<std::string> Filenames;
+  std::vector<StringRef> Filenames;
   std::vector<ProfileMappingRecord> MappingRecords;
   InstrProfSymtab ProfileNames;
   size_t CurrentRecord = 0;
@@ -195,6 +197,10 @@ class BinaryCoverageReader : public CoverageMappingReader {
   // D69471, which can split up function records into multiple sections on ELF.
   FuncRecordsStorage FuncRecords;
 
+  // Used to tie the lifetimes of decompressed strings to the lifetime of this
+  // BinaryCoverageReader instance.
+  DecompressedData Decompressed;
+
   BinaryCoverageReader(FuncRecordsStorage &&FuncRecords)
       : FuncRecords(std::move(FuncRecords)) {}
 
@@ -220,23 +226,20 @@ class BinaryCoverageReader : public CoverageMappingReader {
 
 /// Reader for the raw coverage filenames.
 class RawCoverageFilenamesReader : public RawCoverageReader {
-  std::vector<std::string> &Filenames;
-  StringRef CompilationDir;
+  std::vector<StringRef> &Filenames;
 
   // Read an uncompressed sequence of filenames.
-  Error readUncompressed(CovMapVersion Version, uint64_t NumFilenames);
+  Error readUncompressed(uint64_t NumFilenames);
 
 public:
-  RawCoverageFilenamesReader(StringRef Data,
-                             std::vector<std::string> &Filenames,
-                             StringRef CompilationDir = "")
-      : RawCoverageReader(Data), Filenames(Filenames),
-        CompilationDir(CompilationDir) {}
+  RawCoverageFilenamesReader(StringRef Data, std::vector<StringRef> &Filenames)
+      : RawCoverageReader(Data), Filenames(Filenames) {}
   RawCoverageFilenamesReader(const RawCoverageFilenamesReader &) = delete;
   RawCoverageFilenamesReader &
   operator=(const RawCoverageFilenamesReader &) = delete;
 
-  Error read(CovMapVersion Version);
+  Error read(CovMapVersion Version,
+             BinaryCoverageReader::DecompressedData &Decompressed);
 };
 
 } // end namespace coverage
diff --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMappingWriter.h b/llvm/include/llvm/ProfileData/Coverage/CoverageMappingWriter.h
@@ -27,10 +27,10 @@ namespace coverage {
 /// Writer of the filenames section for the instrumentation
 /// based code coverage.
 class CoverageFilenamesSectionWriter {
-  ArrayRef<std::string> Filenames;
+  ArrayRef<StringRef> Filenames;
 
 public:
-  CoverageFilenamesSectionWriter(ArrayRef<std::string> Filenames);
+  CoverageFilenamesSectionWriter(ArrayRef<StringRef> Filenames);
 
   /// Write encoded filenames to the given output stream. If \p Compress is
   /// true, attempt to compress the filenames.
diff --git a/llvm/include/llvm/ProfileData/InstrProfData.inc b/llvm/include/llvm/ProfileData/InstrProfData.inc
@@ -650,7 +650,7 @@ serializeValueProfDataFrom(ValueProfRecordClosure *Closure,
 /* Indexed profile format version (start from 1). */
 #define INSTR_PROF_INDEX_VERSION 7
 /* Coverage mapping format version (start from 0). */
-#define INSTR_PROF_COVMAP_VERSION 5
+#define INSTR_PROF_COVMAP_VERSION 4
 
 /* Profile version is always of type uint64_t. Reserve the upper 8 bits in the
  * version for other variants of profile. We set the lowest bit of the upper 8
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp b/llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
diff --git a/llvm/test/tools/llvm-cov/Inputs/binary-formats.v6.linux64l b/llvm/test/tools/llvm-cov/Inputs/binary-formats.v6.linux64l
diff --git a/llvm/test/tools/llvm-cov/binary-formats.c b/llvm/test/tools/llvm-cov/binary-formats.c
diff --git a/llvm/test/tools/llvm-cov/compilation_dir.c b/llvm/test/tools/llvm-cov/compilation_dir.c
diff --git a/llvm/unittests/ProfileData/CoverageMappingTest.cpp b/llvm/unittests/ProfileData/CoverageMappingTest.cpp
