diff options
author | 2024-11-08 16:16:50 +0000 | |
---|---|---|
committer | 2025-02-05 03:13:01 +0000 | |
commit | 1f10f684516b131b5e6aebdf9b7915cf07409510 (patch) | |
tree | e4c5d3f6092fedc33807da4532b023bad3759a92 | |
parent | e068e0cb169335d5c76a27aba7f77088c52fb060 (diff) |
rust: Propagate static libs correctly
A dylib link is a final link, so we should not propagate staticlibs
through them. Most of the time this made no difference, but this
was breaking edge cases on coverage builds.
This CL breaks apart linkObjects so we can more finely tune which
dependencies we propagate and which we should not.
rlibs should only propagate non-whole static librares, while rlibs and
dylibs propagate whole static libs.
Bug: 377932175
Test: m
Test: New Soong tests
Change-Id: Ib2ae3dcb7d775a57bd4b1715ec528f48cc0ca026
-rw-r--r-- | rust/binary.go | 5 | ||||
-rw-r--r-- | rust/library.go | 16 | ||||
-rw-r--r-- | rust/rust.go | 107 | ||||
-rw-r--r-- | rust/rust_test.go | 175 |
4 files changed, 265 insertions, 38 deletions
diff --git a/rust/binary.go b/rust/binary.go index 3c7a48274..5a03d91c2 100644 --- a/rust/binary.go +++ b/rust/binary.go @@ -139,7 +139,10 @@ func (binary *binaryDecorator) compile(ctx ModuleContext, flags Flags, deps Path flags.RustFlags = append(flags.RustFlags, deps.depFlags...) flags.LinkFlags = append(flags.LinkFlags, deps.depLinkFlags...) - flags.LinkFlags = append(flags.LinkFlags, deps.linkObjects...) + flags.LinkFlags = append(flags.LinkFlags, deps.rustLibObjects...) + flags.LinkFlags = append(flags.LinkFlags, deps.sharedLibObjects...) + flags.LinkFlags = append(flags.LinkFlags, deps.staticLibObjects...) + flags.LinkFlags = append(flags.LinkFlags, deps.wholeStaticLibObjects...) if binary.stripper.NeedsStrip(ctx) { strippedOutputFile := outputFile diff --git a/rust/library.go b/rust/library.go index 95e7099a6..dd194374f 100644 --- a/rust/library.go +++ b/rust/library.go @@ -671,7 +671,10 @@ func (library *libraryDecorator) compile(ctx ModuleContext, flags Flags, deps Pa flags.RustFlags = append(flags.RustFlags, deps.depFlags...) flags.LinkFlags = append(flags.LinkFlags, deps.depLinkFlags...) - flags.LinkFlags = append(flags.LinkFlags, deps.linkObjects...) + flags.LinkFlags = append(flags.LinkFlags, deps.rustLibObjects...) + flags.LinkFlags = append(flags.LinkFlags, deps.sharedLibObjects...) + flags.LinkFlags = append(flags.LinkFlags, deps.staticLibObjects...) + flags.LinkFlags = append(flags.LinkFlags, deps.wholeStaticLibObjects...) if String(library.Properties.Version_script) != "" { if String(library.Properties.Extra_exported_symbols) != "" { @@ -719,9 +722,17 @@ func (library *libraryDecorator) compile(ctx ModuleContext, flags Flags, deps Pa ret.kytheFile = TransformSrctoShared(ctx, crateRootPath, deps, flags, outputFile).kytheFile } + // rlibs and dylibs propagate their shared, whole static, and rustlib dependencies if library.rlib() || library.dylib() { library.flagExporter.exportLinkDirs(deps.linkDirs...) - library.flagExporter.exportLinkObjects(deps.linkObjects...) + library.flagExporter.exportRustLibs(deps.rustLibObjects...) + library.flagExporter.exportSharedLibs(deps.sharedLibObjects...) + library.flagExporter.exportWholeStaticLibs(deps.wholeStaticLibObjects...) + } + + // rlibs also propagate their staticlibs dependencies + if library.rlib() { + library.flagExporter.exportStaticLibs(deps.staticLibObjects...) } // Since we have FFI rlibs, we need to collect their includes as well @@ -756,6 +767,7 @@ func (library *libraryDecorator) compile(ctx ModuleContext, flags Flags, deps Pa } cc.AddStubDependencyProviders(ctx) + // Set our flagexporter provider to export relevant Rust flags library.flagExporter.setProvider(ctx) return ret diff --git a/rust/rust.go b/rust/rust.go index 5cc8c07ec..f4fda2224 100644 --- a/rust/rust.go +++ b/rust/rust.go @@ -497,8 +497,11 @@ type PathDeps struct { // linkDirs are link paths passed via -L to rustc. linkObjects are objects passed directly to the linker // Both of these are exported and propagate to dependencies. - linkDirs []string - linkObjects []string + linkDirs []string + rustLibObjects []string + staticLibObjects []string + wholeStaticLibObjects []string + sharedLibObjects []string // exportedLinkDirs are exported linkDirs for direct rlib dependencies to // cc_library_static dependants of rlibs. @@ -531,7 +534,10 @@ type RustLibrary struct { type exportedFlagsProducer interface { exportLinkDirs(...string) - exportLinkObjects(...string) + exportRustLibs(...string) + exportStaticLibs(...string) + exportWholeStaticLibs(...string) + exportSharedLibs(...string) } type xref interface { @@ -539,23 +545,41 @@ type xref interface { } type flagExporter struct { - linkDirs []string - ccLinkDirs []string - linkObjects []string + linkDirs []string + ccLinkDirs []string + rustLibPaths []string + staticLibObjects []string + sharedLibObjects []string + wholeStaticLibObjects []string } func (flagExporter *flagExporter) exportLinkDirs(dirs ...string) { flagExporter.linkDirs = android.FirstUniqueStrings(append(flagExporter.linkDirs, dirs...)) } -func (flagExporter *flagExporter) exportLinkObjects(flags ...string) { - flagExporter.linkObjects = android.FirstUniqueStrings(append(flagExporter.linkObjects, flags...)) +func (flagExporter *flagExporter) exportRustLibs(flags ...string) { + flagExporter.rustLibPaths = android.FirstUniqueStrings(append(flagExporter.rustLibPaths, flags...)) +} + +func (flagExporter *flagExporter) exportStaticLibs(flags ...string) { + flagExporter.staticLibObjects = android.FirstUniqueStrings(append(flagExporter.staticLibObjects, flags...)) +} + +func (flagExporter *flagExporter) exportSharedLibs(flags ...string) { + flagExporter.sharedLibObjects = android.FirstUniqueStrings(append(flagExporter.sharedLibObjects, flags...)) +} + +func (flagExporter *flagExporter) exportWholeStaticLibs(flags ...string) { + flagExporter.wholeStaticLibObjects = android.FirstUniqueStrings(append(flagExporter.wholeStaticLibObjects, flags...)) } func (flagExporter *flagExporter) setProvider(ctx ModuleContext) { android.SetProvider(ctx, FlagExporterInfoProvider, FlagExporterInfo{ - LinkDirs: flagExporter.linkDirs, - LinkObjects: flagExporter.linkObjects, + LinkDirs: flagExporter.linkDirs, + RustLibObjects: flagExporter.rustLibPaths, + StaticLibObjects: flagExporter.staticLibObjects, + WholeStaticLibObjects: flagExporter.wholeStaticLibObjects, + SharedLibPaths: flagExporter.sharedLibObjects, }) } @@ -566,9 +590,12 @@ func NewFlagExporter() *flagExporter { } type FlagExporterInfo struct { - Flags []string - LinkDirs []string // TODO: this should be android.Paths - LinkObjects []string // TODO: this should be android.Paths + Flags []string + LinkDirs []string + RustLibObjects []string + StaticLibObjects []string + WholeStaticLibObjects []string + SharedLibPaths []string } var FlagExporterInfoProvider = blueprint.NewProvider[FlagExporterInfo]() @@ -1547,10 +1574,14 @@ func (mod *Module) depsToPaths(ctx android.ModuleContext) PathDeps { } exportedInfo, _ := android.OtherModuleProvider(ctx, dep, FlagExporterInfoProvider) - //Append the dependencies exportedDirs, except for proc-macros which target a different arch/OS + + //Append the dependencies exported objects, except for proc-macros which target a different arch/OS if depTag != procMacroDepTag { depPaths.depFlags = append(depPaths.depFlags, exportedInfo.Flags...) - depPaths.linkObjects = append(depPaths.linkObjects, exportedInfo.LinkObjects...) + depPaths.rustLibObjects = append(depPaths.rustLibObjects, exportedInfo.RustLibObjects...) + depPaths.sharedLibObjects = append(depPaths.sharedLibObjects, exportedInfo.SharedLibPaths...) + depPaths.staticLibObjects = append(depPaths.staticLibObjects, exportedInfo.StaticLibObjects...) + depPaths.wholeStaticLibObjects = append(depPaths.wholeStaticLibObjects, exportedInfo.WholeStaticLibObjects...) depPaths.linkDirs = append(depPaths.linkDirs, exportedInfo.LinkDirs...) } @@ -1583,8 +1614,8 @@ func (mod *Module) depsToPaths(ctx android.ModuleContext) PathDeps { return } } - linkObject := linkableInfo.OutputFile - if !linkObject.Valid() { + ccLibPath := linkableInfo.OutputFile + if !ccLibPath.Valid() { if !ctx.Config().AllowMissingDependencies() { ctx.ModuleErrorf("Invalid output file when adding dep %q to %q", depName, ctx.ModuleName()) } else { @@ -1593,7 +1624,7 @@ func (mod *Module) depsToPaths(ctx android.ModuleContext) PathDeps { return } - linkPath := linkPathFromFilePath(linkObject.Path()) + linkPath := linkPathFromFilePath(ccLibPath.Path()) exportDep := false switch { @@ -1602,20 +1633,25 @@ func (mod *Module) depsToPaths(ctx android.ModuleContext) PathDeps { // rustc will bundle static libraries when they're passed with "-lstatic=<lib>". This will fail // if the library is not prefixed by "lib". if mod.Binary() { - // Binaries may sometimes need to link whole static libraries that don't start with 'lib'. // Since binaries don't need to 'rebundle' these like libraries and only use these for the // final linkage, pass the args directly to the linker to handle these cases. - depPaths.depLinkFlags = append(depPaths.depLinkFlags, []string{"-Wl,--whole-archive", linkObject.Path().String(), "-Wl,--no-whole-archive"}...) - } else if libName, ok := libNameFromFilePath(linkObject.Path()); ok { - depPaths.depFlags = append(depPaths.depFlags, "-lstatic="+libName) + depPaths.depLinkFlags = append(depPaths.depLinkFlags, []string{"-Wl,--whole-archive", ccLibPath.Path().String(), "-Wl,--no-whole-archive"}...) + } else if libName, ok := libNameFromFilePath(ccLibPath.Path()); ok { + depPaths.depFlags = append(depPaths.depFlags, "-lstatic:+whole-archive="+libName) + depPaths.depLinkFlags = append(depPaths.depLinkFlags, ccLibPath.Path().String()) } else { ctx.ModuleErrorf("'%q' cannot be listed as a whole_static_library in Rust modules unless the output is prefixed by 'lib'", depName, ctx.ModuleName()) } } - // Add this to linkObjects to pass the library directly to the linker as well. This propagates - // to dependencies to avoid having to redeclare static libraries for dependents of the dylib variant. - depPaths.linkObjects = append(depPaths.linkObjects, linkObject.String()) + if cc.IsWholeStaticLib(depTag) { + // Add whole staticlibs to wholeStaticLibObjects to propagate to Rust all dependents. + depPaths.wholeStaticLibObjects = append(depPaths.wholeStaticLibObjects, ccLibPath.String()) + } else { + // Otherwise add to staticLibObjects, which only propagate through rlibs to their dependents. + depPaths.staticLibObjects = append(depPaths.staticLibObjects, ccLibPath.String()) + + } depPaths.linkDirs = append(depPaths.linkDirs, linkPath) exportedInfo, _ := android.OtherModuleProvider(ctx, dep, cc.FlagExporterInfoProvider) @@ -1647,8 +1683,8 @@ func (mod *Module) depsToPaths(ctx android.ModuleContext) PathDeps { // Re-get linkObject as ChooseStubOrImpl actually tells us which // object (either from stub or non-stub) to use. - linkObject = android.OptionalPathForPath(sharedLibraryInfo.SharedLibrary) - if !linkObject.Valid() { + ccLibPath = android.OptionalPathForPath(sharedLibraryInfo.SharedLibrary) + if !ccLibPath.Valid() { if !ctx.Config().AllowMissingDependencies() { ctx.ModuleErrorf("Invalid output file when adding dep %q to %q", depName, ctx.ModuleName()) } else { @@ -1656,10 +1692,10 @@ func (mod *Module) depsToPaths(ctx android.ModuleContext) PathDeps { } return } - linkPath = linkPathFromFilePath(linkObject.Path()) + linkPath = linkPathFromFilePath(ccLibPath.Path()) depPaths.linkDirs = append(depPaths.linkDirs, linkPath) - depPaths.linkObjects = append(depPaths.linkObjects, linkObject.String()) + depPaths.sharedLibObjects = append(depPaths.sharedLibObjects, ccLibPath.String()) depPaths.depIncludePaths = append(depPaths.depIncludePaths, exportedInfo.IncludeDirs...) depPaths.depSystemIncludePaths = append(depPaths.depSystemIncludePaths, exportedInfo.SystemIncludeDirs...) depPaths.depClangFlags = append(depPaths.depClangFlags, exportedInfo.Flags...) @@ -1678,15 +1714,15 @@ func (mod *Module) depsToPaths(ctx android.ModuleContext) PathDeps { depPaths.depGeneratedHeaders = append(depPaths.depGeneratedHeaders, exportedInfo.GeneratedHeaders...) mod.Properties.AndroidMkHeaderLibs = append(mod.Properties.AndroidMkHeaderLibs, makeLibName) case depTag == cc.CrtBeginDepTag: - depPaths.CrtBegin = append(depPaths.CrtBegin, linkObject.Path()) + depPaths.CrtBegin = append(depPaths.CrtBegin, ccLibPath.Path()) case depTag == cc.CrtEndDepTag: - depPaths.CrtEnd = append(depPaths.CrtEnd, linkObject.Path()) + depPaths.CrtEnd = append(depPaths.CrtEnd, ccLibPath.Path()) } - // Make sure these dependencies are propagated + // Make sure shared dependencies are propagated if lib, ok := mod.compiler.(exportedFlagsProducer); ok && exportDep { lib.exportLinkDirs(linkPath) - lib.exportLinkObjects(linkObject.String()) + lib.exportSharedLibs(ccLibPath.String()) } } else { switch { @@ -1769,7 +1805,10 @@ func (mod *Module) depsToPaths(ctx android.ModuleContext) PathDeps { // Dedup exported flags from dependencies depPaths.linkDirs = android.FirstUniqueStrings(depPaths.linkDirs) - depPaths.linkObjects = android.FirstUniqueStrings(depPaths.linkObjects) + depPaths.rustLibObjects = android.FirstUniqueStrings(depPaths.rustLibObjects) + depPaths.staticLibObjects = android.FirstUniqueStrings(depPaths.staticLibObjects) + depPaths.wholeStaticLibObjects = android.FirstUniqueStrings(depPaths.wholeStaticLibObjects) + depPaths.sharedLibObjects = android.FirstUniqueStrings(depPaths.sharedLibObjects) depPaths.depFlags = android.FirstUniqueStrings(depPaths.depFlags) depPaths.depClangFlags = android.FirstUniqueStrings(depPaths.depClangFlags) depPaths.depIncludePaths = android.FirstUniquePaths(depPaths.depIncludePaths) diff --git a/rust/rust_test.go b/rust/rust_test.go index 9f65dec41..858c4db95 100644 --- a/rust/rust_test.go +++ b/rust/rust_test.go @@ -204,7 +204,7 @@ func TestDepsTracking(t *testing.T) { t.Errorf("Static library dependency not detected (dependency missing from AndroidMkStaticLibs)") } - if !strings.Contains(rustc.Args["rustcFlags"], "-lstatic=wholestatic") { + if !strings.Contains(rustc.Args["rustcFlags"], "-lstatic:+whole-archive=wholestatic") { t.Errorf("-lstatic flag not being passed to rustc for static library %#v", rustc.Args["rustcFlags"]) } @@ -575,3 +575,176 @@ func TestStdLinkMismatch(t *testing.T) { } `) } + +func TestRustLinkPropagation(t *testing.T) { + // Test static and whole static propagation behavior + // + // Whole static libs propagate through rlibs and through dylibs to + // dependencies further down. rustc does not re-export whole-archived + // static libs for dylibs, so this simulates re-exporting those symbols. + // + // Static libs only propagate through rlibs to some final dylib. We propagate + // normal static libs because we allow rustlib dependencies to represent + // either rlibs or dylibs. Not propagating static libs through rlibs would + // mean we'd need to always redeclare static libs throughout a dependency tree + // We don't propagate past dylibs because they represent a final link. + + ctx := testRust(t, ` + rust_library_rlib { + name: "librlib1", + crate_name: "rlib1", + srcs: ["src/lib.rs"], + static_libs: ["libcc_static_rlib1"], + whole_static_libs: ["libcc_whole_static_rlib1"], + } + + rust_library_dylib { + name: "libdylib1", + crate_name: "dylib1", + static_libs: ["libcc_static_dylib1"], + srcs: ["src/lib.rs"], + whole_static_libs: ["libcc_whole_static_dylib1"], + } + + rust_library_rlib { + name: "librlib2", + crate_name: "rlib2", + srcs: ["src/lib.rs"], + rlibs: ["librlib1"], + static_libs: ["libcc_static_rlib2"], + whole_static_libs: ["libcc_whole_static_rlib2"], + } + + rust_library_dylib { + name: "libdylib2", + crate_name: "dylib2", + srcs: ["src/lib.rs"], + rlibs: ["librlib1"], + rustlibs: ["libdylib1"], + static_libs: ["libcc_static_dylib2"], + whole_static_libs: ["libcc_whole_static_dylib2"], + } + + cc_library_static { + name: "libcc_static_rlib1", + srcs:["foo.c"], + } + + cc_library_static { + name: "libcc_static_rlib2", + srcs:["foo.c"], + } + + cc_library_static { + name: "libcc_static_dylib1", + srcs:["foo.c"], + } + + cc_library_static { + name: "libcc_static_dylib2", + srcs:["foo.c"], + } + + cc_library_static { + name: "libcc_whole_static_rlib1", + srcs:["foo.c"], + } + + cc_library_static { + name: "libcc_whole_static_rlib2", + srcs:["foo.c"], + } + + cc_library_static { + name: "libcc_whole_static_dylib1", + srcs:["foo.c"], + } + + cc_library_static { + name: "libcc_whole_static_dylib2", + srcs:["foo.c"], + } + + rust_library_rlib { + name: "librlib3", + crate_name: "rlib3", + srcs: ["src/lib.rs"], + rlibs: ["librlib2"], + } + + rust_library_dylib { + name: "libdylib3", + crate_name: "dylib3", + srcs: ["src/lib.rs"], + rlibs: ["librlib2"], + rustlibs: ["libdylib2"], + } + `) + + librlib3 := ctx.ModuleForTests("librlib3", "android_arm64_armv8-a_rlib_dylib-std").Rule("rustc") + libdylib3 := ctx.ModuleForTests("libdylib3", "android_arm64_armv8-a_dylib").Rule("rustc") + + // Test static lib propagation from: + // rlib -> rlib + if !strings.Contains(librlib3.Args["linkFlags"], "libcc_static_rlib2.a") { + t.Errorf("direct dependency static lib not propagating from rlib to rlib; linkFlags %#v", + librlib3.Args["linkFlags"]) + } + // rlib -> rlib -> rlib + if !strings.Contains(librlib3.Args["linkFlags"], "libcc_static_rlib1.a") { + t.Errorf("indirect dependency static lib not propagating from rlib to rlib: linkFlags %#v", + librlib3.Args["linkFlags"]) + } + // rlib -> rlib -> dylib + if !strings.Contains(libdylib3.Args["linkFlags"], "libcc_static_rlib1.a") { + t.Errorf("indirect dependency static lib not propagating from rlib to dylib: linkFlags %#v", + libdylib3.Args["linkFlags"]) + } + // rlib -> dylib + if !strings.Contains(libdylib3.Args["linkFlags"], "libcc_static_rlib2.a") { + t.Errorf("direct dependency static lib not propagating from rlib to dylib: linkFlags: %#v", + libdylib3.Args["linkFlags"]) + } + // dylib -> dylib (negative case, should not propagate) + if strings.Contains(libdylib3.Args["linkFlags"], "libcc_static_dylib2.a") { + t.Errorf("direct dependency static lib propagating from dylib to dylib: linkFlags: %#v", + libdylib3.Args["linkFlags"]) + } + // dylib -> dylib -> dylib (negative case, should not propagate) + if strings.Contains(libdylib3.Args["linkFlags"], "libcc_static_dylib1.a") { + t.Errorf("indirect dependency static lib propagating from dylib to dylib: linkFlags: %#v", + libdylib3.Args["linkFlags"]) + } + + // Test whole static lib propagation from: + // rlib -> rlib + if !strings.Contains(librlib3.Args["linkFlags"], "libcc_whole_static_rlib2.a") { + t.Errorf("direct dependency whole static lib not propagating from rlib to rlib: linkFlags %#v", + librlib3.Args["linkFlags"]) + } + // rlib -> rlib -> rlib + if !strings.Contains(librlib3.Args["linkFlags"], "libcc_whole_static_rlib1.a") { + t.Errorf("indirect dependency whole static lib not propagating from rlib to rlib: linkFlags %#v", + librlib3.Args["linkFlags"]) + } + // rlib -> dylib + if !strings.Contains(libdylib3.Args["linkFlags"], "libcc_whole_static_rlib2.a") { + t.Errorf("direct dependency whole static lib not propagating from rlib to dylib: linkFlags %#v", + libdylib3.Args["linkFlags"]) + } + // rlib -> rlib -> dylib + if !strings.Contains(libdylib3.Args["linkFlags"], "libcc_whole_static_rlib1.a") { + t.Errorf("indirect dependency whole static lib not propagating from rlib to dylib: linkFlags %#v", + libdylib3.Args["linkFlags"]) + } + // dylib -> dylib + if !strings.Contains(libdylib3.Args["linkFlags"], "libcc_whole_static_dylib2.a") { + t.Errorf("direct dependency whole static lib not propagating from dylib to dylib: linkFlags %#v", + libdylib3.Args["linkFlags"]) + } + // dylib -> dylib -> dylib + if !strings.Contains(libdylib3.Args["linkFlags"], "libcc_whole_static_dylib1.a") { + t.Errorf("indirect dependency whole static lib not propagating from dylib to dylib: linkFlags %#v", + libdylib3.Args["linkFlags"]) + } +} |