From bd88c882f6fe4d495c34eab6735d9f36fbd6514d Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Thu, 7 Apr 2022 23:32:19 +0100 Subject: [PATCH] Remove implementation details from stub flags in sdk snapshot Previously, the build applied the same filtering to remove implementation details from the sdk snapshot's stub-flags.csv file as it did for its all-flags.csv, i.e. removing the signatures that only had a "blocked" flag. Unfortunately, that had no effect on the stub flags as the implementation signatures had no flags, not a single blocked flag. That meant that the sdk snapshot's filtered-stub-flags.csv file contained a lot of implementation details. This change removes signatures from stub-flags.csv that have no flags which removes all implementation details from the sdk snapshot. Bug: 194063708 Test: atest --host verify_overlaps_test m out/soong/hiddenapi/hiddenapi-flags.csv m art-module-sdk # Check contents of its filtered-stub-flags.csv file Change-Id: I30edc77348fad118ea732e787ae8e206c8841f84 --- java/hiddenapi_modular.go | 59 ++++++++++++++++++---- scripts/hiddenapi/verify_overlaps.py | 32 +++++++++--- scripts/hiddenapi/verify_overlaps_test.py | 60 ++++++++++++++++++++--- 3 files changed, 128 insertions(+), 23 deletions(-) diff --git a/java/hiddenapi_modular.go b/java/hiddenapi_modular.go index 44cdfa5dd..534a8145f 100644 --- a/java/hiddenapi_modular.go +++ b/java/hiddenapi_modular.go @@ -295,6 +295,12 @@ func hiddenAPIRetrieveDexJarBuildPath(ctx android.ModuleContext, module android. return dexJar.Path() } +// HIDDENAPI_STUB_FLAGS_IMPL_FLAGS is the set of flags that identify implementation only signatures, +// i.e. those signatures that are not part of any API (including the hidden API). +var HIDDENAPI_STUB_FLAGS_IMPL_FLAGS = []string{} + +var HIDDENAPI_FLAGS_CSV_IMPL_FLAGS = []string{"blocked"} + // buildRuleToGenerateHiddenAPIStubFlagsFile creates a rule to create a hidden API stub flags file. // // The rule is initialized but not built so that the caller can modify it and select an appropriate @@ -345,7 +351,8 @@ func buildRuleToGenerateHiddenAPIStubFlagsFile(ctx android.BuilderContext, name, // If there are stub flag files that have been generated by fragments on which this depends then // use them to validate the stub flag file generated by the rules created by this method. if len(stubFlagSubsets) > 0 { - validFile := buildRuleValidateOverlappingCsvFiles(ctx, name, desc, outputPath, stubFlagSubsets) + validFile := buildRuleValidateOverlappingCsvFiles(ctx, name, desc, outputPath, stubFlagSubsets, + HIDDENAPI_STUB_FLAGS_IMPL_FLAGS) // Add the file that indicates that the file generated by this is valid. // @@ -904,7 +911,8 @@ func buildRuleToGenerateHiddenApiFlags(ctx android.BuilderContext, name, desc st // If there are flag files that have been generated by fragments on which this depends then use // them to validate the flag file generated by the rules created by this method. if len(flagSubsets) > 0 { - validFile := buildRuleValidateOverlappingCsvFiles(ctx, name, desc, outputPath, flagSubsets) + validFile := buildRuleValidateOverlappingCsvFiles(ctx, name, desc, outputPath, flagSubsets, + HIDDENAPI_FLAGS_CSV_IMPL_FLAGS) // Add the file that indicates that the file generated by this is valid. // @@ -968,13 +976,29 @@ func buildRuleSignaturePatternsFile( return patternsFile } -// buildRuleRemoveBlockedFlag creates a rule that will remove entries from the input path which -// only have blocked flags. It will not remove entries that have blocked as well as other flags, -// e.g. blocked,core-platform-api. -func buildRuleRemoveBlockedFlag(ctx android.BuilderContext, name string, desc string, inputPath android.Path, filteredPath android.WritablePath) { +// buildRuleRemoveSignaturesWithImplementationFlags creates a rule that will remove signatures from +// the input flags file which have only the implementation flags, i.e. are not part of an API. +// +// The implementationFlags specifies the set of default flags that identifies the signature of a +// private, implementation only, member. Signatures that match those flags are removed from the +// flags as they are implementation only. +// +// This is used to remove implementation only signatures from the signature files that are persisted +// in the sdk snapshot as the sdk snapshots should not include implementation details. The +// signatures generated by this method will be compared by the buildRuleValidateOverlappingCsvFiles +// method which treats any missing signatures as if they were implementation only signatures. +func buildRuleRemoveSignaturesWithImplementationFlags(ctx android.BuilderContext, + name string, desc string, inputPath android.Path, filteredPath android.WritablePath, + implementationFlags []string) { + rule := android.NewRuleBuilder(pctx, ctx) + implementationFlagPattern := "" + for _, implementationFlag := range implementationFlags { + implementationFlagPattern = implementationFlagPattern + "," + implementationFlag + } rule.Command(). - Text(`grep -vE "^[^,]+,blocked$"`).Input(inputPath).Text(">").Output(filteredPath). + Text(`grep -vE "^[^,]+` + implementationFlagPattern + `$"`).Input(inputPath). + Text(">").Output(filteredPath). // Grep's exit code depends on whether it finds anything. It is 0 (build success) when it finds // something and 1 (build failure) when it does not and 2 (when it encounters an error). // However, while it is unlikely it is not an error if this does not find any matches. The @@ -986,7 +1010,14 @@ func buildRuleRemoveBlockedFlag(ctx android.BuilderContext, name string, desc st // buildRuleValidateOverlappingCsvFiles checks that the modular CSV files, i.e. the files generated // by the individual bootclasspath_fragment modules are subsets of the monolithic CSV file. -func buildRuleValidateOverlappingCsvFiles(ctx android.BuilderContext, name string, desc string, monolithicFilePath android.WritablePath, csvSubsets SignatureCsvSubsets) android.WritablePath { +// +// The implementationFlags specifies the set of default flags that identifies the signature of a +// private, implementation only, member. A signature which is present in a monolithic flags subset +// defined by SignatureCsvSubset but which is not present in the flags file from the corresponding +// module is assumed to be an implementation only member and so must have these flags. +func buildRuleValidateOverlappingCsvFiles(ctx android.BuilderContext, name string, desc string, + monolithicFilePath android.WritablePath, csvSubsets SignatureCsvSubsets, + implementationFlags []string) android.WritablePath { // The file which is used to record that the flags file is valid. validFile := pathForValidation(ctx, monolithicFilePath) @@ -1003,6 +1034,10 @@ func buildRuleValidateOverlappingCsvFiles(ctx android.BuilderContext, name strin Implicit(subset.CsvFile).Implicit(subset.SignaturePatternsFile) } + for _, implementationFlag := range implementationFlags { + command.FlagWithArg("--implementation-flag ", implementationFlag) + } + // If validation passes then update the file that records that. command.Text("&& touch").Output(validFile) rule.Build(name+"Validation", desc+" validation") @@ -1076,12 +1111,16 @@ func hiddenAPIRulesForBootclasspathFragment(ctx android.ModuleContext, contents // Generate the filtered-stub-flags.csv file which contains the filtered stub flags that will be // compared against the monolithic stub flags. filteredStubFlagsCSV := android.PathForModuleOut(ctx, hiddenApiSubDir, "filtered-stub-flags.csv") - buildRuleRemoveBlockedFlag(ctx, "modularHiddenApiFilteredStubFlags", "modular hiddenapi filtered stub flags", stubFlagsCSV, filteredStubFlagsCSV) + buildRuleRemoveSignaturesWithImplementationFlags(ctx, "modularHiddenApiFilteredStubFlags", + "modular hiddenapi filtered stub flags", stubFlagsCSV, filteredStubFlagsCSV, + HIDDENAPI_STUB_FLAGS_IMPL_FLAGS) // Generate the filtered-flags.csv file which contains the filtered flags that will be compared // against the monolithic flags. filteredFlagsCSV := android.PathForModuleOut(ctx, hiddenApiSubDir, "filtered-flags.csv") - buildRuleRemoveBlockedFlag(ctx, "modularHiddenApiFilteredFlags", "modular hiddenapi filtered flags", allFlagsCSV, filteredFlagsCSV) + buildRuleRemoveSignaturesWithImplementationFlags(ctx, "modularHiddenApiFilteredFlags", + "modular hiddenapi filtered flags", allFlagsCSV, filteredFlagsCSV, + HIDDENAPI_FLAGS_CSV_IMPL_FLAGS) // Store the paths in the info for use by other modules and sdk snapshot generation. output := HiddenAPIOutput{ diff --git a/scripts/hiddenapi/verify_overlaps.py b/scripts/hiddenapi/verify_overlaps.py index 940532bc9..f985a4916 100755 --- a/scripts/hiddenapi/verify_overlaps.py +++ b/scripts/hiddenapi/verify_overlaps.py @@ -107,7 +107,8 @@ def read_signature_csv_from_file_as_dict(csv_file): return read_signature_csv_from_stream_as_dict(f) -def compare_signature_flags(monolithic_flags_dict, modular_flags_dict): +def compare_signature_flags(monolithic_flags_dict, modular_flags_dict, + implementation_flags): """Compare the signature flags between the two dicts. :param monolithic_flags_dict: the dict containing the subset of the @@ -130,7 +131,7 @@ def compare_signature_flags(monolithic_flags_dict, modular_flags_dict): modular_row = modular_flags_dict.get(signature, {}) modular_flags = modular_row.get(None, []) else: - modular_flags = ["blocked"] + modular_flags = implementation_flags if monolithic_flags != modular_flags: mismatching_signatures.append( (signature, modular_flags, monolithic_flags)) @@ -140,7 +141,13 @@ def compare_signature_flags(monolithic_flags_dict, modular_flags_dict): def main(argv): args_parser = argparse.ArgumentParser( description="Verify that sets of hidden API flags are each a subset of " - "the monolithic flag file.") + "the monolithic flag file. For each module this uses the provided " + "signature patterns to select a subset of the monolithic flags and " + "then it compares that subset against the filtered flags provided by " + "the module. If the module's filtered flags does not contain flags for " + "a signature then it is assumed to have been filtered out because it " + "was not part of an API and so is assumed to have the implementation " + "flags.") args_parser.add_argument( "--monolithic-flags", help="The monolithic flag file") args_parser.add_argument( @@ -149,18 +156,30 @@ def main(argv): help="A colon separated pair of paths. The first is a path to a " "filtered set of flags, and the second is a path to a set of " "signature patterns that identify the set of classes belonging to " - "a single bootclasspath_fragment module, ") + "a single bootclasspath_fragment module. Specify once for each module " + "that needs to be checked.") + args_parser.add_argument( + "--implementation-flag", + action="append", + help="A flag in the set of flags that identifies a signature which is " + "not part of an API, i.e. is the signature of a private implementation " + "member. Specify as many times as necessary to define the " + "implementation flag set. If this is not specified then the " + "implementation flag set is empty.") args = args_parser.parse_args(argv[1:]) # Read in all the flags into the trie monolithic_flags_path = args.monolithic_flags monolithic_trie = read_flag_trie_from_file(monolithic_flags_path) + implementation_flags = args.implementation_flag or [] + # For each subset specified on the command line, create dicts for the flags # provided by the subset and the corresponding flags from the complete set # of flags and compare them. failed = False - for modular_pair in args.module_flags: + module_pairs = args.module_flags or [] + for modular_pair in module_pairs: parts = modular_pair.split(":") modular_flags_path = parts[0] modular_patterns_path = parts[1] @@ -170,7 +189,8 @@ def main(argv): extract_subset_from_monolithic_flags_as_dict_from_file( monolithic_trie, modular_patterns_path) mismatching_signatures = compare_signature_flags( - monolithic_flags_subset_dict, modular_flags_dict) + monolithic_flags_subset_dict, modular_flags_dict, + implementation_flags) if mismatching_signatures: failed = True print("ERROR: Hidden API flags are inconsistent:") diff --git a/scripts/hiddenapi/verify_overlaps_test.py b/scripts/hiddenapi/verify_overlaps_test.py index ead8a4e96..0a489ee11 100755 --- a/scripts/hiddenapi/verify_overlaps_test.py +++ b/scripts/hiddenapi/verify_overlaps_test.py @@ -221,7 +221,8 @@ Ljava/lang/Object;->hashCode()I,public-api,system-api,test-api modular = self.read_signature_csv_from_string_as_dict(""" Ljava/lang/Object;->hashCode()I,public-api,system-api,test-api """) - mismatches = vo.compare_signature_flags(monolithic, modular) + mismatches = vo.compare_signature_flags(monolithic, modular, + ["blocked"]) expected = [] self.assertEqual(expected, mismatches) @@ -232,7 +233,8 @@ Ljava/lang/Object;->toString()Ljava/lang/String;,public-api modular = self.read_signature_csv_from_string_as_dict(""" Ljava/lang/Object;->toString()Ljava/lang/String;,public-api,system-api,test-api """) - mismatches = vo.compare_signature_flags(monolithic, modular) + mismatches = vo.compare_signature_flags(monolithic, modular, + ["blocked"]) expected = [ ( "Ljava/lang/Object;->toString()Ljava/lang/String;", @@ -249,7 +251,8 @@ Ljava/lang/Object;->toString()Ljava/lang/String;,blocked modular = self.read_signature_csv_from_string_as_dict(""" Ljava/lang/Object;->toString()Ljava/lang/String;,public-api,system-api,test-api """) - mismatches = vo.compare_signature_flags(monolithic, modular) + mismatches = vo.compare_signature_flags(monolithic, modular, + ["blocked"]) expected = [ ( "Ljava/lang/Object;->toString()Ljava/lang/String;", @@ -266,7 +269,8 @@ Ljava/lang/Object;->toString()Ljava/lang/String;,public-api,system-api,test-api modular = self.read_signature_csv_from_string_as_dict(""" Ljava/lang/Object;->toString()Ljava/lang/String;,blocked """) - mismatches = vo.compare_signature_flags(monolithic, modular) + mismatches = vo.compare_signature_flags(monolithic, modular, + ["blocked"]) expected = [ ( "Ljava/lang/Object;->toString()Ljava/lang/String;", @@ -281,7 +285,8 @@ Ljava/lang/Object;->toString()Ljava/lang/String;,blocked modular = self.read_signature_csv_from_string_as_dict(""" Ljava/lang/Object;->toString()Ljava/lang/String;,public-api,system-api,test-api """) - mismatches = vo.compare_signature_flags(monolithic, modular) + mismatches = vo.compare_signature_flags(monolithic, modular, + ["blocked"]) expected = [ ( "Ljava/lang/Object;->toString()Ljava/lang/String;", @@ -296,7 +301,8 @@ Ljava/lang/Object;->toString()Ljava/lang/String;,public-api,system-api,test-api Ljava/lang/Object;->hashCode()I,public-api,system-api,test-api """) modular = {} - mismatches = vo.compare_signature_flags(monolithic, modular) + mismatches = vo.compare_signature_flags(monolithic, modular, + ["blocked"]) expected = [ ( "Ljava/lang/Object;->hashCode()I", @@ -311,7 +317,47 @@ Ljava/lang/Object;->hashCode()I,public-api,system-api,test-api Ljava/lang/Object;->hashCode()I,blocked """) modular = {} - mismatches = vo.compare_signature_flags(monolithic, modular) + mismatches = vo.compare_signature_flags(monolithic, modular, + ["blocked"]) + expected = [] + self.assertEqual(expected, mismatches) + + def test_match_treat_missing_from_modular_as_empty(self): + monolithic = self.read_signature_csv_from_string_as_dict("") + modular = self.read_signature_csv_from_string_as_dict(""" +Ljava/lang/Object;->toString()Ljava/lang/String;,public-api,system-api,test-api +""") + mismatches = vo.compare_signature_flags(monolithic, modular, []) + expected = [ + ( + "Ljava/lang/Object;->toString()Ljava/lang/String;", + ["public-api", "system-api", "test-api"], + [], + ), + ] + self.assertEqual(expected, mismatches) + + def test_mismatch_treat_missing_from_modular_as_empty(self): + monolithic = self.read_signature_csv_from_string_as_dict(""" +Ljava/lang/Object;->hashCode()I,public-api,system-api,test-api +""") + modular = {} + mismatches = vo.compare_signature_flags(monolithic, modular, []) + expected = [ + ( + "Ljava/lang/Object;->hashCode()I", + [], + ["public-api", "system-api", "test-api"], + ), + ] + self.assertEqual(expected, mismatches) + + def test_empty_missing_from_modular(self): + monolithic = self.read_signature_csv_from_string_as_dict(""" +Ljava/lang/Object;->hashCode()I +""") + modular = {} + mismatches = vo.compare_signature_flags(monolithic, modular, []) expected = [] self.assertEqual(expected, mismatches)