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

@@ -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)

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))
 

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)

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) {
+			mctx.WalkDeps(func(child, parent android.Module) bool {
-				if d, ok := module.(*Module); ok && d.sanitize != nil &&
+				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) {
+func sanitizerRuntimeDepsMutator(mctx android.TopDownMutatorContext) {
-	return func(mctx android.TopDownMutatorContext) {
+	if c, ok := mctx.Module().(*Module); ok && c.sanitize != nil {
-		if c, ok := mctx.Module().(*Module); ok && c.sanitize != nil {
+		mctx.WalkDeps(func(child, parent android.Module) bool {
-			mctx.VisitDepsDepthFirst(func(module android.Module) {
+			if !isSanitizableDependencyTag(mctx.OtherModuleDependencyTag(child)) {
-				if d, ok := module.(*Module); ok && d.static() && d.sanitize != nil {
+				return false
+			}
+			if d, ok := child.(*Module); ok && d.static() && d.sanitize != nil {
 
-					if enableMinimalRuntime(d.sanitize) {
+				if enableMinimalRuntime(d.sanitize) {
-						// If a static dependency is built with the minimal runtime,
+					// If a static dependency is built with the minimal runtime,
-						// make sure we include the ubsan minimal runtime.
+					// make sure we include the ubsan minimal runtime.
-						c.sanitize.Properties.MinimalRuntimeDep = true
+					c.sanitize.Properties.MinimalRuntimeDep = true
-					} else if Bool(d.sanitize.Properties.Sanitize.Diag.Integer_overflow) ||
+				} else if Bool(d.sanitize.Properties.Sanitize.Diag.Integer_overflow) ||
-						len(d.sanitize.Properties.Sanitize.Diag.Misc_undefined) > 0 {
+					len(d.sanitize.Properties.Sanitize.Diag.Misc_undefined) > 0 {
-						// If a static dependency runs with full ubsan diagnostics,
+					// If a static dependency runs with full ubsan diagnostics,
-						// make sure we include the ubsan runtime.
+					// make sure we include the ubsan runtime.
-						c.sanitize.Properties.UbsanRuntimeDep = true
+					c.sanitize.Properties.UbsanRuntimeDep = true
-					}
 				}
-			})
+			}
-		}
+			return true
+		})
 	}
 }
 

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) {
+	ctx.WalkDeps(func(child, parent android.Module) bool {
-		if ctx.OtherModuleDependencyTag(module) != pythonLibTag {
+		if ctx.OtherModuleDependencyTag(child) != pythonLibTag {
-			return
+			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
 	})
 }