From 10566a035f648cd85b9af444b06cc2eb9975a9ef Mon Sep 17 00:00:00 2001 From: Martin Stjernholm Date: Tue, 24 Mar 2020 01:19:52 +0000 Subject: [PATCH] Propagate empty vs unspecified system_shared_libs correctly. Necessary to get correct prebuilts for many Bionic libs. Cleaned up numerious "system_shared_libs: []" from test fixtures, since they otherwise would need correction in the expected results, and it is better to have a single test focused on testing system_shared_libs propagation. Test: m nothing Bug: 152255951 Change-Id: If2e8a5296223e6281d833312660e8e9e4cd184c0 --- cc/binary_sdk_member.go | 5 +- cc/cc.go | 2 +- cc/library.go | 19 +++- cc/library_sdk_member.go | 7 +- cc/linker.go | 10 +- sdk/cc_sdk_test.go | 223 ++++++++++++++++++++++++++++++++++----- 6 files changed, 235 insertions(+), 31 deletions(-) diff --git a/cc/binary_sdk_member.go b/cc/binary_sdk_member.go index 9b3235c5d..1d9cc54f8 100644 --- a/cc/binary_sdk_member.go +++ b/cc/binary_sdk_member.go @@ -18,6 +18,7 @@ import ( "path/filepath" "android/soong/android" + "github.com/google/blueprint" ) @@ -137,7 +138,9 @@ func (p *nativeBinaryInfoProperties) AddToPropertySet(ctx android.SdkMemberConte propertySet.AddPropertyWithTag("shared_libs", p.SharedLibs, builder.SdkMemberReferencePropertyTag(false)) } - if len(p.SystemSharedLibs) > 0 { + // SystemSharedLibs needs to be propagated if it's a list, even if it's empty, + // so check for non-nil instead of nonzero length. + if p.SystemSharedLibs != nil { propertySet.AddPropertyWithTag("system_shared_libs", p.SystemSharedLibs, builder.SdkMemberReferencePropertyTag(false)) } } diff --git a/cc/cc.go b/cc/cc.go index 037b99cab..4ed27f4ee 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -378,7 +378,7 @@ type linker interface { type specifiedDeps struct { sharedLibs []string - systemSharedLibs []string + systemSharedLibs []string // Note nil and [] are semantically distinct. } type installer interface { diff --git a/cc/library.go b/cc/library.go index 346e7d84d..2ced2c5dc 100644 --- a/cc/library.go +++ b/cc/library.go @@ -749,10 +749,12 @@ func (library *libraryDecorator) compilerDeps(ctx DepsContext, deps Deps) Deps { func (library *libraryDecorator) linkerDeps(ctx DepsContext, deps Deps) Deps { if library.static() { + // Compare with nil because an empty list needs to be propagated. if library.StaticProperties.Static.System_shared_libs != nil { library.baseLinker.Properties.System_shared_libs = library.StaticProperties.Static.System_shared_libs } } else if library.shared() { + // Compare with nil because an empty list needs to be propagated. if library.SharedProperties.Shared.System_shared_libs != nil { library.baseLinker.Properties.System_shared_libs = library.SharedProperties.Shared.System_shared_libs } @@ -828,10 +830,21 @@ func (library *libraryDecorator) linkerSpecifiedDeps(specifiedDeps specifiedDeps } specifiedDeps.sharedLibs = append(specifiedDeps.sharedLibs, properties.Shared_libs...) - specifiedDeps.systemSharedLibs = append(specifiedDeps.systemSharedLibs, properties.System_shared_libs...) + + // Must distinguish nil and [] in system_shared_libs - ensure that [] in + // either input list doesn't come out as nil. + if specifiedDeps.systemSharedLibs == nil { + specifiedDeps.systemSharedLibs = properties.System_shared_libs + } else { + specifiedDeps.systemSharedLibs = append(specifiedDeps.systemSharedLibs, properties.System_shared_libs...) + } specifiedDeps.sharedLibs = android.FirstUniqueStrings(specifiedDeps.sharedLibs) - specifiedDeps.systemSharedLibs = android.FirstUniqueStrings(specifiedDeps.systemSharedLibs) + if len(specifiedDeps.systemSharedLibs) > 0 { + // Skip this if systemSharedLibs is either nil or [], to ensure they are + // retained. + specifiedDeps.systemSharedLibs = android.FirstUniqueStrings(specifiedDeps.systemSharedLibs) + } return specifiedDeps } @@ -1371,6 +1384,8 @@ func reuseStaticLibrary(mctx android.BottomUpMutatorContext, static, shared *Mod len(sharedCompiler.SharedProperties.Shared.Static_libs) == 0 && len(staticCompiler.StaticProperties.Static.Shared_libs) == 0 && len(sharedCompiler.SharedProperties.Shared.Shared_libs) == 0 && + // Compare System_shared_libs properties with nil because empty lists are + // semantically significant for them. staticCompiler.StaticProperties.Static.System_shared_libs == nil && sharedCompiler.SharedProperties.Shared.System_shared_libs == nil { diff --git a/cc/library_sdk_member.go b/cc/library_sdk_member.go index 49674552f..0a11af126 100644 --- a/cc/library_sdk_member.go +++ b/cc/library_sdk_member.go @@ -212,7 +212,9 @@ func addPossiblyArchSpecificProperties(sdkModuleContext android.ModuleContext, b outputProperties.AddPropertyWithTag("shared_libs", libInfo.SharedLibs, builder.SdkMemberReferencePropertyTag(false)) } - if len(libInfo.SystemSharedLibs) > 0 { + // SystemSharedLibs needs to be propagated if it's a list, even if it's empty, + // so check for non-nil instead of nonzero length. + if libInfo.SystemSharedLibs != nil { outputProperties.AddPropertyWithTag("system_shared_libs", libInfo.SystemSharedLibs, builder.SdkMemberReferencePropertyTag(false)) } @@ -327,7 +329,8 @@ type nativeLibInfoProperties struct { // This field is exported as its contents may not be arch specific. SharedLibs []string - // The set of system shared libraries + // The set of system shared libraries. Note nil and [] are semantically + // distinct - see BaseLinkerProperties.System_shared_libs. // // This field is exported as its contents may not be arch specific. SystemSharedLibs []string diff --git a/cc/linker.go b/cc/linker.go index ae5ee0ac3..f65d7c8d3 100644 --- a/cc/linker.go +++ b/cc/linker.go @@ -491,7 +491,15 @@ func (linker *baseLinker) link(ctx ModuleContext, func (linker *baseLinker) linkerSpecifiedDeps(specifiedDeps specifiedDeps) specifiedDeps { specifiedDeps.sharedLibs = append(specifiedDeps.sharedLibs, linker.Properties.Shared_libs...) - specifiedDeps.systemSharedLibs = append(specifiedDeps.systemSharedLibs, linker.Properties.System_shared_libs...) + + // Must distinguish nil and [] in system_shared_libs - ensure that [] in + // either input list doesn't come out as nil. + if specifiedDeps.systemSharedLibs == nil { + specifiedDeps.systemSharedLibs = linker.Properties.System_shared_libs + } else { + specifiedDeps.systemSharedLibs = append(specifiedDeps.systemSharedLibs, linker.Properties.System_shared_libs...) + } + return specifiedDeps } diff --git a/sdk/cc_sdk_test.go b/sdk/cc_sdk_test.go index ca40afd04..32176e5f7 100644 --- a/sdk/cc_sdk_test.go +++ b/sdk/cc_sdk_test.go @@ -48,7 +48,6 @@ func TestSdkIsCompileMultilibBoth(t *testing.T) { cc_library_shared { name: "sdkmember", srcs: ["Test.cpp"], - system_shared_libs: [], stl: "none", } `) @@ -178,13 +177,11 @@ func TestHostSdkWithCc(t *testing.T) { cc_library_host_shared { name: "sdkshared", - system_shared_libs: [], stl: "none", } cc_library_host_static { name: "sdkstatic", - system_shared_libs: [], stl: "none", } `) @@ -201,25 +198,21 @@ func TestSdkWithCc(t *testing.T) { cc_library_shared { name: "sdkshared", - system_shared_libs: [], stl: "none", } cc_library_static { name: "sdkstatic", - system_shared_libs: [], stl: "none", } cc_library { name: "sdkboth1", - system_shared_libs: [], stl: "none", } cc_library { name: "sdkboth2", - system_shared_libs: [], stl: "none", } `) @@ -295,7 +288,6 @@ func TestSnapshotWithCcDuplicateHeaders(t *testing.T) { "Test.cpp", ], export_include_dirs: ["include"], - system_shared_libs: [], stl: "none", } @@ -305,7 +297,6 @@ func TestSnapshotWithCcDuplicateHeaders(t *testing.T) { "Test.cpp", ], export_include_dirs: ["include"], - system_shared_libs: [], stl: "none", } `) @@ -342,7 +333,6 @@ func TestSnapshotWithCcSharedLibraryCommonProperties(t *testing.T) { export_system_include_dirs: ["arm64/include"], }, }, - system_shared_libs: [], stl: "none", } `) @@ -410,7 +400,6 @@ func TestSnapshotWithCcBinary(t *testing.T) { "Test.cpp", ], compile_multilib: "both", - system_shared_libs: [], stl: "none", } `) @@ -485,7 +474,6 @@ func TestMultipleHostOsTypesSnapshotWithCcBinary(t *testing.T) { "Test.cpp", ], compile_multilib: "both", - system_shared_libs: [], stl: "none", target: { windows: { @@ -586,7 +574,6 @@ func TestSnapshotWithCcSharedLibrary(t *testing.T) { aidl: { export_aidl_headers: true, }, - system_shared_libs: [], stl: "none", } `) @@ -673,7 +660,6 @@ func TestSnapshotWithCcSharedLibrarySharedLibs(t *testing.T) { srcs: [ "Test.cpp", ], - system_shared_libs: [], stl: "none", } @@ -714,7 +700,6 @@ func TestSnapshotWithCcSharedLibrarySharedLibs(t *testing.T) { }, }, }, - system_shared_libs: [], stl: "none", } `) @@ -864,7 +849,6 @@ func TestHostSnapshotWithCcSharedLibrary(t *testing.T) { aidl: { export_aidl_headers: true, }, - system_shared_libs: [], stl: "none", sdk_version: "minimum", } @@ -960,7 +944,6 @@ func TestMultipleHostOsTypesSnapshotWithCcSharedLibrary(t *testing.T) { srcs: [ "Test.cpp", ], - system_shared_libs: [], stl: "none", target: { windows: { @@ -1050,7 +1033,6 @@ func TestSnapshotWithCcStaticLibrary(t *testing.T) { aidl: { export_aidl_headers: true, }, - system_shared_libs: [], stl: "none", } `) @@ -1137,7 +1119,6 @@ func TestHostSnapshotWithCcStaticLibrary(t *testing.T) { aidl: { export_aidl_headers: true, }, - system_shared_libs: [], stl: "none", } `) @@ -1219,7 +1200,6 @@ func TestSnapshotWithCcLibrary(t *testing.T) { "Test.cpp", ], export_include_dirs: ["include"], - system_shared_libs: [], stl: "none", } `) @@ -1322,7 +1302,6 @@ func TestHostSnapshotWithMultiLib64(t *testing.T) { aidl: { export_aidl_headers: true, }, - system_shared_libs: [], stl: "none", } `) @@ -1393,7 +1372,6 @@ func TestSnapshotWithCcHeadersLibrary(t *testing.T) { cc_library_headers { name: "mynativeheaders", export_include_dirs: ["include"], - system_shared_libs: [], stl: "none", } `) @@ -1444,7 +1422,6 @@ func TestHostSnapshotWithCcHeadersLibrary(t *testing.T) { device_supported: false, host_supported: true, export_include_dirs: ["include"], - system_shared_libs: [], stl: "none", } `) @@ -1498,7 +1475,6 @@ func TestDeviceAndHostSnapshotWithCcHeadersLibrary(t *testing.T) { cc_library_headers { name: "mynativeheaders", host_supported: true, - system_shared_libs: [], stl: "none", export_system_include_dirs: ["include"], target: { @@ -1561,3 +1537,202 @@ include-host/HostTest.h -> include/include-host/HostTest.h `), ) } + +func TestSystemSharedLibPropagation(t *testing.T) { + result := testSdkWithCc(t, ` + sdk { + name: "mysdk", + native_shared_libs: ["sslnil", "sslempty", "sslnonempty"], + } + + cc_library { + name: "sslnil", + host_supported: true, + } + + cc_library { + name: "sslempty", + system_shared_libs: [], + } + + cc_library { + name: "sslnonempty", + system_shared_libs: ["sslnil"], + } + `) + + result.CheckSnapshot("mysdk", "", + checkAndroidBpContents(` +// This is auto-generated. DO NOT EDIT. + +cc_prebuilt_library_shared { + name: "mysdk_sslnil@current", + sdk_member_name: "sslnil", + installable: false, + arch: { + arm64: { + srcs: ["arm64/lib/sslnil.so"], + }, + arm: { + srcs: ["arm/lib/sslnil.so"], + }, + }, +} + +cc_prebuilt_library_shared { + name: "sslnil", + prefer: false, + arch: { + arm64: { + srcs: ["arm64/lib/sslnil.so"], + }, + arm: { + srcs: ["arm/lib/sslnil.so"], + }, + }, +} + +cc_prebuilt_library_shared { + name: "mysdk_sslempty@current", + sdk_member_name: "sslempty", + installable: false, + system_shared_libs: [], + arch: { + arm64: { + srcs: ["arm64/lib/sslempty.so"], + }, + arm: { + srcs: ["arm/lib/sslempty.so"], + }, + }, +} + +cc_prebuilt_library_shared { + name: "sslempty", + prefer: false, + system_shared_libs: [], + arch: { + arm64: { + srcs: ["arm64/lib/sslempty.so"], + }, + arm: { + srcs: ["arm/lib/sslempty.so"], + }, + }, +} + +cc_prebuilt_library_shared { + name: "mysdk_sslnonempty@current", + sdk_member_name: "sslnonempty", + installable: false, + system_shared_libs: ["mysdk_sslnil@current"], + arch: { + arm64: { + srcs: ["arm64/lib/sslnonempty.so"], + }, + arm: { + srcs: ["arm/lib/sslnonempty.so"], + }, + }, +} + +cc_prebuilt_library_shared { + name: "sslnonempty", + prefer: false, + system_shared_libs: ["sslnil"], + arch: { + arm64: { + srcs: ["arm64/lib/sslnonempty.so"], + }, + arm: { + srcs: ["arm/lib/sslnonempty.so"], + }, + }, +} + +sdk_snapshot { + name: "mysdk@current", + native_shared_libs: [ + "mysdk_sslnil@current", + "mysdk_sslempty@current", + "mysdk_sslnonempty@current", + ], +} +`)) + + result = testSdkWithCc(t, ` + sdk { + name: "mysdk", + host_supported: true, + native_shared_libs: ["sslvariants"], + } + + cc_library { + name: "sslvariants", + host_supported: true, + target: { + android: { + system_shared_libs: [], + }, + }, + } + `) + + result.CheckSnapshot("mysdk", "", + checkAndroidBpContents(` +// This is auto-generated. DO NOT EDIT. + +cc_prebuilt_library_shared { + name: "mysdk_sslvariants@current", + sdk_member_name: "sslvariants", + host_supported: true, + installable: false, + target: { + android: { + system_shared_libs: [], + }, + android_arm64: { + srcs: ["android/arm64/lib/sslvariants.so"], + }, + android_arm: { + srcs: ["android/arm/lib/sslvariants.so"], + }, + linux_glibc_x86_64: { + srcs: ["linux_glibc/x86_64/lib/sslvariants.so"], + }, + linux_glibc_x86: { + srcs: ["linux_glibc/x86/lib/sslvariants.so"], + }, + }, +} + +cc_prebuilt_library_shared { + name: "sslvariants", + prefer: false, + host_supported: true, + target: { + android: { + system_shared_libs: [], + }, + android_arm64: { + srcs: ["android/arm64/lib/sslvariants.so"], + }, + android_arm: { + srcs: ["android/arm/lib/sslvariants.so"], + }, + linux_glibc_x86_64: { + srcs: ["linux_glibc/x86_64/lib/sslvariants.so"], + }, + linux_glibc_x86: { + srcs: ["linux_glibc/x86/lib/sslvariants.so"], + }, + }, +} + +sdk_snapshot { + name: "mysdk@current", + host_supported: true, + native_shared_libs: ["mysdk_sslvariants@current"], +} +`)) +}