Skip to content

Commit

Permalink
Expose virtual:original header mappings via CcToolchain variables
Browse files Browse the repository at this point in the history
Whenever `prefix` and/or `strip_include_prefix` is passed into
`cc_library` rules, Bazel copies the headers into a special
"_virtual_includes" directory that allows for such path transformations
to occur.  Unfortunately, this also has the side-effect of changing how
source files are designated within the source binaries: they will refer
to the file in the `_virtual_includes` sandbox directory instead of the
actual files in the build tree.

This change allows us to work around this problem by providing a mapping between
the "virtual" and where they are in the filesystem as a
[CcToolchainConfigInfo variable][1], `virtual_to_original_dirs`, which ends up
being a list of `<virtual>=<original>` mappings that can be used in rules_cc
toolchain features, specifically using [-ffile-prefix-map][2].

Questions
=========
- Is `<virtual>=<original>` okay, or do we need a more sophisticated lazy
  expansion involving subvariables like what `libraries_to_link` has?
  E.g., do we imagine another compiler command-line flag format that'd
  require `A:B` instead of `A=B`?  (Then again, considering MSVC's ASan
  is implemented w/ `/fsanitize=address`, maybe they're on the `A=B`
  train now?)

Notes
=====
- We mostly mimicked coverage's `virtual_to_original_headers`.

Fixes upstream bazelbuild#11874.

[1]: https://docs.bazel.build/versions/master/cc-toolchain-config-reference.html#cctoolchainconfiginfo-build-variables
[2]: https://gcc.gnu.org/onlinedocs/gcc/Overall-Options.html#index-ffile-prefix-map

Co-authored-by: Andrew Psaltis <apsaltis@vmware.com>
  • Loading branch information
rbeasley and Andrew Psaltis committed Jan 31, 2024
1 parent 04b23c3 commit 855f858
Show file tree
Hide file tree
Showing 13 changed files with 240 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,16 @@ public final class CcCompilationContext implements CcCompilationContextApi<Artif
// This is needed only when code coverage collection is enabled, to report the actual source file
// name in the coverage output file.
private final NestedSet<Tuple> virtualToOriginalHeaders;

/**
* Preserves mappings of _virtual_include directories to original paths relative to
* the workspace directory. Intended for use by compiler options like GCC's
* {@code -fdebug-prefix-map}. See {@link virtualToOriginalHeaders} for additional
* context. Unlike {@link virtualToOriginalHeaders}, this is populated even if
* coverage isn't enabled.
*/
private final NestedSet<Tuple> virtualToOriginalDirs;

/**
* Caches the actual number of transitive headers reachable through transitiveHeaderInfos. We need
* to create maps to store these and so caching this count can substantially help with memory
Expand All @@ -116,6 +126,7 @@ private CcCompilationContext(
CppModuleMap cppModuleMap,
boolean propagateModuleMapAsActionInput,
NestedSet<Tuple> virtualToOriginalHeaders,
NestedSet<Tuple> virtualToOriginalDirs,
NestedSet<Artifact> headerTokens) {
Preconditions.checkNotNull(commandLineCcCompilationContext);
this.commandLineCcCompilationContext = commandLineCcCompilationContext;
Expand All @@ -130,6 +141,7 @@ private CcCompilationContext(
this.compilationPrerequisites = compilationPrerequisites;
this.propagateModuleMapAsActionInput = propagateModuleMapAsActionInput;
this.virtualToOriginalHeaders = virtualToOriginalHeaders;
this.virtualToOriginalDirs = virtualToOriginalDirs;
this.transitiveHeaderCount = -1;
this.headerTokens = headerTokens;
}
Expand Down Expand Up @@ -253,6 +265,12 @@ public Depset getStarlarkVirtualToOriginalHeaders(StarlarkThread thread) throws
return Depset.of(Tuple.class, getVirtualToOriginalHeaders());
}

@Override
public Depset getStarlarkVirtualToOriginalDirs(StarlarkThread thread) throws EvalException {
CcModule.checkPrivateStarlarkificationAllowlist(thread);
return Depset.of(Tuple.class, getVirtualToOriginalDirs());
}

@Override
@Nullable
public CppModuleMap getStarlarkModuleMap(StarlarkThread thread) throws EvalException {
Expand Down Expand Up @@ -648,6 +666,7 @@ public static CcCompilationContext createWithExtraHeaderTokens(
ccCompilationContext.cppModuleMap,
ccCompilationContext.propagateModuleMapAsActionInput,
ccCompilationContext.virtualToOriginalHeaders,
ccCompilationContext.virtualToOriginalDirs,
headerTokens.build());
}

Expand All @@ -665,6 +684,10 @@ public NestedSet<Tuple> getVirtualToOriginalHeaders() {
return virtualToOriginalHeaders;
}

public NestedSet<Tuple> getVirtualToOriginalDirs() {
return virtualToOriginalDirs;
}

/**
* The parts of the {@code CcCompilationContext} that influence the command line of compilation
* actions.
Expand Down Expand Up @@ -725,6 +748,7 @@ public static class Builder {
private CppModuleMap cppModuleMap;
private boolean propagateModuleMapAsActionInput = true;
private final NestedSetBuilder<Tuple> virtualToOriginalHeaders = NestedSetBuilder.stableOrder();
private final NestedSetBuilder<Tuple> virtualToOriginalDirs = NestedSetBuilder.stableOrder();
private final NestedSetBuilder<Artifact> headerTokens = NestedSetBuilder.stableOrder();

/** Creates a new builder for a {@link CcCompilationContext} instance. */
Expand Down Expand Up @@ -799,6 +823,8 @@ private void mergeDependentCcCompilationContext(
allDefines.addTransitive(otherCcCompilationContext.getDefines());
virtualToOriginalHeaders.addTransitive(
otherCcCompilationContext.getVirtualToOriginalHeaders());
virtualToOriginalDirs.addTransitive(
otherCcCompilationContext.getVirtualToOriginalDirs());

headerTokens.addTransitive(otherCcCompilationContext.getHeaderTokens());
}
Expand Down Expand Up @@ -1073,6 +1099,12 @@ public Builder addVirtualToOriginalHeaders(NestedSet<Tuple> virtualToOriginalHea
return this;
}

@CanIgnoreReturnValue
public Builder addVirtualToOriginalDirs(NestedSet<Tuple> virtualToOriginalDirs) {
this.virtualToOriginalDirs.addTransitive(virtualToOriginalDirs);
return this;
}

/** Builds the {@link CcCompilationContext}. */
public CcCompilationContext build() {
TransitiveSetHelper<String> allDefines = new TransitiveSetHelper<>();
Expand Down Expand Up @@ -1114,6 +1146,7 @@ public CcCompilationContext build() {
cppModuleMap,
propagateModuleMapAsActionInput,
virtualToOriginalHeaders.build(),
virtualToOriginalDirs.build(),
headerTokens.build());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1332,7 +1332,8 @@ private CcToolchainVariables setupCompileBuildVariables(
ccCompilationContext.getSystemIncludeDirs(),
ccCompilationContext.getFrameworkIncludeDirs(),
ccCompilationContext.getDefines(),
ccCompilationContext.getNonTransitiveDefines());
ccCompilationContext.getNonTransitiveDefines(),
ccCompilationContext.getVirtualToOriginalDirs());

if (usePrebuiltParent) {
parent = buildVariables.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,8 @@ public CcToolchainVariables getCompileBuildVariables(
Depset.noneableCast(
frameworkIncludeDirs, String.class, "framework_include_directories"),
Depset.noneableCast(defines, String.class, "preprocessor_defines").toList(),
ImmutableList.of()))
ImmutableList.of(),
/* virtualToOriginalDirs= */ NestedSetBuilder.emptySet(Order.STABLE_ORDER)))
.addStringSequenceVariable("stripopts", asClassImmutableList(stripOpts));
String inputFileString = convertFromNoneable(inputFile, null);
if (inputFileString != null) {
Expand Down Expand Up @@ -807,6 +808,7 @@ public CcCompilationContext createCcCompilationContext(
Object labelForMiddlemanNameObject,
Object externalIncludes,
Object virtualToOriginalHeaders,
Object virtualToOriginalDirs,
Sequence<?> dependentCcCompilationContexts,
Sequence<?> nonCodeInputs,
Sequence<?> looseHdrsDirsObject,
Expand Down Expand Up @@ -878,6 +880,9 @@ public CcCompilationContext createCcCompilationContext(
ccCompilationContext.addVirtualToOriginalHeaders(
Depset.cast(virtualToOriginalHeaders, Tuple.class, "virtual_to_original_headers"));

ccCompilationContext.addVirtualToOriginalDirs(
Depset.cast(virtualToOriginalDirs, Tuple.class, "virtual_to_original_dirs"));

ccCompilationContext.addDependentCcCompilationContexts(
Sequence.cast(
dependentCcCompilationContexts,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.util.Map;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.StarlarkThread;
import net.starlark.java.eval.Tuple;

/** Enum covering all build variables we create for all various {@link CppCompileAction}. */
public enum CompileBuildVariables {
Expand Down Expand Up @@ -130,7 +131,12 @@ public enum CompileBuildVariables {
/** Path to the memprof profile artifact */
MEMPROF_PROFILE_PATH("memprof_profile_path"),
/** Variable for includes that compiler needs to include into sources. */
INCLUDES("includes");
INCLUDES("includes"),
/**
* A list of VIRTUAL=ORIGINAL include paths for "virtual" includes.
* Intended for use with gcc's -fdebug-prefix-map.
*/
VIRTUAL_TO_ORIGINAL_DIRS("virtual_to_original_dirs");

private final String variableName;

Expand Down Expand Up @@ -166,7 +172,8 @@ public static CcToolchainVariables setupVariablesOrReportRuleError(
NestedSet<PathFragment> systemIncludeDirs,
NestedSet<PathFragment> frameworkIncludeDirs,
Iterable<String> defines,
Iterable<String> localDefines)
Iterable<String> localDefines,
NestedSet<Tuple> virtualToOriginalDirs)
throws InterruptedException {
try {
if (usePic
Expand Down Expand Up @@ -201,7 +208,8 @@ public static CcToolchainVariables setupVariablesOrReportRuleError(
getSafePathStrings(systemIncludeDirs),
getSafePathStrings(frameworkIncludeDirs),
defines,
localDefines);
localDefines,
virtualToOriginalDirs);
} catch (EvalException e) {
ruleErrorConsumer.ruleError(e.getMessage());
return CcToolchainVariables.EMPTY;
Expand Down Expand Up @@ -235,7 +243,8 @@ public static CcToolchainVariables setupVariablesOrThrowEvalException(
NestedSet<String> systemIncludeDirs,
NestedSet<String> frameworkIncludeDirs,
Iterable<String> defines,
Iterable<String> localDefines)
Iterable<String> localDefines,
NestedSet<Tuple> virtualToOriginalDirs)
throws EvalException, InterruptedException {
if (usePic
&& !featureConfiguration.isEnabled(CppRuleClasses.PIC)
Expand Down Expand Up @@ -269,7 +278,8 @@ public static CcToolchainVariables setupVariablesOrThrowEvalException(
systemIncludeDirs,
frameworkIncludeDirs,
defines,
localDefines);
localDefines,
virtualToOriginalDirs);
}

private static CcToolchainVariables setupVariables(
Expand Down Expand Up @@ -299,7 +309,8 @@ private static CcToolchainVariables setupVariables(
NestedSet<String> systemIncludeDirs,
NestedSet<String> frameworkIncludeDirs,
Iterable<String> defines,
Iterable<String> localDefines) {
Iterable<String> localDefines,
NestedSet<Tuple> virtualToOriginalDirs) {
CcToolchainVariables.Builder buildVariables = CcToolchainVariables.builder(parent);
setupCommonVariablesInternal(
buildVariables,
Expand All @@ -315,7 +326,8 @@ private static CcToolchainVariables setupVariables(
systemIncludeDirs,
frameworkIncludeDirs,
defines,
localDefines);
localDefines,
virtualToOriginalDirs);
setupSpecificVariables(
buildVariables,
sourceFile,
Expand Down Expand Up @@ -430,7 +442,8 @@ public static void setupCommonVariables(
List<PathFragment> systemIncludeDirs,
List<PathFragment> frameworkIncludeDirs,
Iterable<String> defines,
Iterable<String> localDefines) {
Iterable<String> localDefines,
NestedSet<Tuple> virtualToOriginalDirs) {
setupCommonVariablesInternal(
buildVariables,
featureConfiguration,
Expand All @@ -445,7 +458,8 @@ public static void setupCommonVariables(
getSafePathStrings(systemIncludeDirs),
getSafePathStrings(frameworkIncludeDirs),
defines,
localDefines);
localDefines,
virtualToOriginalDirs);
}

private static void setupCommonVariablesInternal(
Expand All @@ -462,14 +476,16 @@ private static void setupCommonVariablesInternal(
NestedSet<String> systemIncludeDirs,
NestedSet<String> frameworkIncludeDirs,
Iterable<String> defines,
Iterable<String> localDefines) {
Iterable<String> localDefines,
NestedSet<Tuple> virtualToOriginalDirs) {
Preconditions.checkNotNull(directModuleMaps);
Preconditions.checkNotNull(includeDirs);
Preconditions.checkNotNull(quoteIncludeDirs);
Preconditions.checkNotNull(systemIncludeDirs);
Preconditions.checkNotNull(frameworkIncludeDirs);
Preconditions.checkNotNull(defines);
Preconditions.checkNotNull(localDefines);
Preconditions.checkNotNull(virtualToOriginalDirs);

if (featureConfiguration.isEnabled(CppRuleClasses.MODULE_MAPS) && cppModuleMap != null) {
buildVariables.addStringVariable(MODULE_NAME.getVariableName(), cppModuleMap.getName());
Expand Down Expand Up @@ -510,6 +526,17 @@ private static void setupCommonVariablesInternal(

buildVariables.addStringSequenceVariable(PREPROCESSOR_DEFINES.getVariableName(), allDefines);

if (!virtualToOriginalDirs.isEmpty()) {
buildVariables.addStringSequenceVariable(
VIRTUAL_TO_ORIGINAL_DIRS.getVariableName(),
Iterables.transform(
virtualToOriginalDirs.toList(),
vToA -> vToA.get(0) + "=" + vToA.get(1)));
} else {
buildVariables.addStringSequenceVariable(VIRTUAL_TO_ORIGINAL_DIRS.getVariableName(),
ImmutableList.of());
}

buildVariables.addAllStringVariables(additionalBuildVariables);
for (VariablesExtension extension : variablesExtensions) {
extension.addVariables(buildVariables);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,8 @@ private static CcToolchainVariables getVariables(
ccToolchainProvider,
fdoBuildStamp,
codeCoverageEnabled),
/* localDefines= */ ImmutableList.of());
} catch (EvalException e) {
/* localDefines= */ ImmutableList.of(),
/* virtualToOriginalDirs= */ NestedSetBuilder.emptySet(Order.STABLE_ORDER)); } catch (EvalException e) {
throw new RuleErrorException(e.getMessage());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,12 @@ public interface CcCompilationContextApi<
useStarlarkThread = true)
Depset getStarlarkVirtualToOriginalHeaders(StarlarkThread thread) throws EvalException;

@StarlarkMethod(
name = "virtual_to_original_dirs",
documented = false,
useStarlarkThread = true)
Depset getStarlarkVirtualToOriginalDirs(StarlarkThread thread) throws EvalException;

@StarlarkMethod(
name = "module_map",
documented = false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1279,6 +1279,12 @@ LinkingContextT createCcLinkingInfo(
positional = false,
named = true,
defaultValue = "unbound"),
@Param(
name = "virtual_to_original_dirs",
documented = false,
positional = false,
named = true,
defaultValue = "unbound"),
@Param(
name = "dependent_cc_compilation_contexts",
documented = false,
Expand Down Expand Up @@ -1363,6 +1369,7 @@ CompilationContextT createCcCompilationContext(
Object labelForMiddlemanNameObject,
Object externalIncludes,
Object virtualToOriginalHeaders,
Object virtualToOriginalDirs,
Sequence<?> dependentCcCompilationContexts,
Sequence<?> nonCodeInputs,
Sequence<?> looseHdrsDirs,
Expand Down
5 changes: 5 additions & 0 deletions src/main/starlark/builtins_bzl/common/cc/cc_common.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ def _create_compilation_context(
label = _UNBOUND,
external_includes = _UNBOUND,
virtual_to_original_headers = _UNBOUND,
virtual_to_original_dirs = _UNBOUND,
dependent_cc_compilation_contexts = _UNBOUND,
non_code_inputs = _UNBOUND,
headers_checking_mode = _UNBOUND,
Expand All @@ -447,6 +448,7 @@ def _create_compilation_context(
actions != _UNBOUND or \
external_includes != _UNBOUND or \
virtual_to_original_headers != _UNBOUND or \
virtual_to_original_dirs != _UNBOUND or \
dependent_cc_compilation_contexts != _UNBOUND or \
non_code_inputs != _UNBOUND or \
headers_checking_mode != _UNBOUND or \
Expand All @@ -471,6 +473,8 @@ def _create_compilation_context(
external_includes = depset()
if virtual_to_original_headers == _UNBOUND:
virtual_to_original_headers = depset()
if virtual_to_original_dirs == _UNBOUND:
virtual_to_original_dirs = depset()
if dependent_cc_compilation_contexts == _UNBOUND:
dependent_cc_compilation_contexts = []
if non_code_inputs == _UNBOUND:
Expand Down Expand Up @@ -508,6 +512,7 @@ def _create_compilation_context(
label = label,
external_includes = external_includes,
virtual_to_original_headers = virtual_to_original_headers,
virtual_to_original_dirs = virtual_to_original_dirs,
dependent_cc_compilation_contexts = dependent_cc_compilation_contexts,
non_code_inputs = non_code_inputs,
loose_hdrs_dirs = [],
Expand Down
Loading

0 comments on commit 855f858

Please sign in to comment.