[llvm-cov] Allow multiple remaps in --path-equivalence

[tcamin]

Summary

Previously the --path-equivalence parameter would allow to specify a single remap pair (coverage data path - local source file path). This patch changes this allowing to pass as many remaps as needed.

[vsk]

Hi, could you say a little bit about the use case(s) for this?

Also, how would multiple path remaps interact with each other?

[tcamin]

Hi,

Thanks for taking a look at this.

Our application is split into different modules which get built as separate libraries. These libraries are prebuilt and are shared among our devs as a build cache to reduce compilation times. These libraries do not necessarily share a single map between build path and source code location and therefore we’re unable to get coverage data out all of them.

Our requirement is being able to extract coverage data when running integration tests.

Multiple remaps are evaluated in order and are not applied more than once to a file (see line 490).

[keith]

Can you add a test for this? Best if it encodes the multiple remapping interactions above as well

[vsk]

In D105139#2856196, @tcamin wrote:

Hi,

Thanks for taking a look at this.

Our application is split into different modules which get built as separate libraries. These libraries are prebuilt and are shared among our devs as a build cache to reduce compilation times. These libraries do not necessarily share a single map between build path and source code location and therefore we’re unable to get coverage data out all of them.

Our requirement is being able to extract coverage data when running integration tests.

Thanks for explaining. Would it be practical to invoke llvm-cov multiple times (once per {library, path-remap} pair)? Is the issue with this that you would have separate reports per library?

[tcamin]

In D105139#2871524, @vsk wrote:

In D105139#2856196, @tcamin wrote:

Hi,

Thanks for taking a look at this.

Our application is split into different modules which get built as separate libraries. These libraries are prebuilt and are shared among our devs as a build cache to reduce compilation times. These libraries do not necessarily share a single map between build path and source code location and therefore we’re unable to get coverage data out all of them.

Our requirement is being able to extract coverage data when running integration tests.

Thanks for explaining. Would it be practical to invoke llvm-cov multiple times (once per {library, path-remap} pair)? Is the issue with this that you would have separate reports per library?

Sorry for the very late update. As far as I understood running llvm-cov multiple times would not work for us. We are trying to extract coverage from UI tests (integration tests) which are run on our iOS application. At compile time the app, to improve build times, is linked against prebuilt frameworks of our modules. We need to extract a single coverage result which covers all the source code of our app along with the source code of our separate prebuilt modules. Hope this makes sense.

[tcamin]

• Allow multiple remaps in --path-equivalence
• Avoid creating a default constructed value

[tcamin]

Fix formatting

[vsk]

Seconding Keith's comment - please add a test.

Also, please describe the behavior change in docs/CommandGuide/llvm-cov.rst. In particular it'd help to clarify the behavior w.r.t multiple remaps. Say you have the following remaps:

/a -> /x
/a/b -> /y

Then (IIUC) a file at "/a/b/t.c" is remapped to "/x/b/t.c", not "/y/t.c", right? I think that's a surprising result, and is at least worth documenting, if not warning about.

llvm/tools/llvm-cov/CodeCoverage.cpp

836–849

Warn if the number of pairs isn't even?

[keith]

We discovered another use case where we'd actually like this, we compile some modules with vfs overlays, which need 1 path remapping, and normally, which need some other remapping, in that case today I think we have to pick or choose. In this case they also all end up in the same binary, so we can't split up llvm-cov invocations

[alanphipps]

Hi! This use case is something we've run into as well. Do you have a plan to update the patch with a test case?

[tcamin]

I added some tests and updated the docs

[tcamin]

Fix test

[tcamin]

Fix test

[tcamin]

Rebase

[tcamin]

This comment was removed by tcamin.

[tcamin]

Use --binary when generating patch

[tcamin]

This revision should be closed as it has been superseded by https://reviews.llvm.org/D154223. Please refer to the updated revision for the latest changes and progress. Thank you

llvm/docs/CommandGuide/llvm-cov.rst

  Show code coverage only for functions with region coverage greater than the
  given threshold.
 
 .. option:: -region-coverage-lt=<N>
 
  Show code coverage only for functions with region coverage less than the given
  threshold.
 
-.. option:: -path-equivalence=<from>,<to>
+.. option:: -path-equivalence=<from>,<to>[,<from2>,<to2>[,...]]
 
  Map the paths in the coverage data to local source file paths. This allows you
  to generate the coverage data on one machine, and then use llvm-cov on a
- different machine where you have the same files on a different path.
+ different machine where you have the same files on a different path. Multiple
+ comma separated pairs can be passed and are applied in the order they are 
+ specified. If multiple mappings can be applied to a single path, the first 
+ mapping encountered is used.
 
 .. option:: -coverage-watermark=<high>,<low>
 
  Set high and low watermarks for coverage in html format output. This allows you
  to set the high and low watermark of coverage as desired, green when
  coverage >= high, red when coverage < low, and yellow otherwise. Both high and
  low should be between 0-100 and high > low.
 

llvm/test/tools/llvm-cov/Inputs/multiple-path_equivalence.proftext

+f1
+0x0
+1
+1
+
+f2
+0x0
+1
+1
+
+f3
+0x0
+1
+1
+
+f4
+0x0
+1
+1

llvm/test/tools/llvm-cov/multiple-path_equivalence.test

+# multiple-path_equivalence.covmapping contains references to 4 files:
+# /tmp/coverage/main.c
+# /tmp/coverage/f1.c
+# /tmp/coverage/a/f2.c
+# /tmp/coverage/b/f3.c
+# /tmp/coverage/b/c/f4.c
+
+# Setup
+// RUN: touch %/T/main.c; touch %/T/f1.c; touch %/T/f2.c; touch %/T/f3.c; touch %/T/f4.c
+// RUN: llvm-profdata merge %S/Inputs/multiple-path_equivalence.proftext -o %t.profdata
+
+# Make sure that remapping follows the specified order with the first matching entry being used first (f4 comes before f3)
+// RUN: llvm-cov show %S/Inputs/multiple-path_equivalence.covmapping -instr-profile=%t.profdata -path-equivalence=/tmp/coverage/a,%/T,/tmp/coverage/b/c,%/T,/tmp/coverage/b,%/T,/tmp/coverage,%/T 2>&1 | FileCheck %s
+
+// CHECK-DAG: /tmp/coverage/main.c:
+// CHECK-DAG: /tmp/coverage/f1.c:
+// CHECK-DAG: /tmp/coverage/a/f2.c:
+// CHECK-DAG: /tmp/coverage/b/f3.c:
+// CHECK-DAG: /tmp/coverage/b/c/f4.c:
+// CHECK-NOT: isn't covered.
+
+# Make sure remapping follows the specified order by proving paths in an overriding order (f4 comes after f3)
+// RUN: llvm-cov show %S/Inputs/multiple-path_equivalence.covmapping -instr-profile=%t.profdata -path-equivalence=/tmp/coverage/a,%/T,/tmp/coverage/b,%/T,/tmp/coverage/b/c,%/T,/tmp/coverage,%/T 2>&1 | FileCheck %s -check-prefix=OVERRIDING_ORDER
+
+// OVERRIDING_ORDER-DAG: /tmp/coverage/main.c:
+// OVERRIDING_ORDER-DAG: /tmp/coverage/f1.c:
+// OVERRIDING_ORDER-DAG: /tmp/coverage/a/f2.c:
+// OVERRIDING_ORDER-DAG: /tmp/coverage/b/f3.c:
+// OVERRIDING_ORDER-DAG: warning: The file '/tmp/coverage/b/c/f4.c' isn't covered.

llvm/test/tools/llvm-cov/path_equivalence.c

 // RUN: llvm-profdata merge %S/Inputs/path_equivalence.proftext -o %t.profdata
 // RUN: llvm-cov show %S/Inputs/path_equivalence.covmapping -instr-profile=%t.profdata -path-equivalence=/tmp,%S | FileCheck %s
 // RUN: llvm-cov show %S/Inputs/path_equivalence.covmapping -instr-profile=%t.profdata -path-equivalence=/tmp/,%S/ | FileCheck %s
 int main() {} // CHECK: [[@LINE]]|      1|int main() {}
 
 // RUN: not llvm-cov show --instr-profile=/dev/null -path-equivalence=foo /dev/null 2>&1 | FileCheck --check-prefix=INVALID %s
-// INVALID: error: -path-equivalence: invalid argument 'foo', must be in format 'from,to'
+// INVALID: error: -path-equivalence: invalid argument 'foo', must be a variable length sequence of comma separated path equivalence pairs '<from>,<to>[,<from2>,<to2>[,...]]'

llvm/tools/llvm-cov/CodeCoverage.cpp

private:
   std::vector<std::string> SourceFiles;
 
   /// In -path-equivalence mode, this maps the absolute paths from the coverage
   /// mapping data to the input source files.
   StringMap<std::string> RemappedFilenames;
 
   /// The coverage data path to be remapped from, and the source path to be
   /// remapped to, when using -path-equivalence.
-  std::optional<std::pair<std::string, std::string>> PathRemapping;
+  std::optional<std::vector<std::pair<std::string, std::string>>>
+      PathRemappings;
 
   /// File status cache used when finding the same file.
   StringMap<std::optional<sys::fs::file_status>> FileStatusCache;
 
   /// The architecture the coverage mapping data targets.
   std::vector<StringRef> CoverageArches;
 
   /// A cache for demangled symbols.
if (!IgnoreFilenameFilters.matchesFilename(EffectivePath))
     SourceFiles.emplace_back(EffectivePath.str());
   HadSourceFiles = !SourceFiles.empty();
 }
 
 void CodeCoverageTool::collectPaths(const std::string &Path) {
   llvm::sys::fs::file_status Status;
   llvm::sys::fs::status(Path, Status);
   if (!llvm::sys::fs::exists(Status)) {
-    if (PathRemapping)
+    if (PathRemappings)
       addCollectedPath(Path);
     else
       warning("Source file doesn't exist, proceeded by ignoring it.", Path);
     return;
   }
 
   if (llvm::sys::fs::is_regular_file(Status)) {
     addCollectedPath(Path);
if (!SourceFiles.empty())
     removeUnmappedInputs(*Coverage);
 
   demangleSymbols(*Coverage);
 
   return Coverage;
 }
 
 void CodeCoverageTool::remapPathNames(const CoverageMapping &Coverage) {
-  if (!PathRemapping)
+  if (!PathRemappings)
     return;
 
   // Convert remapping paths to native paths with trailing seperators.
   auto nativeWithTrailing = [](StringRef Path) -> std::string {
     if (Path.empty())
       return "";
     SmallString<128> NativePath;
     sys::path::native(Path, NativePath);
     sys::path::remove_dots(NativePath, true);
     if (!NativePath.empty() && !sys::path::is_separator(NativePath.back()))
       NativePath += sys::path::get_separator();
     return NativePath.c_str();
   };
-  std::string RemapFrom = nativeWithTrailing(PathRemapping->first);
-  std::string RemapTo = nativeWithTrailing(PathRemapping->second);
+
+  for (std::pair<std::string, std::string> &PathRemapping : *PathRemappings) {
+    std::string RemapFrom = nativeWithTrailing(PathRemapping.first);
+    std::string RemapTo = nativeWithTrailing(PathRemapping.second);
 
     // Create a mapping from coverage data file paths to local paths.
     for (StringRef Filename : Coverage.getUniqueSourceFiles()) {
+      if (RemappedFilenames.count(Filename) == 1)
+        continue;
+
       SmallString<128> NativeFilename;
       sys::path::native(Filename, NativeFilename);
       sys::path::remove_dots(NativeFilename, true);
       if (NativeFilename.startswith(RemapFrom)) {
         RemappedFilenames[Filename] =
             RemapTo + NativeFilename.substr(RemapFrom.size()).str();
       }
     }
+  }
 
   // Convert input files from local paths to coverage data file paths.
   StringMap<std::string> InvRemappedFilenames;
   for (const auto &RemappedFilename : RemappedFilenames)
     InvRemappedFilenames[RemappedFilename.getValue()] =
         std::string(RemappedFilename.getKey());
-
   for (std::string &Filename : SourceFiles) {
     SmallString<128> NativeFilename;
     sys::path::native(Filename, NativeFilename);
     auto CovFileName = InvRemappedFilenames.find(NativeFilename);
     if (CovFileName != InvRemappedFilenames.end())
       Filename = CovFileName->second;
   }
 }
cl::opt<CoverageViewOptions::OutputFormat> Format(
                  clEnumValN(CoverageViewOptions::OutputFormat::HTML, "html",
                             "HTML output"),
                  clEnumValN(CoverageViewOptions::OutputFormat::Lcov, "lcov",
                             "lcov tracefile output")),
       cl::init(CoverageViewOptions::OutputFormat::Text));
 
   cl::opt<std::string> PathRemap(
       "path-equivalence", cl::Optional,
-      cl::desc("<from>,<to> Map coverage data paths to local source file "
-               "paths"));
+      cl::desc("<from>,<to>[,<from2>,<to2>[,...]] Map coverage data paths to "
+               "local source file paths."));
 
   cl::OptionCategory FilteringCategory("Function filtering options");
 
   cl::list<std::string> NameFilters(
       "name", cl::Optional,
       cl::desc("Show code coverage only for functions with the given name"),
       cl::cat(FilteringCategory));
 
case CoverageViewOptions::OutputFormat::HTML:
       break;
     case CoverageViewOptions::OutputFormat::Lcov:
       if (UseColor == cl::BOU_TRUE)
         errs() << "Color output cannot be enabled when generating lcov.\n";
       ViewOpts.Colors = false;
       break;
     }
 
-    // If path-equivalence was given and is a comma seperated pair then set
-    // PathRemapping.
+    SmallVector<StringRef> EquivPairs;
+
+    // If path-equivalence was given and is a sequence of comma
+    // separated pairs then set PathRemapping.
     if (!PathRemap.empty()) {
-      auto EquivPair = StringRef(PathRemap).split(',');
-      if (EquivPair.first.empty() || EquivPair.second.empty()) {
+      StringRef(PathRemap).split(EquivPairs, ',');
+
+      if (EquivPairs.size() > 0 && EquivPairs.size() % 2 != 0) {
         error("invalid argument '" + PathRemap +
-                  "', must be in format 'from,to'",
+                  "', must be a variable length sequence of comma separated "
+                  "path equivalence pairs '<from>,<to>[,<from2>,<to2>[,...]]'",
               "-path-equivalence");
         return 1;
       }
 
-      PathRemapping = {std::string(EquivPair.first),
-                       std::string(EquivPair.second)};
+      std::vector<std::pair<std::string, std::string>> Remappings;
+      for (size_t I = 1; I < EquivPairs.size(); I += 2) {
+        if (std::string(EquivPairs[I - 1]).empty() ||
+            std::string(EquivPairs[I]).empty()) {
+          return 1;
+        }
+
+        Remappings.push_back(
+            {std::string(EquivPairs[I - 1]), std::string(EquivPairs[I])});
+      }
+      if (!Remappings.empty()) {
+        PathRemappings = Remappings;
+      }
     }
 
     // If a demangler is supplied, check if it exists and register it.
     if (!DemanglerOpts.empty()) {
       auto DemanglerPathOrErr = sys::findProgramByName(DemanglerOpts[0]);
       if (!DemanglerPathOrErr) {
         error("Could not find the demangler!",
               DemanglerPathOrErr.getError().message());
         return 1;