From c449a8211039f9145c111daa4697721ecbe18dec Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 13 Dec 2022 07:09:26 -0800 Subject: [PATCH] Internal change PiperOrigin-RevId: 495012726 Change-Id: Ib02f5c6f844fa42b57e249dc37cd0f413626c8e0 --- .../lib/analysis/test/CoverageCommon.java | 24 ++++++--- .../build/lib/rules/cpp/CcCommon.java | 28 ----------- .../lib/rules/cpp/CcCompilationHelper.java | 6 +++ .../lib/rules/cpp/CcCompilationOutputs.java | 50 +++++++++++++++++++ .../build/lib/rules/cpp/CppConfiguration.java | 1 + .../cpp/CcCompilationOutputsApi.java | 6 +++ .../cpp/CppConfigurationApi.java | 7 ++- .../test/CoverageCommonApi.java | 10 ++++ .../builtins_bzl/common/cc/cc_helper.bzl | 40 +++++++++++++++ .../builtins_bzl/common/cc/cc_library.bzl | 11 ++-- 10 files changed, 140 insertions(+), 43 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/CoverageCommon.java b/src/main/java/com/google/devtools/build/lib/analysis/test/CoverageCommon.java index 545cb17ced8a03..576697978c58fe 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/CoverageCommon.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/CoverageCommon.java @@ -56,6 +56,7 @@ public InstrumentedFilesInfoApi instrumentedFilesInfo( supportFiles, // Depset|Sequence|FilesToRunProvider> Dict environment, // Object extensions, + Sequence metadataFiles, // Sequence StarlarkThread thread) throws EvalException, TypeException { List extensionsList = @@ -96,13 +97,17 @@ public InstrumentedFilesInfoApi instrumentedFilesInfo( if (!supportFilesBuilder.isEmpty() || !environmentPairs.isEmpty()) { BuiltinRestriction.throwIfNotBuiltinUsage(thread); } + if (!metadataFiles.isEmpty()) { + BuiltinRestriction.throwIfNotBuiltinUsage(thread); + } return createInstrumentedFilesInfo( starlarkRuleContext.getRuleContext(), Sequence.cast(sourceAttributes, String.class, "source_attributes"), Sequence.cast(dependencyAttributes, String.class, "dependency_attributes"), supportFilesBuilder.build(), NestedSetBuilder.wrap(Order.COMPILE_ORDER, environmentPairs), - extensionsList); + extensionsList, + Sequence.cast(metadataFiles, Artifact.class, "metadata_files")); } /** @@ -130,7 +135,8 @@ public static InstrumentedFilesInfo createInstrumentedFilesInfo( dependencyAttributes, NestedSetBuilder.emptySet(Order.STABLE_ORDER), NestedSetBuilder.emptySet(Order.STABLE_ORDER), - extensions); + extensions, + null); } private static InstrumentedFilesInfo createInstrumentedFilesInfo( @@ -139,7 +145,8 @@ private static InstrumentedFilesInfo createInstrumentedFilesInfo( List dependencyAttributes, NestedSet supportFiles, NestedSet> environment, - @Nullable List extensions) { + @Nullable List extensions, + @Nullable List metadataFiles) { FileTypeSet fileTypeSet = FileTypeSet.ANY_FILE; if (extensions != null) { if (extensions.isEmpty()) { @@ -158,11 +165,12 @@ private static InstrumentedFilesInfo createInstrumentedFilesInfo( ruleContext, instrumentationSpec, InstrumentedFilesCollector.NO_METADATA_COLLECTOR, - /* rootFiles = */ ImmutableList.of(), - /* coverageSupportFiles = */ supportFiles, - /* coverageEnvironment = */ environment, - /* withBaselineCoverage = */ !TargetUtils.isTestRule(ruleContext.getTarget()), - /* reportedToActualSources= */ NestedSetBuilder.create(Order.STABLE_ORDER)); + /* rootFiles= */ ImmutableList.of(), + /* coverageSupportFiles= */ supportFiles, + /* coverageEnvironment= */ environment, + /* withBaselineCoverage= */ !TargetUtils.isTestRule(ruleContext.getTarget()), + /* reportedToActualSources= */ NestedSetBuilder.create(Order.STABLE_ORDER), + /* additionalMetadata= */ metadataFiles); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java index 3febb6bf7925ee..fe563bb16158ab 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java @@ -46,7 +46,6 @@ import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; -import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.RuleClass; @@ -779,23 +778,6 @@ Artifact getDefParser() { return ruleContext.getPrerequisiteArtifact("$def_parser"); } - @StarlarkMethod( - name = "instrumented_files_info", - documented = false, - parameters = { - @Param(name = "files", positional = false, named = true), - @Param(name = "with_base_line_coverage", positional = false, named = true), - }) - public InstrumentedFilesInfo getInstrumentedFilesProviderForStarlark( - Sequence files, boolean withBaselineCoverage) throws EvalException { - try { - return getInstrumentedFilesProvider( - Sequence.cast(files, Artifact.class, "files"), withBaselineCoverage); - } catch (RuleErrorException e) { - throw new EvalException(e); - } - } - @StarlarkMethod( name = "instrumented_files_info_from_compilation_context", documented = false, @@ -827,16 +809,6 @@ public InstrumentedFilesInfo getInstrumentedFilesProviderFromCompilationContextF } } - /** Provides support for instrumentation. */ - public InstrumentedFilesInfo getInstrumentedFilesProvider( - Iterable files, boolean withBaselineCoverage) throws RuleErrorException { - return getInstrumentedFilesProvider( - files, - withBaselineCoverage, - /* virtualToOriginalHeaders= */ NestedSetBuilder.emptySet(Order.STABLE_ORDER), - /* additionalMetadata= */ null); - } - public InstrumentedFilesInfo getInstrumentedFilesProvider( Iterable files, boolean withBaselineCoverage, 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 index a6ebd8905ea1c2..8d05cd7e64283d 100644 --- 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 @@ -2012,6 +2012,9 @@ private ImmutableList createSourceAction( // Host targets don't produce .dwo files. result.addPicDwoFile(dwoFile); } + if (gcnoFile != null) { + result.addPicGcnoFile(gcnoFile); + } } if (generateNoPicAction) { @@ -2080,6 +2083,9 @@ private ImmutableList createSourceAction( // Host targets don't produce .dwo files. result.addDwoFile(noPicDwoFile); } + if (gcnoFile != null) { + result.addGcnoFile(gcnoFile); + } } return directOutputs.build(); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationOutputs.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationOutputs.java index a7c432d93bae7d..ddfcfae385b6b4 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationOutputs.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationOutputs.java @@ -60,6 +60,12 @@ public class CcCompilationOutputs implements CcCompilationOutputsApi { */ private final ImmutableList picDwoFiles; + /** All .gcno files built by the target, corresponding to .o outputs. */ + private final ImmutableList gcnoFiles; + + /** All .pic.gcno files built by the target, corresponding to .pic.gcno outputs. */ + private final ImmutableList picGcnoFiles; + /** * All artifacts that are created if "--save_temps" is true. */ @@ -76,6 +82,8 @@ private CcCompilationOutputs( LtoCompilationContext ltoCompilationContext, ImmutableList dwoFiles, ImmutableList picDwoFiles, + ImmutableList gcnoFiles, + ImmutableList picGcnoFiles, NestedSet temps, ImmutableList headerTokenFiles) { this.objectFiles = objectFiles; @@ -83,6 +91,8 @@ private CcCompilationOutputs( this.ltoCompilationContext = ltoCompilationContext; this.dwoFiles = dwoFiles; this.picDwoFiles = picDwoFiles; + this.gcnoFiles = gcnoFiles; + this.picGcnoFiles = picGcnoFiles; this.temps = temps; this.headerTokenFiles = headerTokenFiles; } @@ -170,6 +180,28 @@ public ImmutableList getPicDwoFiles() { return picDwoFiles; } + @Override + public Sequence getStarlarkGcnoFiles(StarlarkThread thread) throws EvalException { + CcModule.checkPrivateStarlarkificationAllowlist(thread); + return StarlarkList.immutableCopyOf(getGcnoFiles()); + } + + @Override + public Sequence getStarlarkPicGcnoFiles(StarlarkThread thread) throws EvalException { + CcModule.checkPrivateStarlarkificationAllowlist(thread); + return StarlarkList.immutableCopyOf(getPicGcnoFiles()); + } + + /** Returns an unmodifiable view of the .gcno files set. */ + public ImmutableList getGcnoFiles() { + return gcnoFiles; + } + + /** Returns an unmodifiable view of the .pic.gcno files set. */ + public ImmutableList getPicGcnoFiles() { + return picGcnoFiles; + } + /** * Returns an unmodifiable view of the temp files set. */ @@ -207,6 +239,8 @@ public static final class Builder { new LtoCompilationContext.Builder(); private final Set dwoFiles = new LinkedHashSet<>(); private final Set picDwoFiles = new LinkedHashSet<>(); + private final Set gcnoFiles = new LinkedHashSet<>(); + private final Set picGcnoFiles = new LinkedHashSet<>(); private final NestedSetBuilder temps = NestedSetBuilder.stableOrder(); private final Set headerTokenFiles = new LinkedHashSet<>(); @@ -221,6 +255,8 @@ public CcCompilationOutputs build() { ltoCompilationContext.build(), ImmutableList.copyOf(dwoFiles), ImmutableList.copyOf(picDwoFiles), + ImmutableList.copyOf(gcnoFiles), + ImmutableList.copyOf(picGcnoFiles), temps.build(), ImmutableList.copyOf(headerTokenFiles)); } @@ -231,6 +267,8 @@ public Builder merge(CcCompilationOutputs outputs) { this.picObjectFiles.addAll(outputs.picObjectFiles); this.dwoFiles.addAll(outputs.dwoFiles); this.picDwoFiles.addAll(outputs.picDwoFiles); + this.gcnoFiles.addAll(outputs.gcnoFiles); + this.picGcnoFiles.addAll(outputs.picGcnoFiles); this.temps.addTransitive(outputs.temps); this.headerTokenFiles.addAll(outputs.headerTokenFiles); this.ltoCompilationContext.addAll(outputs.ltoCompilationContext); @@ -301,6 +339,18 @@ public Builder addPicDwoFile(Artifact artifact) { return this; } + @CanIgnoreReturnValue + public Builder addGcnoFile(Artifact artifact) { + gcnoFiles.add(artifact); + return this; + } + + @CanIgnoreReturnValue + public Builder addPicGcnoFile(Artifact artifact) { + picGcnoFiles.add(artifact); + return this; + } + /** Adds temp files. */ @CanIgnoreReturnValue public Builder addTemps(Iterable artifacts) { 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 bd3d3b0409ff2d..862cbe771ee7ee 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 @@ -882,6 +882,7 @@ public boolean generateLlvmLcovStarlark(StarlarkThread thread) throws EvalExcept return generateLlvmLCov(); } + @Nullable @Override public String fdoInstrumentStarlark(StarlarkThread thread) throws EvalException { checkInExpandedApiAllowlist(thread, "fdo_instrument"); diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/cpp/CcCompilationOutputsApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/cpp/CcCompilationOutputsApi.java index 17ddad12773091..75286805ad6778 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/cpp/CcCompilationOutputsApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/cpp/CcCompilationOutputsApi.java @@ -82,4 +82,10 @@ Depset getStarlarkFilesToCompile(boolean parseHeaders, boolean usePic, StarlarkT @StarlarkMethod(name = "pic_dwo_files", documented = false, useStarlarkThread = true) Sequence getStarlarkPicDwoFiles(StarlarkThread thread) throws EvalException; + + @StarlarkMethod(name = "gcno_files", documented = false, useStarlarkThread = true) + Sequence getStarlarkGcnoFiles(StarlarkThread thread) throws EvalException; + + @StarlarkMethod(name = "pic_gcno_files", documented = false, useStarlarkThread = true) + Sequence getStarlarkPicGcnoFiles(StarlarkThread thread) throws EvalException; } diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/cpp/CppConfigurationApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/cpp/CppConfigurationApi.java index 4bbc9cb11a9733..deed3df1c47a21 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/cpp/CppConfigurationApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/cpp/CppConfigurationApi.java @@ -130,7 +130,12 @@ public interface CppConfigurationApi expected Dict environment, // Object extensions, + Sequence metadataFiles, StarlarkThread thread) throws EvalException, TypeException; } diff --git a/src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl b/src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl index a21c546c066298..cb9ecd148f14b7 100644 --- a/src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl +++ b/src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl @@ -21,6 +21,7 @@ cc_common = _builtins.toplevel.cc_common cc_internal = _builtins.internal.cc_internal CcNativeLibraryInfo = _builtins.internal.CcNativeLibraryInfo config_common = _builtins.toplevel.config_common +coverage_common = _builtins.toplevel.coverage_common platform_common = _builtins.toplevel.platform_common artifact_category = struct( @@ -1165,6 +1166,43 @@ def _system_include_dirs(ctx, additional_make_variable_substitutions): result.append(_get_relative(ctx.bin_dir.path, out_includes_path)) return result +def _get_coverage_environment(ctx, cc_config, cc_toolchain): + if not ctx.configuration.coverage_enabled: + return {} + env = { + "COVERAGE_GCOV_PATH": cc_toolchain.tool_path(tool = "GCOV"), + "LLVM_COV": cc_toolchain.tool_path(tool = "LLVM_COV"), + "LLVM_PROFDATA": cc_toolchain.tool_path(tool = "LLVM_PROFDATA"), + "GENERATE_LLVM_LCOV": "1" if cc_config.generate_llvm_lcov() else "0", + } + for k in list(env.keys()): + if env[k] == None: + env[k] = "" + if cc_config.fdo_instrument(): + env["FDO_DIR"] = cc_config.fdo_instrument() + return env + +def _create_cc_instrumented_files_info(ctx, cc_config, cc_toolchain, metadata_files): + extensions = CC_SOURCE + \ + C_SOURCE + \ + CC_HEADER + \ + ASSESMBLER_WITH_C_PREPROCESSOR + \ + ASSEMBLER + coverage_environment = {} + if ctx.coverage_instrumented(): + coverage_environment = _get_coverage_environment(ctx, cc_config, cc_toolchain) + coverage_support_files = cc_toolchain.coverage_files() if ctx.coverage_instrumented() else depset([]) + info = coverage_common.instrumented_files_info( + ctx = ctx, + source_attributes = ["srcs", "hdrs"], + dependency_attributes = ["implementation_deps", "deps", "data"], + extensions = extensions, + metadata_files = metadata_files, + coverage_support_files = coverage_support_files, + coverage_environment = coverage_environment, + ) + return info + cc_helper = struct( merge_cc_debug_contexts = _merge_cc_debug_contexts, is_code_coverage_enabled = _is_code_coverage_enabled, @@ -1217,4 +1255,6 @@ cc_helper = struct( get_public_hdrs = _get_public_hdrs, report_invalid_options = _report_invalid_options, system_include_dirs = _system_include_dirs, + get_coverage_environment = _get_coverage_environment, + create_cc_instrumented_files_info = _create_cc_instrumented_files_info, ) diff --git a/src/main/starlark/builtins_bzl/common/cc/cc_library.bzl b/src/main/starlark/builtins_bzl/common/cc/cc_library.bzl index 8fd2ff04416feb..66b5818433e1f6 100755 --- a/src/main/starlark/builtins_bzl/common/cc/cc_library.bzl +++ b/src/main/starlark/builtins_bzl/common/cc/cc_library.bzl @@ -256,12 +256,11 @@ def _cc_library_impl(ctx): elif artifacts_to_build.interface_library != None: files_builder.append(artifacts_to_build.interface_library) - instrumented_object_files = [] - instrumented_object_files.extend(compilation_outputs.objects) - instrumented_object_files.extend(compilation_outputs.pic_objects) - instrumented_files_info = common.instrumented_files_info( - files = instrumented_object_files, - with_base_line_coverage = True, + instrumented_files_info = cc_helper.create_cc_instrumented_files_info( + ctx = ctx, + cc_config = ctx.fragments.cpp, + cc_toolchain = cc_toolchain, + metadata_files = compilation_outputs.gcno_files() + compilation_outputs.pic_gcno_files(), ) runfiles_list = []