From 6b75360cbcc4c3e26507092cc910bf001465bef1 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Thu, 21 Jun 2018 13:03:07 -0700 Subject: [PATCH] Use WalkDeps instead of VisitDepsDepthFirst VisitDepsDepthFirst is almost never correct, as it can't query dependency tags of multiple dependencies between the same two modules. Replace VisitDepsDepthFirst with WalkDeps in sanitize.go and python.go. Also verify the dependency tag before continuing to recurse to ensure that they don't recurse through genrules and into unrelated modules. Test: m checkbuild Change-Id: I2f7560126f56b51a40ec39dfbdcc18b5891489f7 --- android/module.go | 3 +++ android/singleton.go | 2 ++ cc/cc.go | 2 +- cc/sanitize.go | 51 +++++++++++++++++++++++++++----------------- python/python.go | 23 +++++++++++++------- 5 files changed, 53 insertions(+), 28 deletions(-) diff --git a/android/module.go b/android/module.go index 3316a4441..b6220dc31 100644 --- a/android/module.go +++ b/android/module.go @@ -144,7 +144,9 @@ type ModuleContext interface { VisitDirectDeps(visit func(Module)) VisitDirectDepsWithTag(tag blueprint.DependencyTag, visit func(Module)) VisitDirectDepsIf(pred func(Module) bool, visit func(Module)) + // Deprecated: use WalkDeps instead to support multiple dependency tags on the same module VisitDepsDepthFirst(visit func(Module)) + // Deprecated: use WalkDeps instead to support multiple dependency tags on the same module VisitDepsDepthFirstIf(pred func(Module) bool, visit func(Module)) WalkDeps(visit func(Module, Module) bool) @@ -539,6 +541,7 @@ func (a *ModuleBase) computeInstallDeps( ctx blueprint.ModuleContext) Paths { result := Paths{} + // TODO(ccross): we need to use WalkDeps and have some way to know which dependencies require installation ctx.VisitDepsDepthFirstIf(isFileInstaller, func(m blueprint.Module) { fileInstaller := m.(fileInstaller) diff --git a/android/singleton.go b/android/singleton.go index f577b0a3c..fa1efdc0f 100644 --- a/android/singleton.go +++ b/android/singleton.go @@ -50,7 +50,9 @@ type SingletonContext interface { VisitAllModules(visit func(Module)) VisitAllModulesIf(pred func(Module) bool, visit func(Module)) + // Deprecated: use WalkDeps instead to support multiple dependency tags on the same module VisitDepsDepthFirst(module Module, visit func(Module)) + // Deprecated: use WalkDeps instead to support multiple dependency tags on the same module VisitDepsDepthFirstIf(module Module, pred func(Module) bool, visit func(Module)) diff --git a/cc/cc.go b/cc/cc.go index b7183d7ae..592f373aa 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -52,7 +52,7 @@ func init() { ctx.TopDown("tsan_deps", sanitizerDepsMutator(tsan)) ctx.BottomUp("tsan", sanitizerMutator(tsan)).Parallel() - ctx.TopDown("sanitize_runtime_deps", sanitizerRuntimeDepsMutator()) + ctx.TopDown("sanitize_runtime_deps", sanitizerRuntimeDepsMutator) ctx.BottomUp("coverage", coverageLinkingMutator).Parallel() ctx.TopDown("vndk_deps", sabiDepsMutator) diff --git a/cc/sanitize.go b/cc/sanitize.go index 38f4fc28d..080ac094b 100644 --- a/cc/sanitize.go +++ b/cc/sanitize.go @@ -21,6 +21,8 @@ import ( "strings" "sync" + "github.com/google/blueprint" + "android/soong/android" "android/soong/cc/config" ) @@ -610,43 +612,54 @@ func (sanitize *sanitize) isSanitizerEnabled(t sanitizerType) bool { return sanitizerVal != nil && *sanitizerVal == true } +func isSanitizableDependencyTag(tag blueprint.DependencyTag) bool { + t, ok := tag.(dependencyTag) + return ok && t.library || t == reuseObjTag +} + // Propagate asan requirements down from binaries func sanitizerDepsMutator(t sanitizerType) func(android.TopDownMutatorContext) { return func(mctx android.TopDownMutatorContext) { if c, ok := mctx.Module().(*Module); ok && c.sanitize.isSanitizerEnabled(t) { - mctx.VisitDepsDepthFirst(func(module android.Module) { - if d, ok := module.(*Module); ok && d.sanitize != nil && + mctx.WalkDeps(func(child, parent android.Module) bool { + if !isSanitizableDependencyTag(mctx.OtherModuleDependencyTag(child)) { + return false + } + if d, ok := child.(*Module); ok && d.sanitize != nil && !Bool(d.sanitize.Properties.Sanitize.Never) && !d.sanitize.isSanitizerExplicitlyDisabled(t) { if (t == cfi && d.static()) || t != cfi { d.sanitize.Properties.SanitizeDep = true } } + return true }) } } } // Propagate the ubsan minimal runtime dependency when there are integer overflow sanitized static dependencies. -func sanitizerRuntimeDepsMutator() func(android.TopDownMutatorContext) { - return func(mctx android.TopDownMutatorContext) { - if c, ok := mctx.Module().(*Module); ok && c.sanitize != nil { - mctx.VisitDepsDepthFirst(func(module android.Module) { - if d, ok := module.(*Module); ok && d.static() && d.sanitize != nil { +func sanitizerRuntimeDepsMutator(mctx android.TopDownMutatorContext) { + if c, ok := mctx.Module().(*Module); ok && c.sanitize != nil { + mctx.WalkDeps(func(child, parent android.Module) bool { + if !isSanitizableDependencyTag(mctx.OtherModuleDependencyTag(child)) { + return false + } + if d, ok := child.(*Module); ok && d.static() && d.sanitize != nil { - if enableMinimalRuntime(d.sanitize) { - // If a static dependency is built with the minimal runtime, - // make sure we include the ubsan minimal runtime. - c.sanitize.Properties.MinimalRuntimeDep = true - } else if Bool(d.sanitize.Properties.Sanitize.Diag.Integer_overflow) || - len(d.sanitize.Properties.Sanitize.Diag.Misc_undefined) > 0 { - // If a static dependency runs with full ubsan diagnostics, - // make sure we include the ubsan runtime. - c.sanitize.Properties.UbsanRuntimeDep = true - } + if enableMinimalRuntime(d.sanitize) { + // If a static dependency is built with the minimal runtime, + // make sure we include the ubsan minimal runtime. + c.sanitize.Properties.MinimalRuntimeDep = true + } else if Bool(d.sanitize.Properties.Sanitize.Diag.Integer_overflow) || + len(d.sanitize.Properties.Sanitize.Diag.Misc_undefined) > 0 { + // If a static dependency runs with full ubsan diagnostics, + // make sure we include the ubsan runtime. + c.sanitize.Properties.UbsanRuntimeDep = true } - }) - } + } + return true + }) } } diff --git a/python/python.go b/python/python.go index 4b9111f3b..feb17da04 100644 --- a/python/python.go +++ b/python/python.go @@ -574,32 +574,39 @@ func (p *Module) walkTransitiveDeps(ctx android.ModuleContext) { destToPyData[path.dest] = path.src.String() } + seen := make(map[android.Module]bool) + // visit all its dependencies in depth first. - ctx.VisitDepsDepthFirst(func(module android.Module) { - if ctx.OtherModuleDependencyTag(module) != pythonLibTag { - return + ctx.WalkDeps(func(child, parent android.Module) bool { + if ctx.OtherModuleDependencyTag(child) != pythonLibTag { + return false } + if seen[child] { + return false + } + seen[child] = true // Python modules only can depend on Python libraries. - if !isPythonLibModule(module) { + if !isPythonLibModule(child) { panic(fmt.Errorf( "the dependency %q of module %q is not Python library!", - ctx.ModuleName(), ctx.OtherModuleName(module))) + ctx.ModuleName(), ctx.OtherModuleName(child))) } - if dep, ok := module.(PythonDependency); ok { + if dep, ok := child.(PythonDependency); ok { srcs := dep.GetSrcsPathMappings() for _, path := range srcs { if !fillInMap(ctx, destToPySrcs, - path.dest, path.src.String(), ctx.ModuleName(), ctx.OtherModuleName(module)) { + path.dest, path.src.String(), ctx.ModuleName(), ctx.OtherModuleName(child)) { continue } } data := dep.GetDataPathMappings() for _, path := range data { fillInMap(ctx, destToPyData, - path.dest, path.src.String(), ctx.ModuleName(), ctx.OtherModuleName(module)) + path.dest, path.src.String(), ctx.ModuleName(), ctx.OtherModuleName(child)) } p.depsSrcsZips = append(p.depsSrcsZips, dep.GetSrcsZip()) } + return true }) }