From a6ddb144f80487f2d6d1be03b601c0a1a75ffd00 Mon Sep 17 00:00:00 2001 From: Hsin-Yi Chen Date: Thu, 27 Oct 2022 14:55:42 +0800 Subject: [PATCH] Refactor the build rules for ABI diff - Move the logic that determines the versioned dump directories from pathForVndkRefAbiDump to linkSAbiDumpFiles. - Move sourceAbiDiff which generates the flags for ABI diff from builder.go to library.go. - Define two functions that generate the rules for cross-version and same-version ABI diff. Bug: 227282691 Test: make Change-Id: Ic33799e86dd9ae627f5680b70973f96e15c030e9 --- cc/androidmk.go | 22 ++------ cc/builder.go | 40 ++------------ cc/library.go | 142 +++++++++++++++++++++++++++++------------------- 3 files changed, 98 insertions(+), 106 deletions(-) diff --git a/cc/androidmk.go b/cc/androidmk.go index 58bb57ca4..aaf21e9c2 100644 --- a/cc/androidmk.go +++ b/cc/androidmk.go @@ -227,27 +227,17 @@ func (library *libraryDecorator) androidMkWriteExportedFlags(entries *android.An } } -func (library *libraryDecorator) getAbiDiffsForAndroidMkDeps() []string { - if library.static() { - return nil - } - var abiDiffs []string - if library.sAbiDiff.Valid() { - abiDiffs = append(abiDiffs, library.sAbiDiff.String()) - } - if library.prevSAbiDiff.Valid() { - abiDiffs = append(abiDiffs, library.prevSAbiDiff.String()) - } - return abiDiffs -} - func (library *libraryDecorator) androidMkEntriesWriteAdditionalDependenciesForSourceAbiDiff(entries *android.AndroidMkEntries) { - entries.AddStrings("LOCAL_ADDITIONAL_DEPENDENCIES", library.getAbiDiffsForAndroidMkDeps()...) + if !library.static() { + entries.AddPaths("LOCAL_ADDITIONAL_DEPENDENCIES", library.sAbiDiff) + } } // TODO(ccross): remove this once apex/androidmk.go is converted to AndroidMkEntries func (library *libraryDecorator) androidMkWriteAdditionalDependenciesForSourceAbiDiff(w io.Writer) { - fmt.Fprintln(w, "LOCAL_ADDITIONAL_DEPENDENCIES +=", strings.Join(library.getAbiDiffsForAndroidMkDeps(), " ")) + if !library.static() { + fmt.Fprintln(w, "LOCAL_ADDITIONAL_DEPENDENCIES +=", strings.Join(library.sAbiDiff.Strings(), " ")) + } } func (library *libraryDecorator) AndroidMkEntries(ctx AndroidMkContext, entries *android.AndroidMkEntries) { diff --git a/cc/builder.go b/cc/builder.go index 46cea0b0b..cf20923d0 100644 --- a/cc/builder.go +++ b/cc/builder.go @@ -923,47 +923,17 @@ func unzipRefDump(ctx android.ModuleContext, zippedRefDump android.Path, baseNam return outputFile } -// sourceAbiDiff registers a build statement to compare linked sAbi dump files (.lsdump). -func sourceAbiDiff(ctx android.ModuleContext, inputDump, referenceDump android.Path, - baseName string, diffFlags []string, prevVersion int, - checkAllApis, isLlndkOrNdk, isVndkExt, previousVersionDiff bool) android.OptionalPath { +func transformAbiDumpToAbiDiff(ctx android.ModuleContext, inputDump, referenceDump android.Path, + baseName, nameExt string, extraFlags []string, errorMessage string) android.Path { var outputFile android.ModuleOutPath - if previousVersionDiff { - outputFile = android.PathForModuleOut(ctx, baseName+"."+strconv.Itoa(prevVersion)+".abidiff") + if nameExt != "" { + outputFile = android.PathForModuleOut(ctx, baseName+"."+nameExt+".abidiff") } else { outputFile = android.PathForModuleOut(ctx, baseName+".abidiff") } libName := strings.TrimSuffix(baseName, filepath.Ext(baseName)) - var extraFlags []string - if checkAllApis { - extraFlags = append(extraFlags, "-check-all-apis") - } else { - extraFlags = append(extraFlags, - "-allow-unreferenced-changes", - "-allow-unreferenced-elf-symbol-changes") - } - - var errorMessage string - if previousVersionDiff { - errorMessage = "error: Please follow https://android.googlesource.com/platform/development/+/master/vndk/tools/header-checker/README.md#configure-cross_version-abi-check to resolve the ABI difference between your source code and version " + strconv.Itoa(prevVersion) + "." - sourceVersion := prevVersion + 1 - extraFlags = append(extraFlags, "-target-version", strconv.Itoa(sourceVersion)) - } else { - errorMessage = "error: Please update ABI references with: $$ANDROID_BUILD_TOP/development/vndk/tools/header-checker/utils/create_reference_dumps.py -l " + libName - extraFlags = append(extraFlags, "-target-version", "current") - } - - if isLlndkOrNdk { - extraFlags = append(extraFlags, "-consider-opaque-types-different") - } - if isVndkExt || previousVersionDiff { - extraFlags = append(extraFlags, "-allow-extensions") - } - // TODO(b/232891473): Simplify the above logic with diffFlags. - extraFlags = append(extraFlags, diffFlags...) - ctx.Build(pctx, android.BuildParams{ Rule: sAbiDiff, Description: "header-abi-diff " + outputFile.Base(), @@ -978,7 +948,7 @@ func sourceAbiDiff(ctx android.ModuleContext, inputDump, referenceDump android.P "errorMessage": errorMessage, }, }) - return android.OptionalPathForPath(outputFile) + return outputFile } // Generate a rule for extracting a table of contents from a shared library (.so) diff --git a/cc/library.go b/cc/library.go index 0729ff4a5..527f92023 100644 --- a/cc/library.go +++ b/cc/library.go @@ -802,10 +802,7 @@ type libraryDecorator struct { sAbiOutputFile android.OptionalPath // Source Abi Diff - sAbiDiff android.OptionalPath - - // Source Abi Diff against previous SDK version - prevSAbiDiff android.OptionalPath + sAbiDiff android.Paths // Location of the static library in the sysroot. Empty if the library is // not included in the NDK. @@ -1796,10 +1793,10 @@ func (library *libraryDecorator) coverageOutputFilePath() android.OptionalPath { return library.coverageOutputFile } -// pathForVndkRefAbiDump returns an OptionalPath representing the path of the +// pathForRefAbiDump returns an OptionalPath representing the path of the // reference abi dump for the given module. This is not guaranteed to be valid. -func pathForVndkRefAbiDump(ctx android.ModuleInstallPathContext, version, fileName string, - isNdk, isVndk, isGzip bool) android.OptionalPath { +func pathForRefAbiDump(ctx android.ModuleInstallPathContext, + versionedDumpDir, fileName string, isGzip bool) android.OptionalPath { currentArchType := ctx.Arch().ArchType primaryArchType := ctx.Config().DevicePrimaryArchType() @@ -1808,17 +1805,6 @@ func pathForVndkRefAbiDump(ctx android.ModuleInstallPathContext, version, fileNa archName += "_" + primaryArchType.String() } - var dirName string - if isNdk { - dirName = "ndk" - } else if isVndk { - dirName = "vndk" - } else { - dirName = "platform" // opt-in libs - } - - binderBitness := ctx.DeviceConfig().BinderBitness() - var ext string if isGzip { ext = ".lsdump.gz" @@ -1826,18 +1812,25 @@ func pathForVndkRefAbiDump(ctx android.ModuleInstallPathContext, version, fileNa ext = ".lsdump" } - return android.ExistentPathForSource(ctx, "prebuilts", "abi-dumps", dirName, - version, binderBitness, archName, "source-based", + return android.ExistentPathForSource(ctx, versionedDumpDir, archName, "source-based", fileName+ext) } -func getRefAbiDumpFile(ctx ModuleContext, vndkVersion, fileName string) android.Path { - // The logic must be consistent with classifySourceAbiDump. - isNdk := ctx.isNdk(ctx.Config()) - isVndk := ctx.useVndk() && ctx.isVndk() +func getRefAbiDumpDir(isNdk, isVndk bool) string { + var dirName string + if isNdk { + dirName = "ndk" + } else if isVndk { + dirName = "vndk" + } else { + dirName = "platform" + } + return filepath.Join("prebuilts", "abi-dumps", dirName) +} - refAbiDumpTextFile := pathForVndkRefAbiDump(ctx, vndkVersion, fileName, isNdk, isVndk, false) - refAbiDumpGzipFile := pathForVndkRefAbiDump(ctx, vndkVersion, fileName, isNdk, isVndk, true) +func getRefAbiDumpFile(ctx ModuleContext, versionedDumpDir, fileName string) android.Path { + refAbiDumpTextFile := pathForRefAbiDump(ctx, versionedDumpDir, fileName, /* isGzip */ false) + refAbiDumpGzipFile := pathForRefAbiDump(ctx, versionedDumpDir, fileName, /* isGzip */ true) if refAbiDumpTextFile.Valid() { if refAbiDumpGzipFile.Valid() { @@ -1854,27 +1847,18 @@ func getRefAbiDumpFile(ctx ModuleContext, vndkVersion, fileName string) android. return nil } -func prevDumpRefVersion(ctx ModuleContext) int { +func prevRefAbiDumpVersion(ctx ModuleContext, dumpDir string) int { sdkVersionInt := ctx.Config().PlatformSdkVersion().FinalInt() sdkVersionStr := ctx.Config().PlatformSdkVersion().String() if ctx.Config().PlatformSdkFinal() { return sdkVersionInt - 1 } else { - var dirName string - - isNdk := ctx.isNdk(ctx.Config()) - if isNdk { - dirName = "ndk" - } else { - dirName = "platform" - } - // The platform SDK version can be upgraded before finalization while the corresponding abi dumps hasn't // been generated. Thus the Cross-Version Check chooses PLATFORM_SDK_VERION - 1 as previous version. // This situation could be identified by checking the existence of the PLATFORM_SDK_VERION dump directory. - refDumpDir := android.ExistentPathForSource(ctx, "prebuilts", "abi-dumps", dirName, sdkVersionStr) - if refDumpDir.Valid() { + versionedDumpDir := android.ExistentPathForSource(ctx, dumpDir, sdkVersionStr) + if versionedDumpDir.Valid() { return sdkVersionInt } else { return sdkVersionInt - 1 @@ -1895,6 +1879,54 @@ func currRefAbiDumpVersion(ctx ModuleContext, isVndk bool) string { } } +// sourceAbiDiff registers a build statement to compare linked sAbi dump files (.lsdump). +func (library *libraryDecorator) sourceAbiDiff(ctx android.ModuleContext, referenceDump android.Path, + baseName, nameExt string, isLlndkOrNdk, allowExtensions bool, + sourceVersion, errorMessage string) { + + sourceDump := library.sAbiOutputFile.Path() + + extraFlags := []string{"-target-version", sourceVersion} + if Bool(library.Properties.Header_abi_checker.Check_all_apis) { + extraFlags = append(extraFlags, "-check-all-apis") + } else { + extraFlags = append(extraFlags, + "-allow-unreferenced-changes", + "-allow-unreferenced-elf-symbol-changes") + } + if isLlndkOrNdk { + extraFlags = append(extraFlags, "-consider-opaque-types-different") + } + if allowExtensions { + extraFlags = append(extraFlags, "-allow-extensions") + } + extraFlags = append(extraFlags, library.Properties.Header_abi_checker.Diff_flags...) + + library.sAbiDiff = append( + library.sAbiDiff, + transformAbiDumpToAbiDiff(ctx, sourceDump, referenceDump, + baseName, nameExt, extraFlags, errorMessage)) +} + +func (library *libraryDecorator) crossVersionAbiDiff(ctx android.ModuleContext, referenceDump android.Path, + baseName string, isLlndkOrNdk bool, sourceVersion, prevVersion string) { + + errorMessage := "error: Please follow https://android.googlesource.com/platform/development/+/master/vndk/tools/header-checker/README.md#configure-cross_version-abi-check to resolve the ABI difference between your source code and version " + prevVersion + "." + + library.sourceAbiDiff(ctx, referenceDump, baseName, prevVersion, + isLlndkOrNdk, /* allowExtensions */ true, sourceVersion, errorMessage) +} + +func (library *libraryDecorator) sameVersionAbiDiff(ctx android.ModuleContext, referenceDump android.Path, + baseName string, isLlndkOrNdk, allowExtensions bool) { + + libName := strings.TrimSuffix(baseName, filepath.Ext(baseName)) + errorMessage := "error: Please update ABI references with: $$ANDROID_BUILD_TOP/development/vndk/tools/header-checker/utils/create_reference_dumps.py -l " + libName + + library.sourceAbiDiff(ctx, referenceDump, baseName, /* nameExt */ "", + isLlndkOrNdk, allowExtensions, "current", errorMessage) +} + func (library *libraryDecorator) linkSAbiDumpFiles(ctx ModuleContext, objs Objects, fileName string, soFile android.Path) { if library.sabi.shouldCreateSourceAbiDump() { exportIncludeDirs := library.flagExporter.exportedIncludes(ctx) @@ -1913,31 +1945,31 @@ func (library *libraryDecorator) linkSAbiDumpFiles(ctx ModuleContext, objs Objec addLsdumpPath(classifySourceAbiDump(ctx) + ":" + library.sAbiOutputFile.String()) + // The logic must be consistent with classifySourceAbiDump. isVndk := ctx.useVndk() && ctx.isVndk() isNdk := ctx.isNdk(ctx.Config()) isLlndk := ctx.isImplementationForLLNDKPublic() + dumpDir := getRefAbiDumpDir(isNdk, isVndk) + binderBitness := ctx.DeviceConfig().BinderBitness() // If NDK or PLATFORM library, check against previous version ABI. if !isVndk { - prevVersion := prevDumpRefVersion(ctx) - prevRefAbiDumpFile := getRefAbiDumpFile(ctx, strconv.Itoa(prevVersion), fileName) - if prevRefAbiDumpFile != nil { - library.prevSAbiDiff = sourceAbiDiff(ctx, library.sAbiOutputFile.Path(), - prevRefAbiDumpFile, fileName, - library.Properties.Header_abi_checker.Diff_flags, prevVersion, - Bool(library.Properties.Header_abi_checker.Check_all_apis), - isLlndk || isNdk, ctx.IsVndkExt(), true) + prevVersionInt := prevRefAbiDumpVersion(ctx, dumpDir) + prevVersion := strconv.Itoa(prevVersionInt) + prevDumpDir := filepath.Join(dumpDir, prevVersion, binderBitness) + prevDumpFile := getRefAbiDumpFile(ctx, prevDumpDir, fileName) + if prevDumpFile != nil { + library.crossVersionAbiDiff(ctx, prevDumpFile, + fileName, isLlndk || isNdk, + strconv.Itoa(prevVersionInt+1), prevVersion) } } - + // Check against the current version. currVersion := currRefAbiDumpVersion(ctx, isVndk) - refAbiDumpFile := getRefAbiDumpFile(ctx, currVersion, fileName) - if refAbiDumpFile != nil { - library.sAbiDiff = sourceAbiDiff(ctx, library.sAbiOutputFile.Path(), - refAbiDumpFile, fileName, - library.Properties.Header_abi_checker.Diff_flags, - /* unused if not previousVersionDiff */ 0, - Bool(library.Properties.Header_abi_checker.Check_all_apis), - isLlndk || isNdk, ctx.IsVndkExt(), false) + currDumpDir := filepath.Join(dumpDir, currVersion, binderBitness) + currDumpFile := getRefAbiDumpFile(ctx, currDumpDir, fileName) + if currDumpFile != nil { + library.sameVersionAbiDiff(ctx, currDumpFile, + fileName, isLlndk || isNdk, ctx.IsVndkExt()) } } }