Skip to content

Commit

Permalink
Remove hardwiring HWASAN from core.
Browse files Browse the repository at this point in the history
This is accomplished by communicating the "is this an HWASAN build" to the toolchain using a configuration flag instead of a feature.

RELNOTES: None.
PiperOrigin-RevId: 352006767
  • Loading branch information
lberki authored and copybara-github committed Jan 15, 2021
1 parent ec55533 commit 5753b32
Show file tree
Hide file tree
Showing 9 changed files with 31 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -294,9 +294,6 @@ private void getAllFeatures(Set<String> allEnabledFeatures, Set<String> allDisab
Set<String> globallyEnabled = new HashSet<>();
Set<String> globallyDisabled = new HashSet<>();
parseFeatures(getConfiguration().getDefaultFeatures(), globallyEnabled, globallyDisabled);
if (getConfiguration().getFatApkSplitSanitizer().feature != null) {
globallyEnabled.add(getConfiguration().getFatApkSplitSanitizer().feature);
}
Set<String> packageEnabled = new HashSet<>();
Set<String> packageDisabled = new HashSet<>();
parseFeatures(getRule().getPackage().getFeatures(), packageEnabled, packageDisabled);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -910,10 +910,6 @@ public Label getAutoCpuEnvironmentGroup() {
return options.autoCpuEnvironmentGroup;
}

public CoreOptions.FatApkSplitSanitizer getFatApkSplitSanitizer() {
return options.fatApkSplitSanitizer;
}

public Class<? extends Fragment> getStarlarkFragmentByName(String name) {
return starlarkVisibleFragments.get(name);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -880,36 +880,6 @@ public IncludeConfigFragmentsEnumConverter() {
}
}

/** Used to specify which sanitizer is enabled in the current APK split. */
public enum FatApkSplitSanitizer {
NONE(null, ""),
HWASAN("hwasan", "-hwasan");

private FatApkSplitSanitizer(String feature, String androidLibDirSuffix) {
this.feature = feature;
this.androidLibDirSuffix = androidLibDirSuffix;
}

public final String feature;
public final String androidLibDirSuffix;
}

/** Converter for {@link FatApkSplitSanitizer}. */
public static class FatApkSplitSanitizerConverter extends EnumConverter<FatApkSplitSanitizer> {
public FatApkSplitSanitizerConverter() {
super(FatApkSplitSanitizer.class, "fat apk split sanitizer");
}
}

@Option(
name = "fat_apk_split_sanitizer",
defaultValue = "NONE",
effectTags = {OptionEffectTag.CHANGES_INPUTS, OptionEffectTag.AFFECTS_OUTPUTS},
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
metadataTags = {OptionMetadataTag.INTERNAL},
converter = FatApkSplitSanitizerConverter.class)
public FatApkSplitSanitizer fatApkSplitSanitizer;

@Override
public FragmentOptions getHost() {
CoreOptions host = (CoreOptions) getDefault();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -763,6 +763,15 @@ public static class Options extends FragmentOptions {
+ " native)")
public boolean incompatibleUseToolchainResolution;

@Option(
name = "android hwasan", // Space is so that this cannot be set on the command line
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
metadataTags = {OptionMetadataTag.INTERNAL},
help = "Whether HWASAN is enabled.")
public boolean hwasan;

@Option(
name = "experimental_filter_r_jars_from_android_test",
defaultValue = "false",
Expand Down Expand Up @@ -929,6 +938,7 @@ public static class Options extends FragmentOptions {
@Override
public FragmentOptions getHost() {
Options host = (Options) super.getHost();
host.hwasan = false;
host.androidCrosstoolTop = androidCrosstoolTop;
host.sdk = sdk;
host.fatApkCpus = ImmutableList.of(); // Fat APK archs don't apply to the host.
Expand Down Expand Up @@ -1007,6 +1017,7 @@ public FragmentOptions getHost() {
private final Label legacyMainDexListGenerator;
private final boolean disableInstrumentationManifestMerging;
private final boolean incompatibleUseToolchainResolution;
private final boolean hwasan;
private final boolean getJavaResourcesFromOptimizedJar;

public AndroidConfiguration(BuildOptions buildOptions) throws InvalidConfigurationException {
Expand Down Expand Up @@ -1066,6 +1077,7 @@ public AndroidConfiguration(BuildOptions buildOptions) throws InvalidConfigurati
this.legacyMainDexListGenerator = options.legacyMainDexListGenerator;
this.disableInstrumentationManifestMerging = options.disableInstrumentationManifestMerging;
this.incompatibleUseToolchainResolution = options.incompatibleUseToolchainResolution;
this.hwasan = options.hwasan;
this.getJavaResourcesFromOptimizedJar = options.getJavaResourcesFromOptimizedJar;

if (incrementalDexingShardsAfterProguard < 0) {
Expand Down Expand Up @@ -1304,6 +1316,11 @@ public boolean incompatibleUseToolchainResolution() {
return incompatibleUseToolchainResolution;
}

@Override
public boolean isHwasan() {
return hwasan;
}

@Override
public String getOutputDirectoryName() {
return configurationDistinguisher.suffix;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -298,15 +298,14 @@ public ImmutableMap<String, BuildOptions> split(
if (cpu.equals("arm64-v8a") && androidOptions.fatApkHwasan) {
BuildOptionsView hwasanSplitOptions = splitOptions.clone();

// Setting the fatApkSplitSanitizer has these consequences:
// - changes the native library install directory to lib/arm64-v8a-hwasan
// - adds "hwasan" to the default feature set such that it gets removed when
// transitioning to host or exec configurations as a consequence of
// CoreOptions.getHost() not copying the fatApkSplitSanitizer field (which rules out
// adding it to defaultFeatures, which gets copied).
hwasanSplitOptions.get(CoreOptions.class).fatApkSplitSanitizer =
CoreOptions.FatApkSplitSanitizer.HWASAN;

// A HWASAN build is different from a regular one in these ways:
// - The native library install directory gets a "-hwasan" suffix
// - Some compiler/linker command line options are different (defined in the Android C++
// toolchain)
// - The name of the output directory is changed so that HWASAN and non-HWASAN artifacts
// do not conflict
hwasanSplitOptions.get(CppOptions.class).outputDirectoryTag = "hwasan";
hwasanSplitOptions.get(AndroidConfiguration.Options.class).hwasan = true;
result.put(cpu + "-hwasan", hwasanSplitOptions.underlying());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ public final class NativeLibs {
private static String getLibDirName(ConfiguredTargetAndData dep) {
BuildConfiguration configuration = dep.getConfiguration();
String name = configuration.getFragment(AndroidConfiguration.class).getCpu();
name += configuration.getFatApkSplitSanitizer().androidLibDirSuffix;
if (configuration.getFragment(AndroidConfiguration.class).isHwasan()) {
name += "-hwasan";
}
return name;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,6 @@ public String toString() {

private final boolean appleGenerateDsym;

private final CoreOptions.FatApkSplitSanitizer fatApkSplitSanitizer;

public CppConfiguration(BuildOptions options) throws InvalidConfigurationException {
CppOptions cppOptions = options.get(CppOptions.class);

Expand Down Expand Up @@ -289,7 +287,6 @@ public CppConfiguration(BuildOptions options) throws InvalidConfigurationExcepti
this.appleGenerateDsym =
(cppOptions.appleGenerateDsym
|| (cppOptions.appleEnableAutoDsymDbg && compilationMode == CompilationMode.DBG));
this.fatApkSplitSanitizer = commonOptions.fatApkSplitSanitizer;
}

/** Returns the label of the <code>cc_compiler</code> rule for the C++ configuration. */
Expand Down Expand Up @@ -532,9 +529,6 @@ public void reportInvalidOptions(EventHandler reporter, BuildOptions buildOption
@Override
public String getOutputDirectoryName() {
String result = cpu;
if (fatApkSplitSanitizer.feature != null) {
result += "-" + fatApkSplitSanitizer.feature;
}
if (!cppOptions.outputDirectoryTag.isEmpty()) {
result += "-" + cppOptions.outputDirectoryTag;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,10 +217,6 @@ public static NativeDepsRunfiles createNativeDepsAction(
if (!ruleContext.getDisabledFeatures().contains(CppRuleClasses.LEGACY_WHOLE_ARCHIVE)) {
requestedFeaturesBuilder.add(CppRuleClasses.LEGACY_WHOLE_ARCHIVE);
}
final String sanitizerFeature = configuration.getFatApkSplitSanitizer().feature;
if (sanitizerFeature != null && !ruleContext.getDisabledFeatures().contains(sanitizerFeature)) {
requestedFeaturesBuilder.add(sanitizerFeature);
}
ImmutableSortedSet<String> requestedFeatures = requestedFeaturesBuilder.build();

FeatureConfiguration featureConfiguration =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,4 +241,7 @@ public interface AndroidConfigurationApi extends StarlarkValue {
doc = "",
documented = false)
boolean incompatibleUseToolchainResolution();

@StarlarkMethod(name = "hwasan", structField = true, doc = "", documented = false)
boolean isHwasan();
}

0 comments on commit 5753b32

Please sign in to comment.