From 694fced1e37267ad2f5932a62777336db728d644 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Tue, 25 Jun 2024 14:56:42 -0700 Subject: [PATCH] Don't mutate non-property module fields Setting sanitize and stl to nil in a mutator isn't valid, if any other mutator creates a variant then the factory method will be called again to create the new variants and will reset sanitize and stl to non-nil. Add a property to sanitize and check it everywhere that checked for sanitize != nil, and set the Stl property. Bug: 319288033 Test: all soong tests pass Flag: EXEMPT refactor Change-Id: If99d5fa0f088ee4a73cc7dccdab4268618a6009f --- cc/cc.go | 10 ++-------- cc/library.go | 8 ++++++-- cc/sanitize.go | 35 +++++++++++++++++++++++++++++++---- sdk/cc_sdk_test.go | 1 + 4 files changed, 40 insertions(+), 14 deletions(-) diff --git a/cc/cc.go b/cc/cc.go index 134d89c07..740be3afe 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -1379,17 +1379,11 @@ func (c *Module) isOrderfileCompile() bool { } func (c *Module) isCfi() bool { - if sanitize := c.sanitize; sanitize != nil { - return Bool(sanitize.Properties.SanitizeMutated.Cfi) - } - return false + return c.sanitize.isSanitizerEnabled(cfi) } func (c *Module) isFuzzer() bool { - if sanitize := c.sanitize; sanitize != nil { - return Bool(sanitize.Properties.SanitizeMutated.Fuzzer) - } - return false + return c.sanitize.isSanitizerEnabled(Fuzzer) } func (c *Module) isNDKStubLibrary() bool { diff --git a/cc/library.go b/cc/library.go index 4373b46b4..560b1ae40 100644 --- a/cc/library.go +++ b/cc/library.go @@ -2220,8 +2220,12 @@ func createVersionVariations(mctx android.BottomUpMutatorContext, versions []str if variants[i] != "" || isLLNDK || isVendorPublicLibrary || isImportedApiLibrary { // A stubs or LLNDK stubs variant. c := m.(*Module) - c.sanitize = nil - c.stl = nil + if c.sanitize != nil { + c.sanitize.Properties.ForceDisable = true + } + if c.stl != nil { + c.stl.Properties.Stl = StringPtr("none") + } c.Properties.PreventInstall = true lib := moduleLibraryInterface(m) isLatest := i == (len(versions) - 1) diff --git a/cc/sanitize.go b/cc/sanitize.go index d43a096af..e6f00faa8 100644 --- a/cc/sanitize.go +++ b/cc/sanitize.go @@ -383,7 +383,19 @@ type SanitizeProperties struct { Sanitize SanitizeUserProps `android:"arch_variant"` SanitizeMutated sanitizeMutatedProperties `blueprint:"mutated"` - SanitizerEnabled bool `blueprint:"mutated"` + // ForceDisable is set by the version mutator to disable sanitization of stubs variants + ForceDisable bool `blueprint:"mutated"` + + // SanitizerEnabled is set by begin() if any of the sanitize boolean properties are set after + // applying the logic that enables globally enabled sanitizers and disables any unsupported + // sanitizers. + // TODO(b/349906293): this has some unintuitive behavior. It is set in begin() before the sanitize + // mutator is run if any of the individual sanitizes properties are set, and then the individual + // sanitize properties are cleared in the non-sanitized variants, but this value is never cleared. + // That results in SanitizerEnabled being set in variants that have no sanitizers enabled, causing + // some of the sanitizer logic in flags() to be applied to the non-sanitized variant. + SanitizerEnabled bool `blueprint:"mutated"` + MinimalRuntimeDep bool `blueprint:"mutated"` BuiltinsDep bool `blueprint:"mutated"` UbsanRuntimeDep bool `blueprint:"mutated"` @@ -455,6 +467,10 @@ func (sanitize *sanitize) begin(ctx BaseModuleContext) { s := &sanitize.Properties.SanitizeMutated s.copyUserPropertiesToMutated(&sanitize.Properties.Sanitize) + if sanitize.Properties.ForceDisable { + return + } + // Don't apply sanitizers to NDK code. if ctx.useSdk() { s.Never = BoolPtr(true) @@ -765,6 +781,10 @@ func toDisableUnsignedShiftBaseChange(flags []string) bool { } func (s *sanitize) flags(ctx ModuleContext, flags Flags) Flags { + if s.Properties.ForceDisable { + return flags + } + if !s.Properties.SanitizerEnabled && !s.Properties.UbsanRuntimeDep { return flags } @@ -1104,7 +1124,7 @@ func (s *sanitize) isSanitizerEnabled(t SanitizerType) bool { if s == nil { return false } - if proptools.Bool(s.Properties.SanitizeMutated.Never) { + if s.Properties.ForceDisable || proptools.Bool(s.Properties.SanitizeMutated.Never) { return false } @@ -1329,7 +1349,7 @@ func (s *sanitizerSplitMutator) Mutate(mctx android.BottomUpMutatorContext, vari } func (c *Module) SanitizeNever() bool { - return Bool(c.sanitize.Properties.SanitizeMutated.Never) + return c.sanitize.Properties.ForceDisable || Bool(c.sanitize.Properties.SanitizeMutated.Never) } func (c *Module) IsSanitizerExplicitlyDisabled(t SanitizerType) bool { @@ -1340,6 +1360,9 @@ func (c *Module) IsSanitizerExplicitlyDisabled(t SanitizerType) bool { func sanitizerRuntimeDepsMutator(mctx android.TopDownMutatorContext) { // Change this to PlatformSanitizable when/if non-cc modules support ubsan sanitizers. if c, ok := mctx.Module().(*Module); ok && c.sanitize != nil { + if c.sanitize.Properties.ForceDisable { + return + } isSanitizableDependencyTag := c.SanitizableDepTagChecker() mctx.WalkDeps(func(child, parent android.Module) bool { if !isSanitizableDependencyTag(mctx.OtherModuleDependencyTag(child)) { @@ -1350,7 +1373,7 @@ func sanitizerRuntimeDepsMutator(mctx android.TopDownMutatorContext) { if !ok || !d.static() { return false } - if d.sanitize != nil { + if d.sanitize != nil && !d.sanitize.Properties.ForceDisable { if enableMinimalRuntime(d.sanitize) { // If a static dependency is built with the minimal runtime, // make sure we include the ubsan minimal runtime. @@ -1385,6 +1408,10 @@ func sanitizerRuntimeMutator(mctx android.BottomUpMutatorContext) { if !c.Enabled(mctx) { return } + if c.sanitize.Properties.ForceDisable { + return + } + var sanitizers []string var diagSanitizers []string diff --git a/sdk/cc_sdk_test.go b/sdk/cc_sdk_test.go index c9e20b3b8..5d76930b4 100644 --- a/sdk/cc_sdk_test.go +++ b/sdk/cc_sdk_test.go @@ -2196,6 +2196,7 @@ cc_prebuilt_library_shared { prefer: false, visibility: ["//visibility:public"], apex_available: ["//apex_available:platform"], + stl: "none", compile_multilib: "both", stubs: { versions: [