From a70f067899c73b45558b862953ae229c313dcb12 Mon Sep 17 00:00:00 2001 From: Jooyung Han Date: Fri, 18 Jan 2019 15:20:43 +0900 Subject: [PATCH] Add checks for double_loadable dependencies Vendor-available libs can be double-loaded if LLNDK libs depend on them. Currently soong checks only 'direct' dependency bewteen LLNDK and VNDK lib. With this change, soong checks if every dependencies from LLNDK is also LLNDK or VNDK-SP or marked as 'double_loadable:true'. This change causes many libs to be marked as 'double_loadable'. Bug: 121280180 Test: m -j Change-Id: Ibc1879b6fd465a3141520abe0150018c3051c0a7 --- android/mutator.go | 14 +++ cc/cc.go | 57 +++++---- cc/cc_test.go | 306 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 356 insertions(+), 21 deletions(-) diff --git a/android/mutator.go b/android/mutator.go index e5f742f7f..509b67fa4 100644 --- a/android/mutator.go +++ b/android/mutator.go @@ -132,11 +132,15 @@ type TopDownMutatorContext interface { VisitDepsDepthFirst(visit func(Module)) VisitDepsDepthFirstIf(pred func(Module) bool, visit func(Module)) WalkDeps(visit func(Module, Module) bool) + // GetWalkPath is supposed to be called in visit function passed in WalkDeps() + // and returns a top-down dependency path from a start module to current child module. + GetWalkPath() []Module } type androidTopDownMutatorContext struct { blueprint.TopDownMutatorContext androidBaseContextImpl + walkPath []Module } type AndroidBottomUpMutator func(BottomUpMutatorContext) @@ -287,10 +291,16 @@ func (a *androidTopDownMutatorContext) VisitDepsDepthFirstIf(pred func(Module) b } func (a *androidTopDownMutatorContext) WalkDeps(visit func(Module, Module) bool) { + a.walkPath = []Module{a.Module()} a.TopDownMutatorContext.WalkDeps(func(child, parent blueprint.Module) bool { childAndroidModule, _ := child.(Module) parentAndroidModule, _ := parent.(Module) if childAndroidModule != nil && parentAndroidModule != nil { + // record walkPath before visit + for a.walkPath[len(a.walkPath)-1] != parentAndroidModule { + a.walkPath = a.walkPath[0 : len(a.walkPath)-1] + } + a.walkPath = append(a.walkPath, childAndroidModule) return visit(childAndroidModule, parentAndroidModule) } else { return false @@ -298,6 +308,10 @@ func (a *androidTopDownMutatorContext) WalkDeps(visit func(Module, Module) bool) }) } +func (a *androidTopDownMutatorContext) GetWalkPath() []Module { + return a.walkPath +} + func (a *androidTopDownMutatorContext) AppendProperties(props ...interface{}) { for _, p := range props { err := proptools.AppendMatchingProperties(a.Module().base().customizableProperties, diff --git a/cc/cc.go b/cc/cc.go index 117bbc2a8..284b58d48 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -68,6 +68,8 @@ func init() { ctx.TopDown("lto_deps", ltoDepsMutator) ctx.BottomUp("lto", ltoMutator).Parallel() + + ctx.TopDown("double_loadable", checkDoubleLoadableLibraries).Parallel() }) pctx.Import("android/soong/cc/config") @@ -1469,30 +1471,44 @@ func checkLinkType(ctx android.ModuleContext, from *Module, to *Module, tag depe } // Tests whether the dependent library is okay to be double loaded inside a single process. -// If a library is a member of VNDK and at the same time dependencies of an LLNDK library, -// it is subject to be double loaded. Such lib should be explicitly marked as double_loaded: true +// 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 checkDoubleLoadableLibries(ctx android.ModuleContext, from *Module, to *Module) { - if _, ok := from.linker.(*libraryDecorator); !ok { - return - } +func checkDoubleLoadableLibraries(ctx android.TopDownMutatorContext) { + check := func(child, parent android.Module) bool { + to, ok := child.(*Module) + if !ok { + // follow thru cc.Defaults, etc. + return true + } - if inList(ctx.ModuleName(), llndkLibraries) || - (from.useVndk() && Bool(from.VendorProperties.Double_loadable)) { - _, depIsLlndk := to.linker.(*llndkStubDecorator) - depIsVndkSp := false - if to.vndkdep != nil && to.vndkdep.isVndkSp() { - depIsVndkSp = true + if lib, ok := to.linker.(*libraryDecorator); !ok || !lib.shared() { + return false } - depIsVndk := false - if to.vndkdep != nil && to.vndkdep.isVndk() { - depIsVndk = true + + // if target lib has no vendor variant, keep checking dependency graph + if !to.hasVendorVariant() { + return true } - depIsDoubleLoadable := Bool(to.VendorProperties.Double_loadable) - if !depIsLlndk && !depIsVndkSp && !depIsDoubleLoadable && depIsVndk { - ctx.ModuleErrorf("links VNDK library %q that isn't double loadable (not also LL-NDK, "+ - "VNDK-SP, or explicitly marked as 'double_loadable').", - ctx.OtherModuleName(to)) + + if to.isVndkSp() || inList(child.Name(), llndkLibraries) || Bool(to.VendorProperties.Double_loadable) { + return false + } + + var stringPath []string + for _, m := range ctx.GetWalkPath() { + stringPath = append(stringPath, m.Name()) + } + ctx.ModuleErrorf("links a library %q which is not LL-NDK, "+ + "VNDK-SP, or explicitly marked as 'double_loadable:true'. "+ + "(dependency: %s)", ctx.OtherModuleName(to), strings.Join(stringPath, " -> ")) + return false + } + if module, ok := ctx.Module().(*Module); ok { + if lib, ok := module.linker.(*libraryDecorator); ok && lib.shared() { + if inList(ctx.ModuleName(), llndkLibraries) || Bool(module.VendorProperties.Double_loadable) { + ctx.WalkDeps(check) + } } } } @@ -1648,7 +1664,6 @@ func (c *Module) depsToPaths(ctx android.ModuleContext) PathDeps { } checkLinkType(ctx, c, ccDep, t) - checkDoubleLoadableLibries(ctx, c, ccDep) } var ptr *android.Paths diff --git a/cc/cc_test.go b/cc/cc_test.go index a0914c859..8c0bcfe80 100644 --- a/cc/cc_test.go +++ b/cc/cc_test.go @@ -71,6 +71,9 @@ func createTestContext(t *testing.T, config android.Config, bp string, os androi ctx.BottomUp("version", VersionMutator).Parallel() ctx.BottomUp("begin", BeginMutator).Parallel() }) + ctx.PostDepsMutators(func(ctx android.RegisterMutatorsContext) { + ctx.TopDown("double_loadable", checkDoubleLoadableLibraries).Parallel() + }) ctx.Register() // add some modules that are required by the compiler and/or linker @@ -428,6 +431,309 @@ func TestVndkDepError(t *testing.T) { nocrt: true, } `) + + // Check whether an error is emitted when a VNDK lib depends on a non-VNDK lib. + testCcError(t, "module \".*\" variant \".*\": \\(.*\\) should not link to \".*\"", ` + cc_library { + name: "libvndk", + vendor_available: true, + vndk: { + enabled: true, + }, + shared_libs: ["libnonvndk"], + nocrt: true, + } + + cc_library { + name: "libnonvndk", + vendor_available: true, + nocrt: true, + } + `) + + // Check whether an error is emitted when a VNDK-private lib depends on a non-VNDK lib. + testCcError(t, "module \".*\" variant \".*\": \\(.*\\) should not link to \".*\"", ` + cc_library { + name: "libvndkprivate", + vendor_available: false, + vndk: { + enabled: true, + }, + shared_libs: ["libnonvndk"], + nocrt: true, + } + + cc_library { + name: "libnonvndk", + vendor_available: true, + nocrt: true, + } + `) + + // Check whether an error is emitted when a VNDK-sp lib depends on a non-VNDK lib. + testCcError(t, "module \".*\" variant \".*\": \\(.*\\) should not link to \".*\"", ` + cc_library { + name: "libvndksp", + vendor_available: true, + vndk: { + enabled: true, + support_system_process: true, + }, + shared_libs: ["libnonvndk"], + nocrt: true, + } + + cc_library { + name: "libnonvndk", + vendor_available: true, + nocrt: true, + } + `) + + // Check whether an error is emitted when a VNDK-sp-private lib depends on a non-VNDK lib. + testCcError(t, "module \".*\" variant \".*\": \\(.*\\) should not link to \".*\"", ` + cc_library { + name: "libvndkspprivate", + vendor_available: false, + vndk: { + enabled: true, + support_system_process: true, + }, + shared_libs: ["libnonvndk"], + nocrt: true, + } + + cc_library { + name: "libnonvndk", + vendor_available: true, + nocrt: true, + } + `) +} + +func TestDoubleLoadbleDep(t *testing.T) { + // okay to link : LLNDK -> double_loadable VNDK + testCc(t, ` + cc_library { + name: "libllndk", + shared_libs: ["libdoubleloadable"], + } + + llndk_library { + name: "libllndk", + symbol_file: "", + } + + cc_library { + name: "libdoubleloadable", + vendor_available: true, + vndk: { + enabled: true, + }, + double_loadable: true, + } + `) + // okay to link : LLNDK -> VNDK-SP + testCc(t, ` + cc_library { + name: "libllndk", + shared_libs: ["libvndksp"], + } + + llndk_library { + name: "libllndk", + symbol_file: "", + } + + cc_library { + name: "libvndksp", + vendor_available: true, + vndk: { + enabled: true, + support_system_process: true, + }, + } + `) + // okay to link : double_loadable -> double_loadable + testCc(t, ` + cc_library { + name: "libdoubleloadable1", + shared_libs: ["libdoubleloadable2"], + vendor_available: true, + double_loadable: true, + } + + cc_library { + name: "libdoubleloadable2", + vendor_available: true, + double_loadable: true, + } + `) + // okay to link : double_loadable VNDK -> double_loadable VNDK private + testCc(t, ` + cc_library { + name: "libdoubleloadable", + vendor_available: true, + vndk: { + enabled: true, + }, + double_loadable: true, + shared_libs: ["libnondoubleloadable"], + } + + cc_library { + name: "libnondoubleloadable", + vendor_available: false, + vndk: { + enabled: true, + }, + double_loadable: true, + } + `) + // okay to link : LLNDK -> core-only -> vendor_available & double_loadable + testCc(t, ` + cc_library { + name: "libllndk", + shared_libs: ["libcoreonly"], + } + + llndk_library { + name: "libllndk", + symbol_file: "", + } + + cc_library { + name: "libcoreonly", + shared_libs: ["libvendoravailable"], + } + + // indirect dependency of LLNDK + cc_library { + name: "libvendoravailable", + vendor_available: true, + double_loadable: true, + } + `) +} + +func TestDoubleLoadableDepError(t *testing.T) { + // Check whether an error is emitted when a LLNDK depends on a non-double_loadable VNDK lib. + testCcError(t, "module \".*\" variant \".*\": link.* \".*\" which is not LL-NDK, VNDK-SP, .*double_loadable", ` + cc_library { + name: "libllndk", + shared_libs: ["libnondoubleloadable"], + } + + llndk_library { + name: "libllndk", + symbol_file: "", + } + + cc_library { + name: "libnondoubleloadable", + vendor_available: true, + vndk: { + enabled: true, + }, + } + `) + + // Check whether an error is emitted when a LLNDK depends on a non-double_loadable vendor_available lib. + testCcError(t, "module \".*\" variant \".*\": link.* \".*\" which is not LL-NDK, VNDK-SP, .*double_loadable", ` + cc_library { + name: "libllndk", + no_libgcc: true, + shared_libs: ["libnondoubleloadable"], + } + + llndk_library { + name: "libllndk", + symbol_file: "", + } + + cc_library { + name: "libnondoubleloadable", + vendor_available: true, + } + `) + + // Check whether an error is emitted when a double_loadable lib depends on a non-double_loadable vendor_available lib. + testCcError(t, "module \".*\" variant \".*\": link.* \".*\" which is not LL-NDK, VNDK-SP, .*double_loadable", ` + cc_library { + name: "libdoubleloadable", + vendor_available: true, + double_loadable: true, + shared_libs: ["libnondoubleloadable"], + } + + cc_library { + name: "libnondoubleloadable", + vendor_available: true, + } + `) + + // Check whether an error is emitted when a double_loadable lib depends on a non-double_loadable VNDK lib. + testCcError(t, "module \".*\" variant \".*\": link.* \".*\" which is not LL-NDK, VNDK-SP, .*double_loadable", ` + cc_library { + name: "libdoubleloadable", + vendor_available: true, + double_loadable: true, + shared_libs: ["libnondoubleloadable"], + } + + cc_library { + name: "libnondoubleloadable", + vendor_available: true, + vndk: { + enabled: true, + }, + } + `) + + // Check whether an error is emitted when a double_loadable VNDK depends on a non-double_loadable VNDK private lib. + testCcError(t, "module \".*\" variant \".*\": link.* \".*\" which is not LL-NDK, VNDK-SP, .*double_loadable", ` + cc_library { + name: "libdoubleloadable", + vendor_available: true, + vndk: { + enabled: true, + }, + double_loadable: true, + shared_libs: ["libnondoubleloadable"], + } + + cc_library { + name: "libnondoubleloadable", + vendor_available: false, + vndk: { + enabled: true, + }, + } + `) + + // Check whether an error is emitted when a LLNDK depends on a non-double_loadable indirectly. + testCcError(t, "module \".*\" variant \".*\": link.* \".*\" which is not LL-NDK, VNDK-SP, .*double_loadable", ` + cc_library { + name: "libllndk", + shared_libs: ["libcoreonly"], + } + + llndk_library { + name: "libllndk", + symbol_file: "", + } + + cc_library { + name: "libcoreonly", + shared_libs: ["libvendoravailable"], + } + + // indirect dependency of LLNDK + cc_library { + name: "libvendoravailable", + vendor_available: true, + } + `) } func TestVndkMustNotBeProductSpecific(t *testing.T) {