Skip to content

Commit

Permalink
Convert trivial TopDown mutators to BottomUp
Browse files Browse the repository at this point in the history
Many TopDown mutators can be easily converted to BottomUp mutators,
which are easier to handle for incremental and partial analysis.

Bug: 367784740
Test: all soong tests pass
Test: no change to build.ninja
Flag: EXEMPT refactor
Change-Id: I82955e844ed0eb6680854678c0744ac5398eb7ba
  • Loading branch information
colincross committed Sep 18, 2024
1 parent 1672300 commit da279cf
Show file tree
Hide file tree
Showing 9 changed files with 60 additions and 54 deletions.
16 changes: 10 additions & 6 deletions android/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ type Defaultable interface {

// Apply defaults from the supplied Defaults to the property structures supplied to
// setProperties(...).
applyDefaults(TopDownMutatorContext, []Defaults)
applyDefaults(BottomUpMutatorContext, []Defaults)

// Set the hook to be called after any defaults have been applied.
//
Expand Down Expand Up @@ -209,7 +209,7 @@ func InitDefaultsModule(module DefaultsModule) {

var _ Defaults = (*DefaultsModuleBase)(nil)

func (defaultable *DefaultableModuleBase) applyDefaults(ctx TopDownMutatorContext,
func (defaultable *DefaultableModuleBase) applyDefaults(ctx BottomUpMutatorContext,
defaultsList []Defaults) {

for _, defaults := range defaultsList {
Expand All @@ -226,7 +226,7 @@ func (defaultable *DefaultableModuleBase) applyDefaults(ctx TopDownMutatorContex
// Product variable properties need special handling, the type of the filtered product variable
// property struct may not be identical between the defaults module and the defaultable module.
// Use PrependMatchingProperties to apply whichever properties match.
func (defaultable *DefaultableModuleBase) applyDefaultVariableProperties(ctx TopDownMutatorContext,
func (defaultable *DefaultableModuleBase) applyDefaultVariableProperties(ctx BottomUpMutatorContext,
defaults Defaults, defaultableProp interface{}) {
if defaultableProp == nil {
return
Expand Down Expand Up @@ -254,7 +254,7 @@ func (defaultable *DefaultableModuleBase) applyDefaultVariableProperties(ctx Top
}
}

func (defaultable *DefaultableModuleBase) applyDefaultProperties(ctx TopDownMutatorContext,
func (defaultable *DefaultableModuleBase) applyDefaultProperties(ctx BottomUpMutatorContext,
defaults Defaults, defaultableProp interface{}) {

for _, def := range defaults.properties() {
Expand All @@ -273,7 +273,7 @@ func (defaultable *DefaultableModuleBase) applyDefaultProperties(ctx TopDownMuta

func RegisterDefaultsPreArchMutators(ctx RegisterMutatorsContext) {
ctx.BottomUp("defaults_deps", defaultsDepsMutator).Parallel()
ctx.TopDown("defaults", defaultsMutator).Parallel()
ctx.BottomUp("defaults", defaultsMutator).Parallel()
}

func defaultsDepsMutator(ctx BottomUpMutatorContext) {
Expand All @@ -282,8 +282,12 @@ func defaultsDepsMutator(ctx BottomUpMutatorContext) {
}
}

func defaultsMutator(ctx TopDownMutatorContext) {
func defaultsMutator(ctx BottomUpMutatorContext) {
if defaultable, ok := ctx.Module().(Defaultable); ok {
if _, isDefaultsModule := ctx.Module().(Defaults); isDefaultsModule {
// Don't squash transitive defaults into defaults modules
return
}
defaults := defaultable.defaults().Defaults
if len(defaults) > 0 {
var defaultsList []Defaults
Expand Down
16 changes: 12 additions & 4 deletions android/mutator.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,16 +193,16 @@ type BaseMutatorContext interface {
// Rename all variants of a module. The new name is not visible to calls to ModuleName,
// AddDependency or OtherModuleName until after this mutator pass is complete.
Rename(name string)

// CreateModule creates a new module by calling the factory method for the specified moduleType, and applies
// the specified property structs to it as if the properties were set in a blueprint file.
CreateModule(ModuleFactory, ...interface{}) Module
}

type TopDownMutator func(TopDownMutatorContext)

type TopDownMutatorContext interface {
BaseMutatorContext

// CreateModule creates a new module by calling the factory method for the specified moduleType, and applies
// the specified property structs to it as if the properties were set in a blueprint file.
CreateModule(ModuleFactory, ...interface{}) Module
}

type topDownMutatorContext struct {
Expand Down Expand Up @@ -739,6 +739,14 @@ func (b *bottomUpMutatorContext) Rename(name string) {
b.Module().base().commonProperties.DebugName = name
}

func (b *bottomUpMutatorContext) createModule(factory blueprint.ModuleFactory, name string, props ...interface{}) blueprint.Module {
return b.bp.CreateModule(factory, name, props...)
}

func (b *bottomUpMutatorContext) CreateModule(factory ModuleFactory, props ...interface{}) Module {
return createModule(b, factory, "_bottomUpMutatorModule", props...)
}

func (b *bottomUpMutatorContext) AddDependency(module blueprint.Module, tag blueprint.DependencyTag, name ...string) []blueprint.Module {
if b.baseModuleContext.checkedMissingDeps() {
panic("Adding deps not allowed after checking for missing deps")
Expand Down
4 changes: 2 additions & 2 deletions android/visibility.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ func RegisterVisibilityRuleGatherer(ctx RegisterMutatorsContext) {

// This must be registered after the deps have been resolved.
func RegisterVisibilityRuleEnforcer(ctx RegisterMutatorsContext) {
ctx.TopDown("visibilityRuleEnforcer", visibilityRuleEnforcer).Parallel()
ctx.BottomUp("visibilityRuleEnforcer", visibilityRuleEnforcer).Parallel()
}

// Checks the per-module visibility rule lists before defaults expansion.
Expand Down Expand Up @@ -507,7 +507,7 @@ func splitRule(ctx BaseModuleContext, ruleExpression string, currentPkg, propert
return true, pkg, name
}

func visibilityRuleEnforcer(ctx TopDownMutatorContext) {
func visibilityRuleEnforcer(ctx BottomUpMutatorContext) {
qualified := createVisibilityModuleReference(ctx.ModuleName(), ctx.ModuleDir(), ctx.Module())

// Visit all the dependencies making sure that this module has access to them all.
Expand Down
3 changes: 1 addition & 2 deletions apex/apex.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,10 @@ func registerApexBuildComponents(ctx android.RegistrationContext) {
}

func registerPreArchMutators(ctx android.RegisterMutatorsContext) {
ctx.TopDown("prebuilt_apex_module_creator", prebuiltApexModuleCreatorMutator).Parallel()
ctx.BottomUp("prebuilt_apex_module_creator", prebuiltApexModuleCreatorMutator).Parallel()
}

func RegisterPreDepsMutators(ctx android.RegisterMutatorsContext) {
ctx.TopDown("apex_vndk", apexVndkMutator).Parallel()
ctx.BottomUp("apex_vndk_deps", apexVndkDepsMutator).Parallel()
}

Expand Down
14 changes: 7 additions & 7 deletions apex/prebuilt.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ func (p *prebuiltCommon) AndroidMkEntries() []android.AndroidMkEntries {
// prebuiltApexModuleCreator defines the methods that need to be implemented by prebuilt_apex and
// apex_set in order to create the modules needed to provide access to the prebuilt .apex file.
type prebuiltApexModuleCreator interface {
createPrebuiltApexModules(ctx android.TopDownMutatorContext)
createPrebuiltApexModules(ctx android.BottomUpMutatorContext)
}

// prebuiltApexModuleCreatorMutator is the mutator responsible for invoking the
Expand All @@ -275,7 +275,7 @@ type prebuiltApexModuleCreator interface {
// will need to access dependencies added by that (exported modules) but must run before the
// DepsMutator so that the deapexer module it creates can add dependencies onto itself from the
// exported modules.
func prebuiltApexModuleCreatorMutator(ctx android.TopDownMutatorContext) {
func prebuiltApexModuleCreatorMutator(ctx android.BottomUpMutatorContext) {
module := ctx.Module()
if creator, ok := module.(prebuiltApexModuleCreator); ok {
creator.createPrebuiltApexModules(ctx)
Expand Down Expand Up @@ -543,7 +543,7 @@ func PrebuiltFactory() android.Module {
return module
}

func createApexSelectorModule(ctx android.TopDownMutatorContext, name string, apexFileProperties *ApexFileProperties) {
func createApexSelectorModule(ctx android.BottomUpMutatorContext, name string, apexFileProperties *ApexFileProperties) {
props := struct {
Name *string
}{
Expand All @@ -561,7 +561,7 @@ func createApexSelectorModule(ctx android.TopDownMutatorContext, name string, ap
// A deapexer module is only needed when the prebuilt apex specifies one or more modules in either
// the `exported_java_libs` or `exported_bootclasspath_fragments` properties as that indicates that
// the listed modules need access to files from within the prebuilt .apex file.
func (p *prebuiltCommon) createDeapexerModuleIfNeeded(ctx android.TopDownMutatorContext, deapexerName string, apexFileSource string) {
func (p *prebuiltCommon) createDeapexerModuleIfNeeded(ctx android.BottomUpMutatorContext, deapexerName string, apexFileSource string) {
// Only create the deapexer module if it is needed.
if !p.hasExportedDeps() {
return
Expand Down Expand Up @@ -703,7 +703,7 @@ var _ prebuiltApexModuleCreator = (*Prebuilt)(nil)
// / | \
// V V V
// selector <--- deapexer <--- exported java lib
func (p *Prebuilt) createPrebuiltApexModules(ctx android.TopDownMutatorContext) {
func (p *Prebuilt) createPrebuiltApexModules(ctx android.BottomUpMutatorContext) {
apexSelectorModuleName := apexSelectorModuleName(p.Name())
createApexSelectorModule(ctx, apexSelectorModuleName, &p.properties.ApexFileProperties)

Expand Down Expand Up @@ -958,7 +958,7 @@ func apexSetFactory() android.Module {
return module
}

func createApexExtractorModule(ctx android.TopDownMutatorContext, name string, apexExtractorProperties *ApexExtractorProperties) {
func createApexExtractorModule(ctx android.BottomUpMutatorContext, name string, apexExtractorProperties *ApexExtractorProperties) {
props := struct {
Name *string
}{
Expand All @@ -984,7 +984,7 @@ var _ prebuiltApexModuleCreator = (*ApexSet)(nil)
// prebuilt_apex except that instead of creating a selector module which selects one .apex file
// from those provided this creates an extractor module which extracts the appropriate .apex file
// from the zip file containing them.
func (a *ApexSet) createPrebuiltApexModules(ctx android.TopDownMutatorContext) {
func (a *ApexSet) createPrebuiltApexModules(ctx android.BottomUpMutatorContext) {
apexExtractorModuleName := apexExtractorModuleName(a.Name())
createApexExtractorModule(ctx, apexExtractorModuleName, &a.properties.ApexExtractorProperties)

Expand Down
45 changes: 20 additions & 25 deletions apex/vndk.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,26 @@ type apexVndkProperties struct {
Vndk_version *string
}

func apexVndkMutator(mctx android.TopDownMutatorContext) {
if ab, ok := mctx.Module().(*apexBundle); ok && ab.vndkApex {
if ab.IsNativeBridgeSupported() {
func apexVndkDepsMutator(mctx android.BottomUpMutatorContext) {
if m, ok := mctx.Module().(*cc.Module); ok && cc.IsForVndkApex(mctx, m) {
vndkVersion := m.VndkVersion()

if vndkVersion == "" {
return
}
vndkVersion = "v" + vndkVersion

vndkApexName := "com.android.vndk." + vndkVersion

if mctx.OtherModuleExists(vndkApexName) {
mctx.AddReverseDependency(mctx.Module(), sharedLibTag, vndkApexName)
}
} else if a, ok := mctx.Module().(*apexBundle); ok && a.vndkApex {
if a.IsNativeBridgeSupported() {
mctx.PropertyErrorf("native_bridge_supported", "%q doesn't support native bridge binary.", mctx.ModuleType())
}

vndkVersion := ab.vndkVersion()
vndkVersion := a.vndkVersion()
if vndkVersion != "" {
apiLevel, err := android.ApiLevelFromUser(mctx, vndkVersion)
if err != nil {
Expand All @@ -72,32 +85,14 @@ func apexVndkMutator(mctx android.TopDownMutatorContext) {
if len(targets) > 0 && apiLevel.LessThan(cc.MinApiForArch(mctx, targets[0].Arch.ArchType)) {
// Disable VNDK APEXes for VNDK versions less than the minimum supported API
// level for the primary architecture.
ab.Disable()
a.Disable()
} else {
mctx.AddDependency(mctx.Module(), prebuiltTag, cc.VndkLibrariesTxtModules(vndkVersion, mctx)...)
}
}
}
}

func apexVndkDepsMutator(mctx android.BottomUpMutatorContext) {
if m, ok := mctx.Module().(*cc.Module); ok && cc.IsForVndkApex(mctx, m) {
vndkVersion := m.VndkVersion()

if vndkVersion == "" {
return
}
vndkVersion = "v" + vndkVersion

vndkApexName := "com.android.vndk." + vndkVersion

if mctx.OtherModuleExists(vndkApexName) {
mctx.AddReverseDependency(mctx.Module(), sharedLibTag, vndkApexName)
}
} else if a, ok := mctx.Module().(*apexBundle); ok && a.vndkApex {
vndkVersion := proptools.StringDefault(a.vndkProperties.Vndk_version, "current")
mctx.AddDependency(mctx.Module(), prebuiltTag, cc.VndkLibrariesTxtModules(vndkVersion, mctx)...)
}
}

// name is module.BaseModuleName() which is used as LOCAL_MODULE_NAME and also LOCAL_OVERRIDES_*
func makeCompatSymlinks(name string, ctx android.ModuleContext) (symlinks android.InstallPaths) {
// small helper to add symlink commands
Expand Down
8 changes: 4 additions & 4 deletions cc/cc.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ func RegisterCCBuildComponents(ctx android.RegistrationContext) {
san.registerMutators(ctx)
}

ctx.TopDown("sanitize_runtime_deps", sanitizerRuntimeDepsMutator).Parallel()
ctx.BottomUp("sanitize_runtime_deps", sanitizerRuntimeDepsMutator).Parallel()
ctx.BottomUp("sanitize_runtime", sanitizerRuntimeMutator).Parallel()

ctx.TopDown("fuzz_deps", fuzzMutatorDeps)
ctx.BottomUp("fuzz_deps", fuzzMutatorDeps)

ctx.Transition("coverage", &coverageTransitionMutator{})

Expand All @@ -73,7 +73,7 @@ func RegisterCCBuildComponents(ctx android.RegistrationContext) {
ctx.Transition("lto", &ltoTransitionMutator{})

ctx.BottomUp("check_linktype", checkLinkTypeMutator).Parallel()
ctx.TopDown("double_loadable", checkDoubleLoadableLibraries).Parallel()
ctx.BottomUp("double_loadable", checkDoubleLoadableLibraries).Parallel()
})

ctx.FinalDepsMutators(func(ctx android.RegisterMutatorsContext) {
Expand Down Expand Up @@ -2783,7 +2783,7 @@ func checkLinkTypeMutator(ctx android.BottomUpMutatorContext) {
// If a library has a vendor variant and is a (transitive) dependency of an LLNDK library,
// it is subject to be double loaded. Such lib should be explicitly marked as double_loadable: true
// or as vndk-sp (vndk: { enabled: true, support_system_process: true}).
func checkDoubleLoadableLibraries(ctx android.TopDownMutatorContext) {
func checkDoubleLoadableLibraries(ctx android.BottomUpMutatorContext) {
check := func(child, parent android.Module) bool {
to, ok := child.(*Module)
if !ok {
Expand Down
2 changes: 1 addition & 1 deletion cc/fuzz.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (fuzzer *fuzzer) props() []interface{} {
return []interface{}{&fuzzer.Properties}
}

func fuzzMutatorDeps(mctx android.TopDownMutatorContext) {
func fuzzMutatorDeps(mctx android.BottomUpMutatorContext) {
currentModule, ok := mctx.Module().(*Module)
if !ok {
return
Expand Down
6 changes: 3 additions & 3 deletions cc/sanitize.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ func (t SanitizerType) registerMutators(ctx android.RegisterMutatorsContext) {
switch t {
case cfi, Hwasan, Asan, tsan, Fuzzer, scs, Memtag_stack:
sanitizer := &sanitizerSplitMutator{t}
ctx.TopDown(t.variationName()+"_markapexes", sanitizer.markSanitizableApexesMutator)
ctx.BottomUp(t.variationName()+"_markapexes", sanitizer.markSanitizableApexesMutator)
ctx.Transition(t.variationName(), sanitizer)
case Memtag_heap, Memtag_globals, intOverflow:
// do nothing
Expand Down Expand Up @@ -1153,7 +1153,7 @@ type sanitizerSplitMutator struct {
// If an APEX is sanitized or not depends on whether it contains at least one
// sanitized module. Transition mutators cannot propagate information up the
// dependency graph this way, so we need an auxiliary mutator to do so.
func (s *sanitizerSplitMutator) markSanitizableApexesMutator(ctx android.TopDownMutatorContext) {
func (s *sanitizerSplitMutator) markSanitizableApexesMutator(ctx android.BottomUpMutatorContext) {
if sanitizeable, ok := ctx.Module().(Sanitizeable); ok {
enabled := sanitizeable.IsSanitizerEnabled(ctx.Config(), s.sanitizer.name())
ctx.VisitDirectDeps(func(dep android.Module) {
Expand Down Expand Up @@ -1355,7 +1355,7 @@ func (c *Module) IsSanitizerExplicitlyDisabled(t SanitizerType) bool {
}

// Propagate the ubsan minimal runtime dependency when there are integer overflow sanitized static dependencies.
func sanitizerRuntimeDepsMutator(mctx android.TopDownMutatorContext) {
func sanitizerRuntimeDepsMutator(mctx android.BottomUpMutatorContext) {
// 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 {
Expand Down

0 comments on commit da279cf

Please sign in to comment.