[7.2.0] Stop listing .gcno files as outputs when using LLVM coverag…

…e map format When using `--experimental_use_llvm_covmap`, no `.gcno` files are output. By listing them as outputs there is a mismatch between expected and actual outputs. Bazel will create these empty files after the action is run, but they non-deterministically get counted in the `Action` outputs, which can lead to cache misses on subsequent runs. Closes bazelbuild#22132. PiperOrigin-RevId: 630996820 Change-Id: Ia7de7a05305095c8befee4f86181cf84d9737814
c-mita · May 7, 2024 · d4d1a1c · d4d1a1c
1 parent c753aac
commit d4d1a1c
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 3 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
@@ -1153,6 +1153,7 @@ private Artifact createCompileActionTemplate(
             usePic,
             /* needsFdoBuildVariables= */ false,
             ccCompilationContext.getCppModuleMap(),
+            /* enableCoverage= */ false,
             /* gcnoFile= */ null,
             /* isUsingFission= */ false,
             /* dwoFile= */ null,
@@ -1262,6 +1263,7 @@ private CcToolchainVariables setupCompileBuildVariables(
       boolean usePic,
       boolean needsFdoBuildVariables,
       CppModuleMap cppModuleMap,
+      boolean enableCoverage,
       Artifact gcnoFile,
       boolean isUsingFission,
       Artifact dwoFile,
@@ -1348,6 +1350,7 @@ private CcToolchainVariables setupCompileBuildVariables(
         buildVariables,
         toPathString(sourceFile),
         toPathString(builder.getOutputFile()),
+        enableCoverage,
         toPathString(gcnoFile),
         toPathString(dwoFile),
         isUsingFission,
@@ -1409,7 +1412,7 @@ private void createModuleCodegenAction(
             ccToolchain, ArtifactCategory.COVERAGE_DATA_FILE, outputName);
     // TODO(djasper): This is now duplicated. Refactor the various create..Action functions.
     Artifact gcnoFile =
-        isCodeCoverageEnabled
+        isCodeCoverageEnabled && !cppConfiguration.useLLVMCoverageMapFormat()
             ? CppHelper.getCompileOutputArtifact(
                 actionConstructionContext, label, gcnoFileName, configuration)
             : null;
@@ -1431,6 +1434,7 @@ private void createModuleCodegenAction(
             /* usePic= */ pic,
             /* needsFdoBuildVariables= */ ccRelativeName != null,
             ccCompilationContext.getCppModuleMap(),
+            isCodeCoverageEnabled,
             gcnoFile,
             generateDwo,
             dwoFile,
@@ -1480,6 +1484,7 @@ private void createHeaderAction(
             generatePicAction,
             /* needsFdoBuildVariables= */ false,
             ccCompilationContext.getCppModuleMap(),
+            /* enableCoverage= */ false,
             /* gcnoFile= */ null,
             /* isUsingFission= */ false,
             /* dwoFile= */ null,
@@ -1547,7 +1552,7 @@ private ImmutableList<Artifact> createSourceAction(
           CppHelper.getArtifactNameForCategory(
               ccToolchain, ArtifactCategory.COVERAGE_DATA_FILE, picOutputBase);
       Artifact gcnoFile =
-          enableCoverage
+          enableCoverage && !cppConfiguration.useLLVMCoverageMapFormat()
               ? CppHelper.getCompileOutputArtifact(
                   actionConstructionContext, label, gcnoFileName, configuration)
               : null;
@@ -1564,6 +1569,7 @@ private ImmutableList<Artifact> createSourceAction(
               /* usePic= */ true,
               /* needsFdoBuildVariables= */ ccRelativeName != null && addObject,
               cppModuleMap,
+              enableCoverage,
               gcnoFile,
               generateDwo,
               dwoFile,
@@ -1624,7 +1630,7 @@ private ImmutableList<Artifact> createSourceAction(
 
       // Create no-PIC compile actions
       Artifact gcnoFile =
-          enableCoverage
+          isCodeCoverageEnabled && !cppConfiguration.useLLVMCoverageMapFormat()
               ? CppHelper.getCompileOutputArtifact(
                   actionConstructionContext, label, gcnoFileName, configuration)
               : null;
@@ -1640,6 +1646,7 @@ private ImmutableList<Artifact> createSourceAction(
               /* usePic= */ false,
               /* needsFdoBuildVariables= */ ccRelativeName != null,
               cppModuleMap,
+              isCodeCoverageEnabled,
               gcnoFile,
               generateDwo,
               noPicDwoFile,
@@ -1810,6 +1817,7 @@ private ImmutableList<Artifact> createTempsActions(
             usePic,
             /* needsFdoBuildVariables= */ ccRelativeName != null,
             ccCompilationContext.getCppModuleMap(),
+            /* enableCoverage= */ false,
             /* gcnoFile= */ null,
             /* isUsingFission= */ false,
             /* dwoFile= */ null,
@@ -1837,6 +1845,7 @@ private ImmutableList<Artifact> createTempsActions(
             usePic,
             /* needsFdoBuildVariables= */ ccRelativeName != null,
             ccCompilationContext.getCppModuleMap(),
+            /* enableCoverage= */ false,
             /* gcnoFile= */ null,
             /* isUsingFission= */ false,
             /* dwoFile= */ null,

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java
@@ -348,6 +348,7 @@ public CcToolchainVariables getCompileBuildVariables(
                         .getCppConfigurationFromFeatureConfigurationCreatedForStarlark_andIKnowWhatImDoing(),
                     convertFromNoneable(sourceFile, /* defaultValue= */ null),
                     convertFromNoneable(outputFile, /* defaultValue= */ null),
+                    /* isCodeCoverageEnabled= */ false,
                     /* gcnoFile= */ null,
                     /* isUsingFission= */ false,
                     /* dwoFile= */ null,

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java
@@ -147,6 +147,7 @@ public static CcToolchainVariables setupVariablesOrReportRuleError(
       CppConfiguration cppConfiguration,
       String sourceFile,
       String outputFile,
+      boolean isCodeCoverageEnabled,
       String gcnoFile,
       boolean isUsingFission,
       String dwoFile,
@@ -179,6 +180,7 @@ public static CcToolchainVariables setupVariablesOrReportRuleError(
           ccToolchainProvider.getBuildVariables(thread, buildOptions, cppConfiguration),
           sourceFile,
           outputFile,
+          isCodeCoverageEnabled,
           gcnoFile,
           isUsingFission,
           dwoFile,
@@ -216,6 +218,7 @@ public static CcToolchainVariables setupVariablesOrThrowEvalException(
       CppConfiguration cppConfiguration,
       String sourceFile,
       String outputFile,
+      boolean isCodeCoverageEnabled,
       String gcnoFile,
       boolean isUsingFission,
       String dwoFile,
@@ -250,6 +253,7 @@ public static CcToolchainVariables setupVariablesOrThrowEvalException(
         ccToolchainProvider.getBuildVariables(thread, buildOptions, cppConfiguration),
         sourceFile,
         outputFile,
+        isCodeCoverageEnabled,
         gcnoFile,
         isUsingFission,
         dwoFile,
@@ -280,6 +284,7 @@ private static CcToolchainVariables setupVariables(
       CcToolchainVariables parent,
       String sourceFile,
       String outputFile,
+      boolean isCodeCoverageEnabled,
       String gcnoFile,
       boolean isUsingFission,
       String dwoFile,
@@ -323,6 +328,7 @@ private static CcToolchainVariables setupVariables(
         buildVariables,
         sourceFile,
         outputFile,
+        isCodeCoverageEnabled,
         gcnoFile,
         dwoFile,
         isUsingFission,
@@ -343,6 +349,7 @@ public static void setupSpecificVariables(
       CcToolchainVariables.Builder buildVariables,
       String sourceFile,
       String outputFile,
+      boolean isCodeCoverageEnabled,
       String gcnoFile,
       String dwoFile,
       boolean isUsingFission,
@@ -380,6 +387,10 @@ public static void setupSpecificVariables(
 
     if (gcnoFile != null) {
       buildVariables.addStringVariable(GCOV_GCNO_FILE.getVariableName(), gcnoFile);
+    } else if (isCodeCoverageEnabled) {
+      // TODO: Blaze currently uses `gcov_gcno_file` to detect if code coverage is enabled. It
+      // should use a different signal.
+      buildVariables.addStringVariable(GCOV_GCNO_FILE.getVariableName(), "");
     }
 
     if (dwoFile != null) {

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelper.java
@@ -167,6 +167,7 @@ private static CcToolchainVariables getVariables(
         cppConfiguration,
         sourceFile.getExecPathString(),
         outputFile.getExecPathString(),
+        /* isCodeCoverageEnabled= */ false,
         /* gcnoFile= */ null,
         /* isUsingFission= */ false,
         /* dwoFile= */ null,