From f50b6ce9baf49ea30f4ae80a9f6c9bc9cdd35361 Mon Sep 17 00:00:00 2001 From: Dan Albert Date: Tue, 25 Sep 2018 13:39:25 -0700 Subject: [PATCH 1/3] Pylint fix. Test: pylint cc/gen_stub_libs.py Bug: None Change-Id: Ifa397b2b69a835d7f61731e4f240fa5971858f4c --- cc/gen_stub_libs.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cc/gen_stub_libs.py b/cc/gen_stub_libs.py index 8c99bbf3e..32765a7c1 100755 --- a/cc/gen_stub_libs.py +++ b/cc/gen_stub_libs.py @@ -274,7 +274,8 @@ class SymbolFileParser(object): elif global_scope and not cpp_symbols: symbols.append(self.parse_symbol()) else: - # We're in a hidden scope or in 'extern "C++"' block. Ignore everything. + # We're in a hidden scope or in 'extern "C++"' block. Ignore + # everything. pass raise ParseError('Unexpected EOF in version block.') From f5b8184abe77f736231afebed4a6d1272cecaef7 Mon Sep 17 00:00:00 2001 From: Dan Albert Date: Tue, 9 Oct 2018 15:22:15 -0700 Subject: [PATCH 2/3] Python 3 fix. Test: nose2 # With python3 nose2 Bug: None Change-Id: I84812c55be0daa58a594752702d866daf3c2d8f0 --- cc/test_gen_stub_libs.py | 46 ++++++++++++++++++++-------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/cc/test_gen_stub_libs.py b/cc/test_gen_stub_libs.py index b20a5c73a..279ef5285 100755 --- a/cc/test_gen_stub_libs.py +++ b/cc/test_gen_stub_libs.py @@ -15,7 +15,7 @@ # limitations under the License. # """Tests for gen_stub_libs.py.""" -import cStringIO +import io import textwrap import unittest @@ -200,7 +200,7 @@ class OmitVersionTest(unittest.TestCase): class SymbolFileParseTest(unittest.TestCase): def test_next_line(self): - input_file = cStringIO.StringIO(textwrap.dedent("""\ + input_file = io.StringIO(textwrap.dedent("""\ foo bar @@ -223,7 +223,7 @@ class SymbolFileParseTest(unittest.TestCase): self.assertEqual('', parser.current_line) def test_parse_version(self): - input_file = cStringIO.StringIO(textwrap.dedent("""\ + input_file = io.StringIO(textwrap.dedent("""\ VERSION_1 { # foo bar baz; qux; # woodly doodly @@ -253,7 +253,7 @@ class SymbolFileParseTest(unittest.TestCase): self.assertEqual([], version.tags) def test_parse_version_eof(self): - input_file = cStringIO.StringIO(textwrap.dedent("""\ + input_file = io.StringIO(textwrap.dedent("""\ VERSION_1 { """)) parser = gsl.SymbolFileParser(input_file, {}) @@ -262,7 +262,7 @@ class SymbolFileParseTest(unittest.TestCase): parser.parse_version() def test_unknown_scope_label(self): - input_file = cStringIO.StringIO(textwrap.dedent("""\ + input_file = io.StringIO(textwrap.dedent("""\ VERSION_1 { foo: } @@ -273,7 +273,7 @@ class SymbolFileParseTest(unittest.TestCase): parser.parse_version() def test_parse_symbol(self): - input_file = cStringIO.StringIO(textwrap.dedent("""\ + input_file = io.StringIO(textwrap.dedent("""\ foo; bar; # baz qux """)) @@ -290,7 +290,7 @@ class SymbolFileParseTest(unittest.TestCase): self.assertEqual(['baz', 'qux'], symbol.tags) def test_wildcard_symbol_global(self): - input_file = cStringIO.StringIO(textwrap.dedent("""\ + input_file = io.StringIO(textwrap.dedent("""\ VERSION_1 { *; }; @@ -301,7 +301,7 @@ class SymbolFileParseTest(unittest.TestCase): parser.parse_version() def test_wildcard_symbol_local(self): - input_file = cStringIO.StringIO(textwrap.dedent("""\ + input_file = io.StringIO(textwrap.dedent("""\ VERSION_1 { local: *; @@ -313,7 +313,7 @@ class SymbolFileParseTest(unittest.TestCase): self.assertEqual([], version.symbols) def test_missing_semicolon(self): - input_file = cStringIO.StringIO(textwrap.dedent("""\ + input_file = io.StringIO(textwrap.dedent("""\ VERSION_1 { foo }; @@ -325,12 +325,12 @@ class SymbolFileParseTest(unittest.TestCase): def test_parse_fails_invalid_input(self): with self.assertRaises(gsl.ParseError): - input_file = cStringIO.StringIO('foo') + input_file = io.StringIO('foo') parser = gsl.SymbolFileParser(input_file, {}) parser.parse() def test_parse(self): - input_file = cStringIO.StringIO(textwrap.dedent("""\ + input_file = io.StringIO(textwrap.dedent("""\ VERSION_1 { local: hidden1; @@ -368,8 +368,8 @@ class GeneratorTest(unittest.TestCase): def test_omit_version(self): # Thorough testing of the cases involved here is handled by # OmitVersionTest, PrivateVersionTest, and SymbolPresenceTest. - src_file = cStringIO.StringIO() - version_file = cStringIO.StringIO() + src_file = io.StringIO() + version_file = io.StringIO() generator = gsl.Generator(src_file, version_file, 'arm', 9, False) version = gsl.Version('VERSION_PRIVATE', None, [], [ @@ -396,8 +396,8 @@ class GeneratorTest(unittest.TestCase): def test_omit_symbol(self): # Thorough testing of the cases involved here is handled by # SymbolPresenceTest. - src_file = cStringIO.StringIO() - version_file = cStringIO.StringIO() + src_file = io.StringIO() + version_file = io.StringIO() generator = gsl.Generator(src_file, version_file, 'arm', 9, False) version = gsl.Version('VERSION_1', None, [], [ @@ -422,8 +422,8 @@ class GeneratorTest(unittest.TestCase): self.assertEqual('', version_file.getvalue()) def test_write(self): - src_file = cStringIO.StringIO() - version_file = cStringIO.StringIO() + src_file = io.StringIO() + version_file = io.StringIO() generator = gsl.Generator(src_file, version_file, 'arm', 9, False) versions = [ @@ -475,7 +475,7 @@ class IntegrationTest(unittest.TestCase): 'P': 9001, } - input_file = cStringIO.StringIO(textwrap.dedent("""\ + input_file = io.StringIO(textwrap.dedent("""\ VERSION_1 { global: foo; # var @@ -508,8 +508,8 @@ class IntegrationTest(unittest.TestCase): parser = gsl.SymbolFileParser(input_file, api_map) versions = parser.parse() - src_file = cStringIO.StringIO() - version_file = cStringIO.StringIO() + src_file = io.StringIO() + version_file = io.StringIO() generator = gsl.Generator(src_file, version_file, 'arm', 9, False) generator.write(versions) @@ -545,7 +545,7 @@ class IntegrationTest(unittest.TestCase): 'Q': 9002, } - input_file = cStringIO.StringIO(textwrap.dedent("""\ + input_file = io.StringIO(textwrap.dedent("""\ VERSION_1 { global: foo; # introduced=O @@ -558,8 +558,8 @@ class IntegrationTest(unittest.TestCase): parser = gsl.SymbolFileParser(input_file, api_map) versions = parser.parse() - src_file = cStringIO.StringIO() - version_file = cStringIO.StringIO() + src_file = io.StringIO() + version_file = io.StringIO() generator = gsl.Generator(src_file, version_file, 'arm', 9001, False) generator.write(versions) From 756f2d0e1beaf6abb741b06b442f42a25e8065e3 Mon Sep 17 00:00:00 2001 From: Dan Albert Date: Tue, 9 Oct 2018 16:36:03 -0700 Subject: [PATCH 3/3] Better error message for multiple defined symbols. Test: nose2 Test: m ndk Bug: http://b/116629622 Change-Id: I30719eaf29d63d8c6595bbab4e5214a1ce6189ca --- cc/gen_stub_libs.py | 84 +++++++++++++++---- cc/test_gen_stub_libs.py | 171 +++++++++++++++++++++++++++++---------- 2 files changed, 196 insertions(+), 59 deletions(-) diff --git a/cc/gen_stub_libs.py b/cc/gen_stub_libs.py index 32765a7c1..c49d19777 100755 --- a/cc/gen_stub_libs.py +++ b/cc/gen_stub_libs.py @@ -20,6 +20,7 @@ import json import logging import os import re +import sys ALL_ARCHITECTURES = ( @@ -107,22 +108,33 @@ def version_is_private(version): return version.endswith('_PRIVATE') or version.endswith('_PLATFORM') -def should_omit_version(name, tags, arch, api, vndk): +def should_omit_version(version, arch, api, vndk): """Returns True if the version section should be ommitted. We want to omit any sections that do not have any symbols we'll have in the stub library. Sections that contain entirely future symbols or only symbols for certain architectures. """ - if version_is_private(name): + if version_is_private(version.name): return True - if 'platform-only' in tags: + if 'platform-only' in version.tags: return True - if 'vndk' in tags and not vndk: + if 'vndk' in version.tags and not vndk: return True - if not symbol_in_arch(tags, arch): + if not symbol_in_arch(version.tags, arch): return True - if not symbol_in_api(tags, arch, api): + if not symbol_in_api(version.tags, arch, api): + return True + return False + + +def should_omit_symbol(symbol, arch, api, vndk): + """Returns True if the symbol should be omitted.""" + if not vndk and 'vndk' in symbol.tags: + return True + if not symbol_in_arch(symbol.tags, arch): + return True + if not symbol_in_api(symbol.tags, arch, api): return True return False @@ -189,6 +201,15 @@ class ParseError(RuntimeError): pass +class MultiplyDefinedSymbolError(RuntimeError): + """A symbol name was multiply defined.""" + def __init__(self, multiply_defined_symbols): + super(MultiplyDefinedSymbolError, self).__init__( + 'Version script contains multiple definitions for: {}'.format( + ', '.join(multiply_defined_symbols))) + self.multiply_defined_symbols = multiply_defined_symbols + + class Version(object): """A version block of a symbol file.""" def __init__(self, name, base, tags, symbols): @@ -221,9 +242,12 @@ class Symbol(object): class SymbolFileParser(object): """Parses NDK symbol files.""" - def __init__(self, input_file, api_map): + def __init__(self, input_file, api_map, arch, api, vndk): self.input_file = input_file self.api_map = api_map + self.arch = arch + self.api = api + self.vndk = vndk self.current_line = None def parse(self): @@ -235,8 +259,36 @@ class SymbolFileParser(object): else: raise ParseError( 'Unexpected contents at top level: ' + self.current_line) + + self.check_no_duplicate_symbols(versions) return versions + def check_no_duplicate_symbols(self, versions): + """Raises errors for multiply defined symbols. + + This situation is the normal case when symbol versioning is actually + used, but this script doesn't currently handle that. The error message + will be a not necessarily obvious "error: redefition of 'foo'" from + stub.c, so it's better for us to catch this situation and raise a + better error. + """ + symbol_names = set() + multiply_defined_symbols = set() + for version in versions: + if should_omit_version(version, self.arch, self.api, self.vndk): + continue + + for symbol in version.symbols: + if should_omit_symbol(symbol, self.arch, self.api, self.vndk): + continue + + if symbol.name in symbol_names: + multiply_defined_symbols.add(symbol.name) + symbol_names.add(symbol.name) + if multiply_defined_symbols: + raise MultiplyDefinedSymbolError( + sorted(list(multiply_defined_symbols))) + def parse_version(self): """Parses a single version section and returns a Version object.""" name = self.current_line.split('{')[0].strip() @@ -325,20 +377,14 @@ class Generator(object): def write_version(self, version): """Writes a single version block's data to the output files.""" - name = version.name - tags = version.tags - if should_omit_version(name, tags, self.arch, self.api, self.vndk): + if should_omit_version(version, self.arch, self.api, self.vndk): return - section_versioned = symbol_versioned_in_api(tags, self.api) + section_versioned = symbol_versioned_in_api(version.tags, self.api) version_empty = True pruned_symbols = [] for symbol in version.symbols: - if not self.vndk and 'vndk' in symbol.tags: - continue - if not symbol_in_arch(symbol.tags, self.arch): - continue - if not symbol_in_api(symbol.tags, self.arch, self.api): + if should_omit_symbol(symbol, self.arch, self.api, self.vndk): continue if symbol_versioned_in_api(symbol.tags, self.api): @@ -433,7 +479,11 @@ def main(): logging.basicConfig(level=verbose_map[verbosity]) with open(args.symbol_file) as symbol_file: - versions = SymbolFileParser(symbol_file, api_map).parse() + try: + versions = SymbolFileParser(symbol_file, api_map, args.arch, api, + args.vndk).parse() + except MultiplyDefinedSymbolError as ex: + sys.exit('{}: error: {}'.format(args.symbol_file, ex)) with open(args.stub_src, 'w') as src_file: with open(args.version_script, 'w') as version_file: diff --git a/cc/test_gen_stub_libs.py b/cc/test_gen_stub_libs.py index 279ef5285..3b5585a02 100755 --- a/cc/test_gen_stub_libs.py +++ b/cc/test_gen_stub_libs.py @@ -163,39 +163,94 @@ class SymbolPresenceTest(unittest.TestCase): class OmitVersionTest(unittest.TestCase): def test_omit_private(self): - self.assertFalse(gsl.should_omit_version('foo', [], 'arm', 9, False)) - - self.assertTrue(gsl.should_omit_version( - 'foo_PRIVATE', [], 'arm', 9, False)) - self.assertTrue(gsl.should_omit_version( - 'foo_PLATFORM', [], 'arm', 9, False)) - - self.assertTrue(gsl.should_omit_version( - 'foo', ['platform-only'], 'arm', 9, False)) - - def test_omit_vndk(self): - self.assertTrue(gsl.should_omit_version( - 'foo', ['vndk'], 'arm', 9, False)) - - self.assertFalse(gsl.should_omit_version('foo', [], 'arm', 9, True)) - self.assertFalse(gsl.should_omit_version( - 'foo', ['vndk'], 'arm', 9, True)) - - def test_omit_arch(self): - self.assertFalse(gsl.should_omit_version('foo', [], 'arm', 9, False)) - self.assertFalse(gsl.should_omit_version( - 'foo', ['arm'], 'arm', 9, False)) - - self.assertTrue(gsl.should_omit_version( - 'foo', ['x86'], 'arm', 9, False)) - - def test_omit_api(self): - self.assertFalse(gsl.should_omit_version('foo', [], 'arm', 9, False)) self.assertFalse( - gsl.should_omit_version('foo', ['introduced=9'], 'arm', 9, False)) + gsl.should_omit_version( + gsl.Version('foo', None, [], []), 'arm', 9, False)) self.assertTrue( - gsl.should_omit_version('foo', ['introduced=14'], 'arm', 9, False)) + gsl.should_omit_version( + gsl.Version('foo_PRIVATE', None, [], []), 'arm', 9, False)) + self.assertTrue( + gsl.should_omit_version( + gsl.Version('foo_PLATFORM', None, [], []), 'arm', 9, False)) + + self.assertTrue( + gsl.should_omit_version( + gsl.Version('foo', None, ['platform-only'], []), 'arm', 9, + False)) + + def test_omit_vndk(self): + self.assertTrue( + gsl.should_omit_version( + gsl.Version('foo', None, ['vndk'], []), 'arm', 9, False)) + + self.assertFalse( + gsl.should_omit_version( + gsl.Version('foo', None, [], []), 'arm', 9, True)) + self.assertFalse( + gsl.should_omit_version( + gsl.Version('foo', None, ['vndk'], []), 'arm', 9, True)) + + def test_omit_arch(self): + self.assertFalse( + gsl.should_omit_version( + gsl.Version('foo', None, [], []), 'arm', 9, False)) + self.assertFalse( + gsl.should_omit_version( + gsl.Version('foo', None, ['arm'], []), 'arm', 9, False)) + + self.assertTrue( + gsl.should_omit_version( + gsl.Version('foo', None, ['x86'], []), 'arm', 9, False)) + + def test_omit_api(self): + self.assertFalse( + gsl.should_omit_version( + gsl.Version('foo', None, [], []), 'arm', 9, False)) + self.assertFalse( + gsl.should_omit_version( + gsl.Version('foo', None, ['introduced=9'], []), 'arm', 9, + False)) + + self.assertTrue( + gsl.should_omit_version( + gsl.Version('foo', None, ['introduced=14'], []), 'arm', 9, + False)) + + +class OmitSymbolTest(unittest.TestCase): + def test_omit_vndk(self): + self.assertTrue( + gsl.should_omit_symbol( + gsl.Symbol('foo', ['vndk']), 'arm', 9, False)) + + self.assertFalse( + gsl.should_omit_symbol(gsl.Symbol('foo', []), 'arm', 9, True)) + self.assertFalse( + gsl.should_omit_symbol( + gsl.Symbol('foo', ['vndk']), 'arm', 9, True)) + + def test_omit_arch(self): + self.assertFalse( + gsl.should_omit_symbol(gsl.Symbol('foo', []), 'arm', 9, False)) + self.assertFalse( + gsl.should_omit_symbol( + gsl.Symbol('foo', ['arm']), 'arm', 9, False)) + + self.assertTrue( + gsl.should_omit_symbol( + gsl.Symbol('foo', ['x86']), 'arm', 9, False)) + + def test_omit_api(self): + self.assertFalse( + gsl.should_omit_symbol(gsl.Symbol('foo', []), 'arm', 9, False)) + self.assertFalse( + gsl.should_omit_symbol( + gsl.Symbol('foo', ['introduced=9']), 'arm', 9, False)) + + self.assertTrue( + gsl.should_omit_symbol( + gsl.Symbol('foo', ['introduced=14']), 'arm', 9, False)) class SymbolFileParseTest(unittest.TestCase): @@ -207,7 +262,7 @@ class SymbolFileParseTest(unittest.TestCase): # baz qux """)) - parser = gsl.SymbolFileParser(input_file, {}) + parser = gsl.SymbolFileParser(input_file, {}, 'arm', 16, False) self.assertIsNone(parser.current_line) self.assertEqual('foo', parser.next_line().strip()) @@ -232,7 +287,7 @@ class SymbolFileParseTest(unittest.TestCase): VERSION_2 { } VERSION_1; # asdf """)) - parser = gsl.SymbolFileParser(input_file, {}) + parser = gsl.SymbolFileParser(input_file, {}, 'arm', 16, False) parser.next_line() version = parser.parse_version() @@ -256,7 +311,7 @@ class SymbolFileParseTest(unittest.TestCase): input_file = io.StringIO(textwrap.dedent("""\ VERSION_1 { """)) - parser = gsl.SymbolFileParser(input_file, {}) + parser = gsl.SymbolFileParser(input_file, {}, 'arm', 16, False) parser.next_line() with self.assertRaises(gsl.ParseError): parser.parse_version() @@ -267,7 +322,7 @@ class SymbolFileParseTest(unittest.TestCase): foo: } """)) - parser = gsl.SymbolFileParser(input_file, {}) + parser = gsl.SymbolFileParser(input_file, {}, 'arm', 16, False) parser.next_line() with self.assertRaises(gsl.ParseError): parser.parse_version() @@ -277,7 +332,7 @@ class SymbolFileParseTest(unittest.TestCase): foo; bar; # baz qux """)) - parser = gsl.SymbolFileParser(input_file, {}) + parser = gsl.SymbolFileParser(input_file, {}, 'arm', 16, False) parser.next_line() symbol = parser.parse_symbol() @@ -295,7 +350,7 @@ class SymbolFileParseTest(unittest.TestCase): *; }; """)) - parser = gsl.SymbolFileParser(input_file, {}) + parser = gsl.SymbolFileParser(input_file, {}, 'arm', 16, False) parser.next_line() with self.assertRaises(gsl.ParseError): parser.parse_version() @@ -307,7 +362,7 @@ class SymbolFileParseTest(unittest.TestCase): *; }; """)) - parser = gsl.SymbolFileParser(input_file, {}) + parser = gsl.SymbolFileParser(input_file, {}, 'arm', 16, False) parser.next_line() version = parser.parse_version() self.assertEqual([], version.symbols) @@ -318,7 +373,7 @@ class SymbolFileParseTest(unittest.TestCase): foo }; """)) - parser = gsl.SymbolFileParser(input_file, {}) + parser = gsl.SymbolFileParser(input_file, {}, 'arm', 16, False) parser.next_line() with self.assertRaises(gsl.ParseError): parser.parse_version() @@ -326,7 +381,7 @@ class SymbolFileParseTest(unittest.TestCase): def test_parse_fails_invalid_input(self): with self.assertRaises(gsl.ParseError): input_file = io.StringIO('foo') - parser = gsl.SymbolFileParser(input_file, {}) + parser = gsl.SymbolFileParser(input_file, {}, 'arm', 16, False) parser.parse() def test_parse(self): @@ -347,7 +402,7 @@ class SymbolFileParseTest(unittest.TestCase): qwerty; } VERSION_1; """)) - parser = gsl.SymbolFileParser(input_file, {}) + parser = gsl.SymbolFileParser(input_file, {}, 'arm', 16, False) versions = parser.parse() expected = [ @@ -505,7 +560,7 @@ class IntegrationTest(unittest.TestCase): wobble; } VERSION_4; """)) - parser = gsl.SymbolFileParser(input_file, api_map) + parser = gsl.SymbolFileParser(input_file, api_map, 'arm', 9, False) versions = parser.parse() src_file = io.StringIO() @@ -555,7 +610,7 @@ class IntegrationTest(unittest.TestCase): *; }; """)) - parser = gsl.SymbolFileParser(input_file, api_map) + parser = gsl.SymbolFileParser(input_file, api_map, 'arm', 9001, False) versions = parser.parse() src_file = io.StringIO() @@ -578,6 +633,38 @@ class IntegrationTest(unittest.TestCase): """) self.assertEqual(expected_version, version_file.getvalue()) + def test_multiple_definition(self): + input_file = io.StringIO(textwrap.dedent("""\ + VERSION_1 { + global: + foo; + foo; + bar; + baz; + qux; # arm + local: + *; + }; + + VERSION_2 { + global: + bar; + qux; # arm64 + } VERSION_1; + + VERSION_PRIVATE { + global: + baz; + } VERSION_2; + + """)) + parser = gsl.SymbolFileParser(input_file, {}, 'arm', 16, False) + + with self.assertRaises(gsl.MultiplyDefinedSymbolError) as cm: + parser.parse() + self.assertEquals(['bar', 'foo'], + cm.exception.multiply_defined_symbols) + def main(): suite = unittest.TestLoader().loadTestsFromName(__name__)