From 33eb615eb0886c50d82a048fcb5862706ab82ec7 Mon Sep 17 00:00:00 2001 From: Jooyung Han Date: Mon, 11 Mar 2024 15:46:48 +0900 Subject: [PATCH] ndkstubgen: use llndk= for new llndk stub We want LLNDK symbols to be explicitly marked with llndk tag to handle LLNDK freezing which happens before SDK freezing. If symbols need to be frozen as LLNDK, those symbols must be marked explicitly with correct vFRC version. In the following example, LIBFOO { # introduced=35 foo; bar; bar; # llndk=202404 baz; # llndk=202404 qux; # llndk=202505 }; NDK libfoo will have foo and bar while LLNDK libfoo stub will have bar and baz for 202404. Bug: 329012338 Test: test_ndkstubgen test_symbolfile Change-Id: I384f589b240fa047e8871964bf9550f426024dfc --- cc/cc_test.go | 2 +- cc/library.go | 16 +++--- cc/ndkstubgen/test_ndkstubgen.py | 92 ++++++++++++++++++++++++++++++++ cc/symbolfile/__init__.py | 70 ++++++++++++++++++++++-- cc/symbolfile/test_symbolfile.py | 59 +++++++++++++++++++- 5 files changed, 222 insertions(+), 17 deletions(-) diff --git a/cc/cc_test.go b/cc/cc_test.go index 6cc500b5f..5836b6c59 100644 --- a/cc/cc_test.go +++ b/cc/cc_test.go @@ -2688,7 +2688,7 @@ func TestLlndkLibrary(t *testing.T) { android.AssertArrayString(t, "variants for llndk stubs", expected, actual) params := result.ModuleForTests("libllndk", "android_vendor.29_arm_armv7-a-neon_shared").Description("generate stub") - android.AssertSame(t, "use VNDK version for default stubs", "current", params.Args["apiLevel"]) + android.AssertSame(t, "use Vendor API level for default stubs", "202404", params.Args["apiLevel"]) checkExportedIncludeDirs := func(module, variant string, expectedDirs ...string) { t.Helper() diff --git a/cc/library.go b/cc/library.go index bbff49a68..089bbf97c 100644 --- a/cc/library.go +++ b/cc/library.go @@ -677,18 +677,16 @@ func (library *libraryDecorator) getHeaderAbiCheckerProperties(ctx android.BaseM func (library *libraryDecorator) compile(ctx ModuleContext, flags Flags, deps PathDeps) Objects { if ctx.IsLlndk() { + vendorApiLevel := ctx.Config().VendorApiLevel() + if vendorApiLevel == "" { + // TODO(b/321892570): Some tests relying on old fixtures which + // doesn't set vendorApiLevel. Needs to fix them. + vendorApiLevel = ctx.Config().PlatformSdkVersion().String() + } // This is the vendor variant of an LLNDK library, build the LLNDK stubs. - vndkVer := ctx.Module().(*Module).VndkVersion() - if !inList(vndkVer, ctx.Config().PlatformVersionActiveCodenames()) || vndkVer == "" { - // For non-enforcing devices, vndkVer is empty. Use "current" in that case, too. - vndkVer = "current" - } - if library.stubsVersion() != "" { - vndkVer = library.stubsVersion() - } nativeAbiResult := parseNativeAbiDefinition(ctx, String(library.Properties.Llndk.Symbol_file), - android.ApiLevelOrPanic(ctx, vndkVer), "--llndk") + android.ApiLevelOrPanic(ctx, vendorApiLevel), "--llndk") objs := compileStubLibrary(ctx, flags, nativeAbiResult.stubSrc) if !Bool(library.Properties.Llndk.Unversioned) { library.versionScriptPath = android.OptionalPathForPath( diff --git a/cc/ndkstubgen/test_ndkstubgen.py b/cc/ndkstubgen/test_ndkstubgen.py index 1e0bdf3ff..22f31d9f1 100755 --- a/cc/ndkstubgen/test_ndkstubgen.py +++ b/cc/ndkstubgen/test_ndkstubgen.py @@ -463,6 +463,98 @@ class IntegrationTest(unittest.TestCase): """) self.assertEqual(expected_version, version_file.getvalue()) + def test_integration_with_llndk(self) -> None: + input_file = io.StringIO(textwrap.dedent("""\ + VERSION_34 { # introduced=34 + global: + foo; + bar; # llndk + }; + VERSION_35 { # introduced=35 + global: + wiggle; + waggle; + waggle; # llndk=202404 + bubble; # llndk=202404 + duddle; + duddle; # llndk=202504 + } VERSION_34; + """)) + f = copy(self.filter) + f.llndk = True + f.api = 202404 + parser = symbolfile.SymbolFileParser(input_file, {}, f) + versions = parser.parse() + + src_file = io.StringIO() + version_file = io.StringIO() + symbol_list_file = io.StringIO() + + generator = ndkstubgen.Generator(src_file, + version_file, symbol_list_file, f) + generator.write(versions) + + expected_src = textwrap.dedent("""\ + void foo() {} + void bar() {} + void waggle() {} + void bubble() {} + """) + self.assertEqual(expected_src, src_file.getvalue()) + + expected_version = textwrap.dedent("""\ + VERSION_34 { + global: + foo; + bar; + }; + VERSION_35 { + global: + waggle; + bubble; + } VERSION_34; + """) + self.assertEqual(expected_version, version_file.getvalue()) + + def test_integration_with_llndk_with_single_version_block(self) -> None: + input_file = io.StringIO(textwrap.dedent("""\ + LIBANDROID { + global: + foo; # introduced=34 + bar; # introduced=35 + bar; # llndk=202404 + baz; # introduced=35 + }; + """)) + f = copy(self.filter) + f.llndk = True + f.api = 202404 + parser = symbolfile.SymbolFileParser(input_file, {}, f) + versions = parser.parse() + + src_file = io.StringIO() + version_file = io.StringIO() + symbol_list_file = io.StringIO() + + generator = ndkstubgen.Generator(src_file, + version_file, symbol_list_file, f) + generator.write(versions) + + expected_src = textwrap.dedent("""\ + void foo() {} + void bar() {} + """) + self.assertEqual(expected_src, src_file.getvalue()) + + expected_version = textwrap.dedent("""\ + LIBANDROID { + global: + foo; + bar; + }; + """) + self.assertEqual(expected_version, version_file.getvalue()) + def test_empty_stub(self) -> None: """Tests that empty stubs can be generated. diff --git a/cc/symbolfile/__init__.py b/cc/symbolfile/__init__.py index 345e9f983..4553616ac 100644 --- a/cc/symbolfile/__init__.py +++ b/cc/symbolfile/__init__.py @@ -103,13 +103,24 @@ class Tags: @property def has_llndk_tags(self) -> bool: """Returns True if any LL-NDK tags are set.""" - return 'llndk' in self.tags + for tag in self.tags: + if tag == 'llndk' or tag.startswith('llndk='): + return True + return False @property def has_platform_only_tags(self) -> bool: """Returns True if any platform-only tags are set.""" return 'platform-only' in self.tags + def copy_introduced_from(self, tags: Tags) -> None: + """Copies introduced= or introduced-*= tags.""" + for tag in tags: + if tag.startswith('introduced=') or tag.startswith('introduced-'): + name, _ = split_tag(tag) + if not any(self_tag.startswith(name + '=') for self_tag in self.tags): + self.tags += (tag,) + @dataclass class Symbol: @@ -147,6 +158,8 @@ def is_api_level_tag(tag: Tag) -> bool: """Returns true if this tag has an API level that may need decoding.""" if tag.startswith('llndk-deprecated='): return True + if tag.startswith('llndk='): + return True if tag.startswith('introduced='): return True if tag.startswith('introduced-'): @@ -237,15 +250,22 @@ class Filter: This defines the rules shared between version tagging and symbol tagging. """ - # The apex and llndk tags will only exclude APIs from other modes. If in + # LLNDK mode/tags follow the similar filtering except that API level checking + # is based llndk= instead of introduced=. + if self.llndk: + if tags.has_mode_tags and not tags.has_llndk_tags: + return True + if not symbol_in_arch(tags, self.arch): + return True + if not symbol_in_llndk_api(tags, self.arch, self.api): + return True + return False # APEX or LLNDK mode and neither tag is provided, we fall back to the # default behavior because all NDK symbols are implicitly available to # APEX and LLNDK. if tags.has_mode_tags: if self.apex and tags.has_apex_tags: return False - if self.llndk and tags.has_llndk_tags: - return False if self.systemapi and tags.has_systemapi_tags: return False return True @@ -266,6 +286,10 @@ class Filter: return True if version.tags.has_platform_only_tags: return True + # Include all versions when targeting LLNDK because LLNDK symbols are self-versioned. + # Empty version block will be handled separately. + if self.llndk: + return False return self._should_omit_tags(version.tags) def should_omit_symbol(self, symbol: Symbol) -> bool: @@ -292,6 +316,14 @@ def symbol_in_arch(tags: Tags, arch: Arch) -> bool: # for the tagged architectures. return not has_arch_tags +def symbol_in_llndk_api(tags: Iterable[Tag], arch: Arch, api: int) -> bool: + """Returns true if the symbol is present for the given LLNDK API level.""" + # Check llndk= first. + for tag in tags: + if tag.startswith('llndk='): + return api >= int(get_tag_value(tag)) + # If not, we keep old behavior: NDK symbols in <= 34 are LLNDK symbols. + return symbol_in_api(tags, arch, 34) def symbol_in_api(tags: Iterable[Tag], arch: Arch, api: int) -> bool: """Returns true if the symbol is present for the given API level.""" @@ -368,6 +400,7 @@ class SymbolFileParser: f'Unexpected contents at top level: {self.current_line}') self.check_no_duplicate_symbols(versions) + self.check_llndk_introduced(versions) return versions def check_no_duplicate_symbols(self, versions: Iterable[Version]) -> None: @@ -396,6 +429,31 @@ class SymbolFileParser: raise MultiplyDefinedSymbolError( sorted(list(multiply_defined_symbols))) + def check_llndk_introduced(self, versions: Iterable[Version]) -> None: + """Raises errors when llndk= is missing for new llndk symbols.""" + if not self.filter.llndk: + return + + def assert_llndk_with_version(tags: Tags, name: str) -> None: + has_llndk_introduced = False + for tag in tags: + if tag.startswith('llndk='): + has_llndk_introduced = True + break + if not has_llndk_introduced: + raise ParseError(f'{name}: missing version. `llndk=yyyymm`') + + arch = self.filter.arch + for version in versions: + # llndk symbols >= introduced=35 should be tagged + # explicitly with llndk=yyyymm. + for symbol in version.symbols: + if not symbol.tags.has_llndk_tags: + continue + if symbol_in_api(symbol.tags, arch, 34): + continue + assert_llndk_with_version(symbol.tags, symbol.name) + def parse_version(self) -> Version: """Parses a single version section and returns a Version object.""" assert self.current_line is not None @@ -429,7 +487,9 @@ class SymbolFileParser: else: raise ParseError('Unknown visiblity label: ' + visibility) elif global_scope and not cpp_symbols: - symbols.append(self.parse_symbol()) + symbol = self.parse_symbol() + symbol.tags.copy_introduced_from(tags) + symbols.append(symbol) else: # We're in a hidden scope or in 'extern "C++"' block. Ignore # everything. diff --git a/cc/symbolfile/test_symbolfile.py b/cc/symbolfile/test_symbolfile.py index 83becc2c7..8b412b98a 100644 --- a/cc/symbolfile/test_symbolfile.py +++ b/cc/symbolfile/test_symbolfile.py @@ -344,6 +344,45 @@ class OmitSymbolTest(unittest.TestCase): self.assertInclude(f_llndk, s_none) self.assertInclude(f_llndk, s_llndk) + def test_omit_llndk_versioned(self) -> None: + f_ndk = self.filter + f_ndk.api = 35 + + f_llndk = copy(f_ndk) + f_llndk.llndk = True + f_llndk.api = 202404 + + s = Symbol('foo', Tags()) + s_llndk = Symbol('foo', Tags.from_strs(['llndk'])) + s_llndk_202404 = Symbol('foo', Tags.from_strs(['llndk=202404'])) + s_34 = Symbol('foo', Tags.from_strs(['introduced=34'])) + s_34_llndk = Symbol('foo', Tags.from_strs(['introduced=34', 'llndk'])) + s_35 = Symbol('foo', Tags.from_strs(['introduced=35'])) + s_35_llndk_202404 = Symbol('foo', Tags.from_strs(['introduced=35', 'llndk=202404'])) + s_35_llndk_202504 = Symbol('foo', Tags.from_strs(['introduced=35', 'llndk=202504'])) + + # When targeting NDK, omit LLNDK tags + self.assertInclude(f_ndk, s) + self.assertOmit(f_ndk, s_llndk) + self.assertOmit(f_ndk, s_llndk_202404) + self.assertInclude(f_ndk, s_34) + self.assertOmit(f_ndk, s_34_llndk) + self.assertInclude(f_ndk, s_35) + self.assertOmit(f_ndk, s_35_llndk_202404) + self.assertOmit(f_ndk, s_35_llndk_202504) + + # When targeting LLNDK, old symbols without any mode tags are included as LLNDK + self.assertInclude(f_llndk, s) + # When targeting LLNDK, old symbols with #llndk are included as LLNDK + self.assertInclude(f_llndk, s_llndk) + self.assertInclude(f_llndk, s_llndk_202404) + self.assertInclude(f_llndk, s_34) + self.assertInclude(f_llndk, s_34_llndk) + # When targeting LLNDK, new symbols(>=35) should be tagged with llndk-introduced=. + self.assertOmit(f_llndk, s_35) + self.assertInclude(f_llndk, s_35_llndk_202404) + self.assertOmit(f_llndk, s_35_llndk_202504) + def test_omit_apex(self) -> None: f_none = self.filter f_apex = copy(f_none) @@ -451,9 +490,12 @@ class SymbolFileParseTest(unittest.TestCase): self.assertIsNone(version.base) self.assertEqual(Tags.from_strs(['weak', 'introduced=35']), version.tags) + # Inherit introduced= tags from version block so that + # should_omit_tags() can differently based on introduced API level when treating + # LLNDK-available symbols. expected_symbols = [ - Symbol('baz', Tags()), - Symbol('qux', Tags.from_strs(['apex', 'llndk'])), + Symbol('baz', Tags.from_strs(['introduced=35'])), + Symbol('qux', Tags.from_strs(['apex', 'llndk', 'introduced=35'])), ] self.assertEqual(expected_symbols, version.symbols) @@ -601,6 +643,19 @@ class SymbolFileParseTest(unittest.TestCase): ] self.assertEqual(expected_symbols, version.symbols) + def test_parse_llndk_version_is_missing(self) -> None: + input_file = io.StringIO(textwrap.dedent("""\ + VERSION_1 { # introduced=35 + foo; + bar; # llndk + }; + """)) + f = copy(self.filter) + f.llndk = True + parser = symbolfile.SymbolFileParser(input_file, {}, f) + with self.assertRaises(symbolfile.ParseError): + parser.parse() + def main() -> None: suite = unittest.TestLoader().loadTestsFromName(__name__)