From fabaff6bd74cda382dd7d5152df7362bffe233a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thi=C3=A9baud=20Weksteen?= Date: Thu, 27 Aug 2020 13:48:36 +0200 Subject: [PATCH] rust: strip libraries and binaries Reuses the cc.Stripper logic. Abstracts Stripper to avoid the spreading of references to the cc package. rustc requires unstripped libraries (precisely, with the `.rustc` section) when building dependent targets. Contrary to cc, the output of a compiler module will remain unstripped and only an extra build rule will be added. This rule will be referenced at install time (in baseCompiler.install or androidmk). This change drastically reduces the size of the installed libraries: (unstripped, from out/target/product/crosshatch/system) $ find . -name \*.dylib.so -print0 | du -c --files0-from=- 149996 total (stripped, with this change) $ find . -name \*.dylib.so -print0 | du -c --files0-from=- 42380 total Bug: 153430439 Test: cd external/rust; mma Change-Id: I94fd8bbcec97e0610aa325d3db4460be84d01734 --- rust/Android.bp | 1 + rust/androidmk.go | 11 +++++++---- rust/binary.go | 14 ++++++++++---- rust/binary_test.go | 30 ++++++++++++++++++++++++++++++ rust/compiler.go | 11 ++++++++--- rust/library.go | 22 +++++++++++++++------- rust/library_test.go | 32 ++++++++++++++++++++++++++++++++ rust/prebuilt.go | 3 --- rust/proc_macro.go | 3 --- rust/rust.go | 4 ++-- rust/strip.go | 30 ++++++++++++++++++++++++++++++ rust/test.go | 4 ++-- 12 files changed, 137 insertions(+), 28 deletions(-) create mode 100644 rust/strip.go diff --git a/rust/Android.bp b/rust/Android.bp index bbeb28d0b..ad2bbd86b 100644 --- a/rust/Android.bp +++ b/rust/Android.bp @@ -20,6 +20,7 @@ bootstrap_go_package { "proc_macro.go", "project_json.go", "rust.go", + "strip.go", "source_provider.go", "test.go", "testing.go", diff --git a/rust/androidmk.go b/rust/androidmk.go index fda0a2579..10f10d86f 100644 --- a/rust/androidmk.go +++ b/rust/androidmk.go @@ -96,7 +96,6 @@ func (binary *binaryDecorator) AndroidMk(ctx AndroidMkContext, ret *android.Andr ret.Class = "EXECUTABLES" ret.Extra = append(ret.Extra, func(w io.Writer, outputFile android.Path) { - fmt.Fprintln(w, "LOCAL_SOONG_UNSTRIPPED_BINARY :=", binary.unstrippedOutputFile.String()) if binary.coverageOutputZipFile.Valid() { fmt.Fprintln(w, "LOCAL_PREBUILT_COVERAGE_ARCHIVE := "+binary.coverageOutputZipFile.String()) } @@ -139,9 +138,6 @@ func (library *libraryDecorator) AndroidMk(ctx AndroidMkContext, ret *android.An } ret.Extra = append(ret.Extra, func(w io.Writer, outputFile android.Path) { - if !library.rlib() { - fmt.Fprintln(w, "LOCAL_SOONG_UNSTRIPPED_BINARY :=", library.unstrippedOutputFile.String()) - } if library.coverageOutputZipFile.Valid() { fmt.Fprintln(w, "LOCAL_PREBUILT_COVERAGE_ARCHIVE := "+library.coverageOutputZipFile.String()) } @@ -180,12 +176,19 @@ func (bindgen *bindgenDecorator) AndroidMk(ctx AndroidMkContext, ret *android.An } func (compiler *baseCompiler) AndroidMk(ctx AndroidMkContext, ret *android.AndroidMkData) { + var unstrippedOutputFile android.OptionalPath // Soong installation is only supported for host modules. Have Make // installation trigger Soong installation. if ctx.Target().Os.Class == android.Host { ret.OutputFile = android.OptionalPathForPath(compiler.path) + } else if compiler.strippedOutputFile.Valid() { + unstrippedOutputFile = ret.OutputFile + ret.OutputFile = compiler.strippedOutputFile } ret.Extra = append(ret.Extra, func(w io.Writer, outputFile android.Path) { + if compiler.strippedOutputFile.Valid() { + fmt.Fprintln(w, "LOCAL_SOONG_UNSTRIPPED_BINARY :=", unstrippedOutputFile) + } path, file := filepath.Split(compiler.path.ToMakePath().String()) stem, suffix, _ := android.SplitFileExt(file) fmt.Fprintln(w, "LOCAL_MODULE_SUFFIX := "+suffix) diff --git a/rust/binary.go b/rust/binary.go index d287a06ef..1d02453db 100644 --- a/rust/binary.go +++ b/rust/binary.go @@ -28,6 +28,7 @@ type BinaryCompilerProperties struct { type binaryDecorator struct { *baseCompiler + stripper Stripper Properties BinaryCompilerProperties } @@ -86,7 +87,8 @@ func (binary *binaryDecorator) compilerDeps(ctx DepsContext, deps Deps) Deps { func (binary *binaryDecorator) compilerProps() []interface{} { return append(binary.baseCompiler.compilerProps(), - &binary.Properties) + &binary.Properties, + &binary.stripper.StripProperties) } func (binary *binaryDecorator) nativeCoverage() bool { @@ -95,16 +97,20 @@ func (binary *binaryDecorator) nativeCoverage() bool { func (binary *binaryDecorator) compile(ctx ModuleContext, flags Flags, deps PathDeps) android.Path { fileName := binary.getStem(ctx) + ctx.toolchain().ExecutableSuffix() - srcPath, _ := srcPathFromModuleSrcs(ctx, binary.baseCompiler.Properties.Srcs) - outputFile := android.PathForModuleOut(ctx, fileName) - binary.unstrippedOutputFile = outputFile flags.RustFlags = append(flags.RustFlags, deps.depFlags...) flags.LinkFlags = append(flags.LinkFlags, deps.linkObjects...) outputs := TransformSrcToBinary(ctx, srcPath, deps, flags, outputFile, deps.linkDirs) + + if binary.stripper.NeedsStrip(ctx) { + strippedOutputFile := android.PathForModuleOut(ctx, "stripped", fileName) + binary.stripper.StripExecutableOrSharedLib(ctx, outputFile, strippedOutputFile) + binary.strippedOutputFile = android.OptionalPathForPath(strippedOutputFile) + } + binary.coverageFile = outputs.coverageFile var coverageFiles android.Paths diff --git a/rust/binary_test.go b/rust/binary_test.go index 2fc38ed74..cfef57a77 100644 --- a/rust/binary_test.go +++ b/rust/binary_test.go @@ -96,3 +96,33 @@ func TestLinkObjects(t *testing.T) { t.Errorf("missing shared dependency 'libfoo.so' in linkFlags: %#v", linkFlags) } } + +// Test that stripped versions are correctly generated and used. +func TestStrippedBinary(t *testing.T) { + ctx := testRust(t, ` + rust_binary { + name: "foo", + srcs: ["foo.rs"], + } + rust_binary { + name: "bar", + srcs: ["foo.rs"], + strip: { + none: true + } + } + `) + + foo := ctx.ModuleForTests("foo", "android_arm64_armv8-a") + foo.Output("stripped/foo") + // Check that the `cp` rules is using the stripped version as input. + cp := foo.Rule("android.Cp") + if !strings.HasSuffix(cp.Input.String(), "stripped/foo") { + t.Errorf("installed binary not based on stripped version: %v", cp.Input) + } + + fizzBar := ctx.ModuleForTests("bar", "android_arm64_armv8-a").MaybeOutput("stripped/bar") + if fizzBar.Rule != nil { + t.Errorf("stripped version of bar has been generated") + } +} diff --git a/rust/compiler.go b/rust/compiler.go index 2600f4d0f..ddf1fac3a 100644 --- a/rust/compiler.go +++ b/rust/compiler.go @@ -129,8 +129,9 @@ type baseCompiler struct { location installLocation coverageOutputZipFile android.OptionalPath - unstrippedOutputFile android.Path distFile android.OptionalPath + // Stripped output file. If Valid(), this file will be installed instead of outputFile. + strippedOutputFile android.OptionalPath } func (compiler *baseCompiler) Disabled() bool { @@ -269,8 +270,12 @@ func (compiler *baseCompiler) nativeCoverage() bool { return false } -func (compiler *baseCompiler) install(ctx ModuleContext, file android.Path) { - compiler.path = ctx.InstallFile(compiler.installDir(ctx), file.Base(), file) +func (compiler *baseCompiler) install(ctx ModuleContext) { + path := ctx.RustModule().outputFile + if compiler.strippedOutputFile.Valid() { + path = compiler.strippedOutputFile + } + compiler.path = ctx.InstallFile(compiler.installDir(ctx), path.Path().Base(), path.Path()) } func (compiler *baseCompiler) getStem(ctx ModuleContext) string { diff --git a/rust/library.go b/rust/library.go index 450c9d429..a44293307 100644 --- a/rust/library.go +++ b/rust/library.go @@ -78,6 +78,7 @@ type LibraryMutatedProperties struct { type libraryDecorator struct { *baseCompiler *flagExporter + stripper Stripper Properties LibraryCompilerProperties MutatedProperties LibraryMutatedProperties @@ -338,7 +339,8 @@ func NewRustLibrary(hod android.HostOrDeviceSupported) (*Module, *libraryDecorat func (library *libraryDecorator) compilerProps() []interface{} { return append(library.baseCompiler.compilerProps(), &library.Properties, - &library.MutatedProperties) + &library.MutatedProperties, + &library.stripper.StripProperties) } func (library *libraryDecorator) compilerDeps(ctx DepsContext, deps Deps) Deps { @@ -371,7 +373,8 @@ func (library *libraryDecorator) compilerFlags(ctx ModuleContext, flags Flags) F } func (library *libraryDecorator) compile(ctx ModuleContext, flags Flags, deps PathDeps) android.Path { - var outputFile android.WritablePath + var outputFile android.ModuleOutPath + var fileName string var srcPath android.Path if library.sourceProvider != nil { @@ -391,31 +394,37 @@ func (library *libraryDecorator) compile(ctx ModuleContext, flags Flags, deps Pa } if library.rlib() { - fileName := library.getStem(ctx) + ctx.toolchain().RlibSuffix() + fileName = library.getStem(ctx) + ctx.toolchain().RlibSuffix() outputFile = android.PathForModuleOut(ctx, fileName) outputs := TransformSrctoRlib(ctx, srcPath, deps, flags, outputFile, deps.linkDirs) library.coverageFile = outputs.coverageFile } else if library.dylib() { - fileName := library.getStem(ctx) + ctx.toolchain().DylibSuffix() + fileName = library.getStem(ctx) + ctx.toolchain().DylibSuffix() outputFile = android.PathForModuleOut(ctx, fileName) outputs := TransformSrctoDylib(ctx, srcPath, deps, flags, outputFile, deps.linkDirs) library.coverageFile = outputs.coverageFile } else if library.static() { - fileName := library.getStem(ctx) + ctx.toolchain().StaticLibSuffix() + fileName = library.getStem(ctx) + ctx.toolchain().StaticLibSuffix() outputFile = android.PathForModuleOut(ctx, fileName) outputs := TransformSrctoStatic(ctx, srcPath, deps, flags, outputFile, deps.linkDirs) library.coverageFile = outputs.coverageFile } else if library.shared() { - fileName := library.sharedLibFilename(ctx) + fileName = library.sharedLibFilename(ctx) outputFile = android.PathForModuleOut(ctx, fileName) outputs := TransformSrctoShared(ctx, srcPath, deps, flags, outputFile, deps.linkDirs) library.coverageFile = outputs.coverageFile } + if !library.rlib() && library.stripper.NeedsStrip(ctx) { + strippedOutputFile := android.PathForModuleOut(ctx, "stripped", fileName) + library.stripper.StripExecutableOrSharedLib(ctx, outputFile, strippedOutputFile) + library.strippedOutputFile = android.OptionalPathForPath(strippedOutputFile) + } + var coverageFiles android.Paths if library.coverageFile != nil { coverageFiles = append(coverageFiles, library.coverageFile) @@ -430,7 +439,6 @@ func (library *libraryDecorator) compile(ctx ModuleContext, flags Flags, deps Pa library.exportDepFlags(deps.depFlags...) library.exportLinkObjects(deps.linkObjects...) } - library.unstrippedOutputFile = outputFile return outputFile } diff --git a/rust/library_test.go b/rust/library_test.go index 0fd9e32f9..f1bc0507b 100644 --- a/rust/library_test.go +++ b/rust/library_test.go @@ -206,3 +206,35 @@ func TestAutoDeps(t *testing.T) { } } + +// Test that stripped versions are correctly generated and used. +func TestStrippedLibrary(t *testing.T) { + ctx := testRust(t, ` + rust_library_dylib { + name: "libfoo", + crate_name: "foo", + srcs: ["foo.rs"], + } + rust_library_dylib { + name: "libbar", + crate_name: "bar", + srcs: ["foo.rs"], + strip: { + none: true + } + } + `) + + foo := ctx.ModuleForTests("libfoo", "android_arm64_armv8-a_dylib") + foo.Output("stripped/libfoo.dylib.so") + // Check that the `cp` rule is using the stripped version as input. + cp := foo.Rule("android.Cp") + if !strings.HasSuffix(cp.Input.String(), "stripped/libfoo.dylib.so") { + t.Errorf("installed binary not based on stripped version: %v", cp.Input) + } + + fizzBar := ctx.ModuleForTests("libbar", "android_arm64_armv8-a_dylib").MaybeOutput("stripped/libbar.dylib.so") + if fizzBar.Rule != nil { + t.Errorf("stripped version of bar has been generated") + } +} diff --git a/rust/prebuilt.go b/rust/prebuilt.go index 3d081c113..f9c8934d6 100644 --- a/rust/prebuilt.go +++ b/rust/prebuilt.go @@ -99,9 +99,6 @@ func (prebuilt *prebuiltLibraryDecorator) compile(ctx ModuleContext, flags Flags if len(paths) > 0 { ctx.PropertyErrorf("srcs", "prebuilt libraries can only have one entry in srcs (the prebuilt path)") } - - prebuilt.unstrippedOutputFile = srcPath - return srcPath } diff --git a/rust/proc_macro.go b/rust/proc_macro.go index 748879cc8..0c6ec9953 100644 --- a/rust/proc_macro.go +++ b/rust/proc_macro.go @@ -66,9 +66,6 @@ func (procMacro *procMacroDecorator) compile(ctx ModuleContext, flags Flags, dep outputFile := android.PathForModuleOut(ctx, fileName) srcPath, _ := srcPathFromModuleSrcs(ctx, procMacro.baseCompiler.Properties.Srcs) - - procMacro.unstrippedOutputFile = outputFile - TransformSrctoProcMacro(ctx, srcPath, deps, flags, outputFile, deps.linkDirs) return outputFile } diff --git a/rust/rust.go b/rust/rust.go index 90329502d..84728df88 100644 --- a/rust/rust.go +++ b/rust/rust.go @@ -284,7 +284,7 @@ type compiler interface { crateName() string inData() bool - install(ctx ModuleContext, path android.Path) + install(ctx ModuleContext) relativeInstallPath() string nativeCoverage() bool @@ -704,7 +704,7 @@ func (mod *Module) GenerateAndroidBuildActions(actx android.ModuleContext) { mod.outputFile = android.OptionalPathForPath(outputFile) if mod.outputFile.Valid() && !mod.Properties.PreventInstall { - mod.compiler.install(ctx, mod.outputFile.Path()) + mod.compiler.install(ctx) } } } diff --git a/rust/strip.go b/rust/strip.go new file mode 100644 index 000000000..d1bbba696 --- /dev/null +++ b/rust/strip.go @@ -0,0 +1,30 @@ +// Copyright 2020 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package rust + +import ( + "android/soong/android" + "android/soong/cc" +) + +// Stripper encapsulates cc.Stripper. +type Stripper struct { + cc.Stripper +} + +func (s *Stripper) StripExecutableOrSharedLib(ctx ModuleContext, in android.Path, out android.ModuleOutPath) { + ccFlags := cc.StripFlags{Toolchain: ctx.RustModule().ccToolchain(ctx)} + s.Stripper.StripExecutableOrSharedLib(ctx, in, out, ccFlags) +} diff --git a/rust/test.go b/rust/test.go index 19802e392..d93fc313f 100644 --- a/rust/test.go +++ b/rust/test.go @@ -88,7 +88,7 @@ func (test *testDecorator) compilerProps() []interface{} { return append(test.binaryDecorator.compilerProps(), &test.Properties) } -func (test *testDecorator) install(ctx ModuleContext, file android.Path) { +func (test *testDecorator) install(ctx ModuleContext) { test.testConfig = tradefed.AutoGenRustTestConfig(ctx, test.Properties.Test_config, test.Properties.Test_config_template, @@ -103,7 +103,7 @@ func (test *testDecorator) install(ctx ModuleContext, file android.Path) { ctx.PropertyErrorf("no_named_install_directory", "Module install directory may only be disabled if relative_install_path is set") } - test.binaryDecorator.install(ctx, file) + test.binaryDecorator.install(ctx) } func (test *testDecorator) compilerFlags(ctx ModuleContext, flags Flags) Flags {