Add support for zipped LLVM profile files.

This change is a follow-up to a recent change which allowed LLVM raw profile files
to be directly used with blaze.  This change allows zipped LLVM raw profile
files.

This uses //tools/zip:zipper to extract the zipped file contents.

This also adds a new option to //tools/zip:zipper, 'j', to junk directories while
unzipping.

Tested:
  blaze test //devtools/blaze/integration:fdo_test
  blaze test //third_party/ijar/test:zip_test

RELNOTES[NEW]: Zipped LLVM profiles are now supported.

PiperOrigin-RevId: 158849516

8 files changed, 160 insertions, 50 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java
index ea3deb880e..af4c3e5acb 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java
@@ -45,6 +45,7 @@ import com.google.devtools.build.lib.packages.License;
 import com.google.devtools.build.lib.rules.MakeVariableProvider;
 import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
 import com.google.devtools.build.lib.rules.cpp.FdoSupport.FdoException;
+import com.google.devtools.build.lib.util.FileType;
 import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
@@ -72,12 +73,16 @@ public class CcToolchain implements RuleConfiguredTargetFactory {
   private static final PathFragment BUILTIN_INCLUDE_FILE_SUFFIX =
       PathFragment.create("include/stdc-predef.h");
 
-  private static String getLLVMProfileFileName(Path fdoProfile) {
-    if (CppFileTypes.LLVM_PROFILE.matches(fdoProfile)) {
+  /*
+   * Returns the profile name with the same file name as fdoProfile and an
+   * extension that matches {@link FileType}.
+   */
+  private static String getLLVMProfileFileName(Path fdoProfile, FileType type) {
+    if (type.matches(fdoProfile)) {
       return fdoProfile.getBaseName();
     } else {
       return FileSystemUtils.removeExtension(fdoProfile.getBaseName())
-          + CppFileTypes.LLVM_PROFILE.getExtensions().get(0);
+          + type.getExtensions().get(0);
     }
   }
 
@@ -91,7 +96,9 @@ public class CcToolchain implements RuleConfiguredTargetFactory {
 
     Artifact profileArtifact =
         ruleContext.getUniqueDirectoryArtifact(
-            "fdo", getLLVMProfileFileName(fdoProfile), ruleContext.getBinOrGenfilesDirectory());
+            "fdo",
+            getLLVMProfileFileName(fdoProfile, CppFileTypes.LLVM_PROFILE),
+            ruleContext.getBinOrGenfilesDirectory());
 
     // If the profile file is already in the desired format, symlink to it and return.
     if (CppFileTypes.LLVM_PROFILE.matches(fdoProfile)) {
@@ -106,14 +113,52 @@ public class CcToolchain implements RuleConfiguredTargetFactory {
 
     Artifact rawProfileArtifact =
         ruleContext.getUniqueDirectoryArtifact(
-            "fdo", fdoProfile.getBaseName(), ruleContext.getBinOrGenfilesDirectory());
+            "fdo",
+            getLLVMProfileFileName(fdoProfile, CppFileTypes.LLVM_PROFILE_RAW),
+            ruleContext.getBinOrGenfilesDirectory());
+
+    if (fdoProfile.getBaseName().endsWith(".zip")) {
+      // Get the zipper binary for unzipping the profile.
+      Artifact zipperBinaryArtifact = ruleContext.getPrerequisiteArtifact(":zipper", Mode.HOST);
+      if (zipperBinaryArtifact == null) {
+        ruleContext.ruleError("Cannot find zipper binary to unzip the profile");
+        return null;
+      }
 
-    ruleContext.registerAction(
-        new SymlinkAction(
-            ruleContext.getActionOwner(),
-            PathFragment.create(fdoProfile.getPathString()),
-            rawProfileArtifact,
-            "Symlinking LLVM Profile " + fdoProfile.getPathString()));
+      // Symlink to the zipped profile file to extract the contents.
+      Artifact zipProfileArtifact =
+          ruleContext.getUniqueDirectoryArtifact(
+              "fdo", fdoProfile.getBaseName(), ruleContext.getBinOrGenfilesDirectory());
+      ruleContext.registerAction(
+          new SymlinkAction(
+              ruleContext.getActionOwner(),
+              PathFragment.create(fdoProfile.getPathString()),
+              zipProfileArtifact,
+              "Symlinking LLVM ZIP Profile " + fdoProfile.getPathString()));
+
+      // Unzip the profile.
+      ruleContext.registerAction(
+          new SpawnAction.Builder()
+              .addInput(zipProfileArtifact)
+              .addInput(zipperBinaryArtifact)
+              .addOutput(rawProfileArtifact)
+              .useDefaultShellEnvironment()
+              .setExecutable(zipperBinaryArtifact)
+              .addArguments("xf", zipProfileArtifact.getExecPathString())
+              .addArguments(
+                  "-d", rawProfileArtifact.getExecPath().getParentDirectory().getSafePathString())
+              .setProgressMessage(
+                  "LLVMUnzipProfileAction: Generating " + rawProfileArtifact.prettyPrint())
+              .setMnemonic("LLVMUnzipProfileAction")
+              .build(ruleContext));
+    } else {
+      ruleContext.registerAction(
+          new SymlinkAction(
+              ruleContext.getActionOwner(),
+              PathFragment.create(fdoProfile.getPathString()),
+              rawProfileArtifact,
+              "Symlinking LLVM Raw Profile " + fdoProfile.getPathString()));
+    }
 
     if (cppConfiguration.getLLVMProfDataExecutable() == null) {
       ruleContext.ruleError(
@@ -154,10 +199,12 @@ public class CcToolchain implements RuleConfiguredTargetFactory {
     Path fdoZip = ruleContext.getConfiguration().getCompilationMode() == CompilationMode.OPT
         ? cppConfiguration.getFdoZip()
         : null;
-    SkyKey fdoKey = FdoSupportValue.key(
-        cppConfiguration.getLipoMode(),
-        fdoZip,
-        cppConfiguration.getFdoInstrument());
+    SkyKey fdoKey =
+        FdoSupportValue.key(
+            cppConfiguration.getLipoMode(),
+            fdoZip,
+            cppConfiguration.getFdoInstrument(),
+            cppConfiguration.isLLVMOptimizedFdo());
 
     SkyFunction.Environment skyframeEnv = ruleContext.getAnalysisEnvironment().getSkyframeEnv();
     FdoSupportValue fdoSupport;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java
index f7414408f8..1fad6472b6 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java
@@ -58,6 +58,7 @@ public final class CcToolchainRule implements RuleDefinition {
 
   @Override
   public RuleClass build(Builder builder, RuleDefinitionEnvironment env) {
+    final Label zipper = env.getToolsLabel("//tools/zip:zipper");
     return builder
         .setUndocumented()
         .requiresConfigurationFragments(CppConfiguration.class)
@@ -85,6 +86,24 @@ public final class CcToolchainRule implements RuleDefinition {
                 .cfg(HOST)
                 .singleArtifact()
                 .value(env.getToolsLabel("//tools/cpp:link_dynamic_library")))
+        .add(
+            attr(":zipper", LABEL)
+                .cfg(HOST)
+                .singleArtifact()
+                .value(
+                    new LateBoundLabel<BuildConfiguration>() {
+                      @Override
+                      public Label resolve(
+                          Rule rule, AttributeMap attributes, BuildConfiguration configuration) {
+                        CppConfiguration cppConfiguration =
+                            configuration.getFragment(CppConfiguration.class);
+                        if (cppConfiguration.isLLVMOptimizedFdo()) {
+                          return zipper;
+                        } else {
+                          return null;
+                        }
+                      }
+                    }))
         .add(attr(":libc_top", LABEL).value(LIBC_TOP))
         .add(
             attr(":lipo_context_collector", LABEL)
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
index 1d50ed608c..617fce8bf2 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
@@ -1601,7 +1601,12 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
     return cppOptions.isFdo()
         && cppOptions.getFdoOptimize() != null
         && (CppFileTypes.LLVM_PROFILE.matches(cppOptions.getFdoOptimize())
-            || CppFileTypes.LLVM_PROFILE_RAW.matches(cppOptions.getFdoOptimize()));
+            || CppFileTypes.LLVM_PROFILE_RAW.matches(cppOptions.getFdoOptimize())
+            // TODO(tmsriram): Checking for "llvm" does not handle all the cases.  This
+            // is temporary until the crosstool configuration is modified to add fields that
+            // indicate which flavor of fdo is being used.
+            || (getToolchainIdentifier().contains("llvm")
+                && cppOptions.getFdoOptimize().endsWith(".zip")));
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupport.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupport.java
index 9bf74e93c5..781061b79b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupport.java
@@ -155,14 +155,6 @@ public class FdoSupport {
   }
 
   /**
-   * Returns true if the given fdoFile represents an LLVM profile.
-   */
-  public static final boolean isLLVMFdo(String fdoFile) {
-    return (CppFileTypes.LLVM_PROFILE.matches(fdoFile)
-        || CppFileTypes.LLVM_PROFILE_RAW.matches(fdoFile));
-  }
-
-  /**
    * Coverage information output directory passed to {@code --fdo_instrument},
    * or {@code null} if FDO instrumentation is disabled.
    */
@@ -262,12 +254,13 @@ public class FdoSupport {
       PathFragment fdoInstrument,
       Path fdoProfile,
       LipoMode lipoMode,
+      boolean llvmFdo,
       Path execRoot)
       throws IOException, FdoException, InterruptedException {
     FdoMode fdoMode;
     if (fdoProfile != null && isAutoFdo(fdoProfile.getBaseName())) {
       fdoMode = FdoMode.AUTO_FDO;
-    } else if (fdoProfile != null && isLLVMFdo(fdoProfile.getBaseName())) {
+    } else if (fdoProfile != null && llvmFdo) {
       fdoMode = FdoMode.LLVM_FDO;
     } else if (fdoProfile != null) {
       fdoMode = FdoMode.VANILLA;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportFunction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportFunction.java
index d4c838969e..0a2f326186 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportFunction.java
@@ -23,9 +23,7 @@ import com.google.devtools.build.skyframe.SkyFunctionException;
 import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
-
 import java.io.IOException;
-
 import javax.annotation.Nullable;
 
 /**
@@ -57,8 +55,14 @@ public class FdoSupportFunction implements SkyFunction {
     FdoSupportValue.Key key = (FdoSupportValue.Key) skyKey.argument();
     FdoSupport fdoSupport;
     try {
-      fdoSupport = FdoSupport.create(env,
-        key.getFdoInstrument(), key.getFdoZip(), key.getLipoMode(), execRoot);
+      fdoSupport =
+          FdoSupport.create(
+              env,
+              key.getFdoInstrument(),
+              key.getFdoZip(),
+              key.getLipoMode(),
+              key.getLLVMFdo(),
+              execRoot);
       if (env.valuesMissing()) {
         return null;
       }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportValue.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportValue.java
index 90ee9326b7..0f94dcd486 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportValue.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoSupportValue.java
@@ -44,11 +44,13 @@ public class FdoSupportValue implements SkyValue {
     private final LipoMode lipoMode;
     private final Path fdoZip;
     private final PathFragment fdoInstrument;
+    private final boolean llvmFdo;
 
-    private Key(LipoMode lipoMode, Path fdoZip, PathFragment fdoInstrument) {
+    private Key(LipoMode lipoMode, Path fdoZip, PathFragment fdoInstrument, boolean llvmFdo) {
       this.lipoMode = lipoMode;
       this.fdoZip = fdoZip;
       this.fdoInstrument = fdoInstrument;
+      this.llvmFdo = llvmFdo;
     }
 
     public LipoMode getLipoMode() {
@@ -63,6 +65,10 @@ public class FdoSupportValue implements SkyValue {
       return fdoInstrument;
     }
 
+    public boolean getLLVMFdo() {
+      return llvmFdo;
+    }
+
     @Override
     public boolean equals(Object o) {
       if (this == o) {
@@ -76,6 +82,7 @@ public class FdoSupportValue implements SkyValue {
       Key that = (Key) o;
       return Objects.equals(this.lipoMode, that.lipoMode)
           && Objects.equals(this.fdoZip, that.fdoZip)
+          && Objects.equals(this.llvmFdo, that.llvmFdo)
           && Objects.equals(this.fdoInstrument, that.fdoInstrument);
     }
 
@@ -95,7 +102,8 @@ public class FdoSupportValue implements SkyValue {
     return fdoSupport;
   }
 
-  public static SkyKey key(LipoMode lipoMode, Path fdoZip, PathFragment fdoInstrument) {
-    return LegacySkyKey.create(SKYFUNCTION, new Key(lipoMode, fdoZip, fdoInstrument));
+  public static SkyKey key(
+      LipoMode lipoMode, Path fdoZip, PathFragment fdoInstrument, boolean llvmFdo) {
+    return LegacySkyKey.create(SKYFUNCTION, new Key(lipoMode, fdoZip, fdoInstrument, llvmFdo));
   }
 }
diff --git a/third_party/ijar/test/zip_test.sh b/third_party/ijar/test/zip_test.sh
index d206a74ee6..d70fa8fcb0 100755
--- a/third_party/ijar/test/zip_test.sh
+++ b/third_party/ijar/test/zip_test.sh
@@ -80,8 +80,8 @@ function test_zipper() {
   # Test flatten option
   (cd ${TEST_TMPDIR}/test && $ZIPPER cf ${TEST_TMPDIR}/output.zip ${filelist})
   $ZIPPER v ${TEST_TMPDIR}/output.zip >$TEST_log
-  expect_log "file"
-  expect_log "other_file"
+  expect_log "^f .* file$"
+  expect_log "^f .* other_file$"
   expect_not_log "path"
   expect_not_log "/"
 
@@ -90,11 +90,31 @@ function test_zipper() {
   echo "abcdefghi" >${TEST_TMPDIR}/test.zip
   cat ${TEST_TMPDIR}/output.zip >>${TEST_TMPDIR}/test.zip
   $ZIPPER v ${TEST_TMPDIR}/test.zip >$TEST_log
-  expect_log "file"
-  expect_log "other_file"
+  expect_log "^f .* file$"
+  expect_log "^f .* other_file$"
   expect_not_log "path"
 }
 
+function test_zipper_junk_paths() {
+  mkdir -p ${TEST_TMPDIR}/test/path/to/some
+  mkdir -p ${TEST_TMPDIR}/test/some/other/path
+  touch ${TEST_TMPDIR}/test/path/to/some/empty_file
+  echo "toto" > ${TEST_TMPDIR}/test/path/to/some/file
+  echo "titi" > ${TEST_TMPDIR}/test/path/to/some/other_file
+  chmod +x ${TEST_TMPDIR}/test/path/to/some/other_file
+  echo "tata" > ${TEST_TMPDIR}/test/file
+  filelist="$(cd ${TEST_TMPDIR}/test && find . | sed 's|^./||' | grep -v '^.$')"
+
+  # Test extract + flatten option
+  (cd ${TEST_TMPDIR}/test && $ZIPPER c ${TEST_TMPDIR}/output.zip ${filelist})
+  $ZIPPER vf ${TEST_TMPDIR}/output.zip >$TEST_log
+  echo $TEST_log
+  expect_log "^f .* file$"
+  expect_log "^f .* other_file$"
+  expect_not_log "path"
+  expect_not_log "/"
+}
+
 function test_zipper_unzip_selective_files() {
   mkdir -p ${TEST_TMPDIR}/test/path/to/some
   mkdir -p ${TEST_TMPDIR}/test/some/other/path
diff --git a/third_party/ijar/zip_main.cc b/third_party/ijar/zip_main.cc
index 51aea54157..5639bc02a8 100644
--- a/third_party/ijar/zip_main.cc
+++ b/third_party/ijar/zip_main.cc
@@ -45,9 +45,11 @@ class UnzipProcessor : public ZipExtractorProcessor {
   // into output_root if "extract" is set to true and will print the list of
   // files and their unix modes if "verbose" is set to true.
   UnzipProcessor(const char *output_root, char **files, bool verbose,
-                 bool extract) : output_root_(output_root),
-                                 verbose_(verbose),
-                                 extract_(extract) {
+                 bool extract, bool flatten)
+      : output_root_(output_root),
+        verbose_(verbose),
+        extract_(extract),
+        flatten_(flatten) {
     if (files != NULL) {
       for (int i = 0; files[i] != NULL; i++) {
         file_names.insert(std::string(files[i]));
@@ -73,6 +75,7 @@ class UnzipProcessor : public ZipExtractorProcessor {
   const char *output_root_;
   const bool verbose_;
   const bool extract_;
+  const bool flatten_;
   std::set<std::string> file_names;
 };
 
@@ -98,17 +101,29 @@ void UnzipProcessor::Process(const char* filename, const u4 attr,
                              const u1* data, const size_t size) {
   mode_t perm = zipattr_to_perm(attr);
   bool isdir = zipattr_is_dir(attr);
+  const char *output_file_name = filename;
   if (attr == 0) {
     // Fallback when the external attribute is not set.
     isdir = filename[strlen(filename)-1] == '/';
     perm = 0777;
   }
+
+  if (flatten_) {
+    if (isdir) {
+      return;
+    }
+    const char *p = strrchr(filename, '/');
+    if (p != NULL) {
+      output_file_name = p + 1;
+    }
+  }
+
   if (verbose_) {
-    printf("%c %o %s\n", isdir ? 'd' : 'f', perm, filename);
+    printf("%c %o %s\n", isdir ? 'd' : 'f', perm, output_file_name);
   }
   if (extract_) {
     char path[PATH_MAX];
-    concat_path(path, PATH_MAX, output_root_, filename);
+    concat_path(path, PATH_MAX, output_root_, output_file_name);
     if (!make_dirs(path, perm) ||
         (!isdir && !write_file(path, perm, data, size))) {
       abort();
@@ -130,8 +145,8 @@ void basename(const char *path, char *output, size_t output_size) {
 }
 
 // Execute the extraction (or just listing if just v is provided)
-int extract(char *zipfile, char* exdir, char **files, bool verbose,
-            bool extract) {
+int extract(char *zipfile, char *exdir, char **files, bool verbose,
+            bool extract, bool flatten) {
   std::string cwd = get_cwd();
   if (cwd.empty()) {
     return -1;
@@ -144,7 +159,7 @@ int extract(char *zipfile, char* exdir, char **files, bool verbose,
     strncpy(output_root, cwd.c_str(), PATH_MAX);
   }
 
-  UnzipProcessor processor(output_root, files, verbose, extract);
+  UnzipProcessor processor(output_root, files, verbose, extract, flatten);
   std::unique_ptr<ZipExtractor> extractor(ZipExtractor::Create(zipfile,
                                                                &processor));
   if (extractor.get() == NULL) {
@@ -346,7 +361,9 @@ static void usage(char *progname) {
           "    an optional directory relative to the current directory "
           "    specified through -d option\n");
   fprintf(stderr, "  c create  - add files to x.zip\n");
-  fprintf(stderr, "  f flatten - flatten files to use with create operation\n");
+  fprintf(stderr,
+          "  f flatten - flatten files to use with create or "
+          "extract operation\n");
   fprintf(stderr,
           "  C compress - compress files when using the create operation\n");
   fprintf(stderr, "x and c cannot be used in the same command-line.\n");
@@ -438,16 +455,13 @@ int main(int argc, char **argv) {
     // Create a zip
     return devtools_ijar::create(argv[2], filelist, flatten, verbose, compress);
   } else {
-    if (flatten) {
-      usage(argv[0]);
-    }
-
     char* exdir = NULL;
     if (argc > 3 && strcmp(argv[3], "-d") == 0) {
       exdir = argv[4];
     }
 
     // Extraction / list mode
-    return devtools_ijar::extract(argv[2], exdir, filelist, verbose, extract);
+    return devtools_ijar::extract(argv[2], exdir, filelist, verbose, extract,
+                                  flatten);
   }
 }