From fa049385b89273f8c0ab3cd76e5ff3724265ec6c Mon Sep 17 00:00:00 2001 From: Joel Galenson Date: Thu, 14 Jan 2021 16:03:18 -0800 Subject: [PATCH] Migrate Rust to LLVM coverage. Bug: 177675913 Test: Manually compile, run, and see output with llvm-cov. Change-Id: I66729cff87a848782e9fa1b95cbbc06318c5761a --- rust/androidmk.go | 7 ---- rust/binary.go | 17 +-------- rust/builder.go | 44 +--------------------- rust/compiler.go | 6 +-- rust/coverage.go | 8 ++-- rust/coverage_test.go | 85 ++----------------------------------------- rust/library.go | 21 ++--------- rust/rust.go | 14 +------ rust/rust_test.go | 2 +- 9 files changed, 19 insertions(+), 185 deletions(-) diff --git a/rust/androidmk.go b/rust/androidmk.go index e9da6fabe..030772722 100644 --- a/rust/androidmk.go +++ b/rust/androidmk.go @@ -82,9 +82,6 @@ func (binary *binaryDecorator) AndroidMk(ctx AndroidMkContext, ret *android.Andr } ret.Class = "EXECUTABLES" - ret.ExtraEntries = append(ret.ExtraEntries, func(entries *android.AndroidMkEntries) { - entries.SetOptionalPath("LOCAL_PREBUILT_COVERAGE_ARCHIVE", binary.coverageOutputZipFile) - }) } func (test *testDecorator) AndroidMk(ctx AndroidMkContext, ret *android.AndroidMkEntries) { @@ -117,10 +114,6 @@ func (library *libraryDecorator) AndroidMk(ctx AndroidMkContext, ret *android.An if library.distFile.Valid() { ret.DistFiles = android.MakeDefaultDistFiles(library.distFile.Path()) } - - ret.ExtraEntries = append(ret.ExtraEntries, func(entries *android.AndroidMkEntries) { - entries.SetOptionalPath("LOCAL_PREBUILT_COVERAGE_ARCHIVE", library.coverageOutputZipFile) - }) } func (procMacro *procMacroDecorator) AndroidMk(ctx AndroidMkContext, ret *android.AndroidMkEntries) { diff --git a/rust/binary.go b/rust/binary.go index ca07d07c1..2963a37af 100644 --- a/rust/binary.go +++ b/rust/binary.go @@ -121,7 +121,7 @@ func (binary *binaryDecorator) compile(ctx ModuleContext, flags Flags, deps Path flags.RustFlags = append(flags.RustFlags, deps.depFlags...) flags.LinkFlags = append(flags.LinkFlags, deps.linkObjects...) - outputs := TransformSrcToBinary(ctx, srcPath, deps, flags, outputFile, deps.linkDirs) + TransformSrcToBinary(ctx, srcPath, deps, flags, outputFile, deps.linkDirs) if binary.stripper.NeedsStrip(ctx) { strippedOutputFile := android.PathForModuleOut(ctx, "stripped", fileName) @@ -129,24 +129,9 @@ func (binary *binaryDecorator) compile(ctx ModuleContext, flags Flags, deps Path binary.strippedOutputFile = android.OptionalPathForPath(strippedOutputFile) } - binary.coverageFile = outputs.coverageFile - - var coverageFiles android.Paths - if outputs.coverageFile != nil { - coverageFiles = append(coverageFiles, binary.coverageFile) - } - if len(deps.coverageFiles) > 0 { - coverageFiles = append(coverageFiles, deps.coverageFiles...) - } - binary.coverageOutputZipFile = TransformCoverageFilesToZip(ctx, coverageFiles, binary.getStem(ctx)) - return outputFile } -func (binary *binaryDecorator) coverageOutputZipPath() android.OptionalPath { - return binary.coverageOutputZipFile -} - func (binary *binaryDecorator) autoDep(ctx BaseModuleContext) autoDep { // Binaries default to dylib dependencies for device, rlib for host. if binary.preferRlib() { diff --git a/rust/builder.go b/rust/builder.go index e5f0ab5e1..becebfe8f 100644 --- a/rust/builder.go +++ b/rust/builder.go @@ -19,10 +19,8 @@ import ( "strings" "github.com/google/blueprint" - "github.com/google/blueprint/pathtools" "android/soong/android" - "android/soong/cc" "android/soong/rust/config" ) @@ -76,8 +74,7 @@ var ( ) type buildOutput struct { - outputFile android.Path - coverageFile android.Path + outputFile android.Path } func init() { @@ -195,27 +192,6 @@ func transformSrctoCrate(ctx ModuleContext, main android.Path, deps PathDeps, fl implicits = append(implicits, deps.CrtBegin.Path(), deps.CrtEnd.Path()) } - if flags.Coverage { - var gcnoFile android.WritablePath - // Provide consistency with cc gcda output, see cc/builder.go init() - profileEmitArg := strings.TrimPrefix(cc.PwdPrefix(), "PWD=") + "/" - - if outputFile.Ext() != "" { - // rustc seems to split the output filename at the first '.' when determining the gcno filename - // so we need to do the same here. - gcnoFile = android.PathForModuleOut(ctx, strings.Split(outputFile.Base(), ".")[0]+".gcno") - rustcFlags = append(rustcFlags, "-Z profile-emit="+profileEmitArg+android.PathForModuleOut( - ctx, pathtools.ReplaceExtension(outputFile.Base(), "gcda")).String()) - } else { - gcnoFile = android.PathForModuleOut(ctx, outputFile.Base()+".gcno") - rustcFlags = append(rustcFlags, "-Z profile-emit="+profileEmitArg+android.PathForModuleOut( - ctx, outputFile.Base()+".gcda").String()) - } - - implicitOutputs = append(implicitOutputs, gcnoFile) - output.coverageFile = gcnoFile - } - if len(deps.SrcDeps) > 0 { genSubDir := "out/" moduleGenDir := android.PathForModuleOut(ctx, genSubDir) @@ -292,21 +268,3 @@ func transformSrctoCrate(ctx ModuleContext, main android.Path, deps PathDeps, fl return output } - -func TransformCoverageFilesToZip(ctx ModuleContext, - covFiles android.Paths, baseName string) android.OptionalPath { - if len(covFiles) > 0 { - - outputFile := android.PathForModuleOut(ctx, baseName+".zip") - - ctx.Build(pctx, android.BuildParams{ - Rule: zip, - Description: "zip " + outputFile.Base(), - Inputs: covFiles, - Output: outputFile, - }) - - return android.OptionalPathForPath(outputFile) - } - return android.OptionalPath{} -} diff --git a/rust/compiler.go b/rust/compiler.go index bcea6cccc..d45a5aa9a 100644 --- a/rust/compiler.go +++ b/rust/compiler.go @@ -136,8 +136,7 @@ type BaseCompilerProperties struct { } type baseCompiler struct { - Properties BaseCompilerProperties - coverageFile android.Path //rustc generates a single gcno file + Properties BaseCompilerProperties // Install related dir string @@ -147,8 +146,7 @@ type baseCompiler struct { path android.InstallPath location installLocation - coverageOutputZipFile android.OptionalPath - distFile android.OptionalPath + distFile android.OptionalPath // Stripped output file. If Valid(), this file will be installed instead of outputFile. strippedOutputFile android.OptionalPath } diff --git a/rust/coverage.go b/rust/coverage.go index 26375f507..dac526a36 100644 --- a/rust/coverage.go +++ b/rust/coverage.go @@ -20,7 +20,9 @@ import ( "android/soong/cc" ) -var CovLibraryName = "libprofile-extras" +var CovLibraryName = "libprofile-clang-extras" + +const profileInstrFlag = "-fprofile-instr-generate=/data/misc/trace/clang-%p-%m.profraw" type coverage struct { Properties cc.CoverageProperties @@ -53,9 +55,9 @@ func (cov *coverage) flags(ctx ModuleContext, flags Flags, deps PathDeps) (Flags flags.Coverage = true coverage := ctx.GetDirectDepWithTag(CovLibraryName, cc.CoverageDepTag).(cc.LinkableInterface) flags.RustFlags = append(flags.RustFlags, - "-Z profile", "-g", "-C opt-level=0", "-C link-dead-code") + "-Z instrument-coverage", "-g", "-C link-dead-code") flags.LinkFlags = append(flags.LinkFlags, - "--coverage", "-g", coverage.OutputFile().Path().String(), "-Wl,--wrap,getenv") + profileInstrFlag, "-g", coverage.OutputFile().Path().String(), "-Wl,--wrap,open") deps.StaticLibs = append(deps.StaticLibs, coverage.OutputFile().Path()) } diff --git a/rust/coverage_test.go b/rust/coverage_test.go index e7f873e91..4b6c9d4b7 100644 --- a/rust/coverage_test.go +++ b/rust/coverage_test.go @@ -56,7 +56,7 @@ func TestCoverageFlags(t *testing.T) { fizzCov := ctx.ModuleForTests("fizz_cov", "android_arm64_armv8-a_cov").Rule("rustc") buzzNoCov := ctx.ModuleForTests("buzzNoCov", "android_arm64_armv8-a").Rule("rustc") - rustcCoverageFlags := []string{"-Z profile", " -g ", "-C opt-level=0", "-C link-dead-code"} + rustcCoverageFlags := []string{"-Z instrument-coverage", " -g ", "-C link-dead-code"} for _, flag := range rustcCoverageFlags { missingErrorStr := "missing rustc flag '%s' for '%s' module with coverage enabled; rustcFlags: %#v" containsErrorStr := "contains rustc flag '%s' for '%s' module with coverage disabled; rustcFlags: %#v" @@ -75,7 +75,7 @@ func TestCoverageFlags(t *testing.T) { } } - linkCoverageFlags := []string{"--coverage", " -g "} + linkCoverageFlags := []string{"-fprofile-instr-generate=/data/misc/trace/clang-%p-%m.profraw", " -g "} for _, flag := range linkCoverageFlags { missingErrorStr := "missing rust linker flag '%s' for '%s' module with coverage enabled; rustcFlags: %#v" containsErrorStr := "contains rust linker flag '%s' for '%s' module with coverage disabled; rustcFlags: %#v" @@ -96,83 +96,6 @@ func TestCoverageFlags(t *testing.T) { } -// Test coverage files are included correctly -func TestCoverageZip(t *testing.T) { - ctx := testRustCov(t, ` - rust_library { - name: "libfoo", - srcs: ["foo.rs"], - rlibs: ["librlib"], - crate_name: "foo", - } - rust_ffi_static { - name: "libbaz", - srcs: ["foo.rs"], - rlibs: ["librlib"], - crate_name: "baz", - } - rust_library_rlib { - name: "librlib", - srcs: ["foo.rs"], - crate_name: "rlib", - } - rust_binary { - name: "fizz", - rlibs: ["librlib"], - static_libs: ["libbaz"], - srcs: ["foo.rs"], - } - cc_binary { - name: "buzz", - static_libs: ["libbaz"], - srcs: ["foo.c"], - } - cc_library { - name: "libbar", - static_libs: ["libbaz"], - compile_multilib: "64", - srcs: ["foo.c"], - }`) - - fizzZipInputs := ctx.ModuleForTests("fizz", "android_arm64_armv8-a_cov").Rule("zip").Inputs.Strings() - libfooZipInputs := ctx.ModuleForTests("libfoo", "android_arm64_armv8-a_dylib_cov").Rule("zip").Inputs.Strings() - buzzZipInputs := ctx.ModuleForTests("buzz", "android_arm64_armv8-a_cov").Rule("zip").Inputs.Strings() - libbarZipInputs := ctx.ModuleForTests("libbar", "android_arm64_armv8-a_shared_cov").Rule("zip").Inputs.Strings() - - // Make sure the expected number of input files are included. - if len(fizzZipInputs) != 3 { - t.Fatalf("expected only 3 coverage inputs for rust 'fizz' binary, got %#v: %#v", len(fizzZipInputs), fizzZipInputs) - } - if len(libfooZipInputs) != 2 { - t.Fatalf("expected only 2 coverage inputs for rust 'libfoo' library, got %#v: %#v", len(libfooZipInputs), libfooZipInputs) - } - if len(buzzZipInputs) != 2 { - t.Fatalf("expected only 2 coverage inputs for cc 'buzz' binary, got %#v: %#v", len(buzzZipInputs), buzzZipInputs) - } - if len(libbarZipInputs) != 2 { - t.Fatalf("expected only 2 coverage inputs for cc 'libbar' library, got %#v: %#v", len(libbarZipInputs), libbarZipInputs) - } - - // Make sure the expected inputs are provided to the zip rule. - if !android.SuffixInList(fizzZipInputs, "android_arm64_armv8-a_rlib_dylib-std_cov/librlib.gcno") || - !android.SuffixInList(fizzZipInputs, "android_arm64_armv8-a_static_cov/libbaz.gcno") || - !android.SuffixInList(fizzZipInputs, "android_arm64_armv8-a_cov/fizz.gcno") { - t.Fatalf("missing expected coverage files for rust 'fizz' binary: %#v", fizzZipInputs) - } - if !android.SuffixInList(libfooZipInputs, "android_arm64_armv8-a_rlib_dylib-std_cov/librlib.gcno") || - !android.SuffixInList(libfooZipInputs, "android_arm64_armv8-a_dylib_cov/libfoo.gcno") { - t.Fatalf("missing expected coverage files for rust 'fizz' binary: %#v", libfooZipInputs) - } - if !android.SuffixInList(buzzZipInputs, "android_arm64_armv8-a_cov/obj/foo.gcno") || - !android.SuffixInList(buzzZipInputs, "android_arm64_armv8-a_static_cov/libbaz.gcno") { - t.Fatalf("missing expected coverage files for cc 'buzz' binary: %#v", buzzZipInputs) - } - if !android.SuffixInList(libbarZipInputs, "android_arm64_armv8-a_static_cov/obj/foo.gcno") || - !android.SuffixInList(libbarZipInputs, "android_arm64_armv8-a_static_cov/libbaz.gcno") { - t.Fatalf("missing expected coverage files for cc 'libbar' library: %#v", libbarZipInputs) - } -} - func TestCoverageDeps(t *testing.T) { ctx := testRustCov(t, ` rust_binary { @@ -181,7 +104,7 @@ func TestCoverageDeps(t *testing.T) { }`) fizz := ctx.ModuleForTests("fizz", "android_arm64_armv8-a_cov").Rule("rustc") - if !strings.Contains(fizz.Args["linkFlags"], "libprofile-extras.a") { - t.Fatalf("missing expected coverage 'libprofile-extras' dependency in linkFlags: %#v", fizz.Args["linkFlags"]) + if !strings.Contains(fizz.Args["linkFlags"], "libprofile-clang-extras.a") { + t.Fatalf("missing expected coverage 'libprofile-clang-extras' dependency in linkFlags: %#v", fizz.Args["linkFlags"]) } } diff --git a/rust/library.go b/rust/library.go index 4ac52b428..643328564 100644 --- a/rust/library.go +++ b/rust/library.go @@ -452,26 +452,22 @@ func (library *libraryDecorator) compile(ctx ModuleContext, flags Flags, deps Pa 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 + TransformSrctoRlib(ctx, srcPath, deps, flags, outputFile, deps.linkDirs) } else if library.dylib() { 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 + TransformSrctoDylib(ctx, srcPath, deps, flags, outputFile, deps.linkDirs) } else if library.static() { 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 + TransformSrctoStatic(ctx, srcPath, deps, flags, outputFile, deps.linkDirs) } else if library.shared() { fileName = library.sharedLibFilename(ctx) outputFile = android.PathForModuleOut(ctx, fileName) - outputs := TransformSrctoShared(ctx, srcPath, deps, flags, outputFile, deps.linkDirs) - library.coverageFile = outputs.coverageFile + TransformSrctoShared(ctx, srcPath, deps, flags, outputFile, deps.linkDirs) } if !library.rlib() && library.stripper.NeedsStrip(ctx) { @@ -480,15 +476,6 @@ func (library *libraryDecorator) compile(ctx ModuleContext, flags Flags, deps Pa library.strippedOutputFile = android.OptionalPathForPath(strippedOutputFile) } - var coverageFiles android.Paths - if library.coverageFile != nil { - coverageFiles = append(coverageFiles, library.coverageFile) - } - if len(deps.coverageFiles) > 0 { - coverageFiles = append(coverageFiles, deps.coverageFiles...) - } - library.coverageOutputZipFile = TransformCoverageFilesToZip(ctx, coverageFiles, library.getStem(ctx)) - if library.rlib() || library.dylib() { library.flagExporter.exportLinkDirs(deps.linkDirs...) library.flagExporter.exportDepFlags(deps.depFlags...) diff --git a/rust/rust.go b/rust/rust.go index 1fa97af96..a2d029230 100644 --- a/rust/rust.go +++ b/rust/rust.go @@ -283,8 +283,6 @@ type PathDeps struct { depGeneratedHeaders android.Paths depSystemIncludePaths android.Paths - coverageFiles android.Paths - CrtBegin android.OptionalPath CrtEnd android.OptionalPath @@ -506,15 +504,7 @@ func (mod *Module) OutputFile() android.OptionalPath { func (mod *Module) CoverageFiles() android.Paths { if mod.compiler != nil { - if !mod.compiler.nativeCoverage() { - return android.Paths{} - } - if library, ok := mod.compiler.(*libraryDecorator); ok { - if library.coverageFile != nil { - return android.Paths{library.coverageFile} - } - return android.Paths{} - } + return android.Paths{} } panic(fmt.Errorf("CoverageFiles called on non-library module: %q", mod.BaseModuleName())) } @@ -817,7 +807,6 @@ func (mod *Module) depsToPaths(ctx android.ModuleContext) PathDeps { ctx.ModuleErrorf("mod %q not an rlib library", depName+rustDep.Properties.SubName) return } - depPaths.coverageFiles = append(depPaths.coverageFiles, rustDep.CoverageFiles()...) directRlibDeps = append(directRlibDeps, rustDep) mod.Properties.AndroidMkRlibs = append(mod.Properties.AndroidMkRlibs, depName+rustDep.Properties.SubName) case procMacroDepTag: @@ -893,7 +882,6 @@ func (mod *Module) depsToPaths(ctx android.ModuleContext) PathDeps { depPaths.depSystemIncludePaths = append(depPaths.depSystemIncludePaths, exportedInfo.SystemIncludeDirs...) depPaths.depClangFlags = append(depPaths.depClangFlags, exportedInfo.Flags...) depPaths.depGeneratedHeaders = append(depPaths.depGeneratedHeaders, exportedInfo.GeneratedHeaders...) - depPaths.coverageFiles = append(depPaths.coverageFiles, ccDep.CoverageFiles()...) directStaticLibDeps = append(directStaticLibDeps, ccDep) mod.Properties.AndroidMkStaticLibs = append(mod.Properties.AndroidMkStaticLibs, depName) case cc.IsSharedDepTag(depTag): diff --git a/rust/rust_test.go b/rust/rust_test.go index 48c8d74f6..fc7f47e08 100644 --- a/rust/rust_test.go +++ b/rust/rust_test.go @@ -134,7 +134,7 @@ func (tctx *testRustCtx) enableCoverage(t *testing.T) { if tctx.config == nil { t.Fatalf("tctx.config not been generated yet. Please call generateConfig first.") } - tctx.config.TestProductVariables.GcovCoverage = proptools.BoolPtr(true) + tctx.config.TestProductVariables.ClangCoverage = proptools.BoolPtr(true) tctx.config.TestProductVariables.Native_coverage = proptools.BoolPtr(true) tctx.config.TestProductVariables.NativeCoveragePaths = []string{"*"} }