Skip to content

bpo-38187: Fix a refleak in Tools/c-analyzer. #16304

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import os.path
from test.support import load_package_tests


def load_tests(*args):
return load_package_tests(os.path.dirname(__file__), *args)
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@ class FromFileTests(unittest.TestCase):

_return_read_tsv = ()

def tearDown(self):
Variable._isglobal.instances.clear()

@property
def calls(self):
try:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import os.path
from test.support import load_package_tests


def load_tests(*args):
return load_package_tests(os.path.dirname(__file__), *args)
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,9 @@ def test_typical(self):
**self.kwargs))

self.assertEqual(found, [
info.Variable.from_parts('dir1/spam.c', None, 'var1', 'int'),
info.Variable.from_parts('dir1/spam.c', None, 'var2', 'static int'),
info.Variable.from_parts('dir1/spam.c', None, 'var3', 'char *'),
info.Variable.from_parts('dir1/eggs.c', None, 'var1', 'static int'),
info.Variable.from_parts('dir1/eggs.c', 'func1', 'var2', 'static char *'),
])
Expand Down Expand Up @@ -299,7 +301,7 @@ def test_typical(self):
info.Variable.from_parts('src1/spam.c', None, 'var1', 'static const char *'),
info.Variable.from_parts('src1/spam.c', None, 'var1b', 'const char *'),
info.Variable.from_parts('src1/spam.c', 'ham', 'initialized', 'static int'),
info.Variable.from_parts('src1/spam.c', 'ham', 'result', 'int'),
info.Variable.from_parts('src1/spam.c', 'ham', 'result', 'int'), # skipped
info.Variable.from_parts('src1/spam.c', None, 'var2', 'static PyObject *'),
info.Variable.from_parts('src1/eggs.c', 'tofu', 'ready', 'static int'),
info.Variable.from_parts('src1/spam.c', None, 'freelist', 'static (PyTupleObject *)[10]'),
Expand All @@ -318,6 +320,7 @@ def test_typical(self):

self.assertEqual(found, [
info.Variable.from_parts('src1/spam.c', None, 'var1', 'static const char *'),
info.Variable.from_parts('src1/spam.c', None, 'var1b', 'const char *'),
info.Variable.from_parts('src1/spam.c', 'ham', 'initialized', 'static int'),
info.Variable.from_parts('src1/spam.c', None, 'var2', 'static PyObject *'),
info.Variable.from_parts('src1/eggs.c', 'tofu', 'ready', 'static int'),
Expand Down
6 changes: 6 additions & 0 deletions Lib/test/test_tools/test_c_analyzer/test_c_parser/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import os.path
from test.support import load_package_tests


def load_tests(*args):
return load_package_tests(os.path.dirname(__file__), *args)
108 changes: 72 additions & 36 deletions Lib/test/test_tools/test_c_analyzer/test_c_parser/test_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from ..util import PseudoStr, StrProxy, Object
from .. import tool_imports_for_tests
with tool_imports_for_tests():
from c_analyzer_common.info import ID
from c_analyzer_common.info import ID, UNKNOWN
from c_parser.info import (
normalize_vartype, Variable,
)
Expand All @@ -31,38 +31,47 @@ class VariableTests(unittest.TestCase):

VALID_ARGS = (
('x/y/z/spam.c', 'func', 'eggs'),
'static',
'int',
)
VALID_KWARGS = dict(zip(Variable._fields, VALID_ARGS))
VALID_EXPECTED = VALID_ARGS

def test_init_typical_global(self):
static = Variable(
id=ID(
filename='x/y/z/spam.c',
funcname=None,
name='eggs',
),
vartype='int',
)
for storage in ('static', 'extern', 'implicit'):
with self.subTest(storage):
static = Variable(
id=ID(
filename='x/y/z/spam.c',
funcname=None,
name='eggs',
),
storage=storage,
vartype='int',
)

self.assertEqual(static, (
('x/y/z/spam.c', None, 'eggs'),
'int',
))
self.assertEqual(static, (
('x/y/z/spam.c', None, 'eggs'),
storage,
'int',
))

def test_init_typical_local(self):
static = Variable(
id=ID(
filename='x/y/z/spam.c',
funcname='func',
name='eggs',
),
vartype='int',
)
for storage in ('static', 'local'):
with self.subTest(storage):
static = Variable(
id=ID(
filename='x/y/z/spam.c',
funcname='func',
name='eggs',
),
storage=storage,
vartype='int',
)

self.assertEqual(static, (
('x/y/z/spam.c', 'func', 'eggs'),
storage,
'int',
))

Expand All @@ -71,10 +80,12 @@ def test_init_all_missing(self):
with self.subTest(repr(value)):
static = Variable(
id=value,
storage=value,
vartype=value,
)

self.assertEqual(static, (
None,
None,
None,
))
Expand All @@ -89,34 +100,42 @@ def test_init_all_coerced(self):
PseudoStr('func'),
PseudoStr('spam'),
),
storage=PseudoStr('static'),
vartype=PseudoStr('int'),
),
(id,
'static',
'int',
)),
('non-str 1',
dict(
id=id,
storage=Object(),
vartype=Object(),
),
(id,
'<object>',
'<object>',
)),
('non-str 2',
dict(
id=id,
storage=StrProxy('static'),
vartype=StrProxy('variable'),
),
(id,
'static',
'variable',
)),
('non-str',
dict(
id=id,
vartype=('a', 'b', 'c'),
storage=('a', 'b', 'c'),
vartype=('x', 'y', 'z'),
),
(id,
"('a', 'b', 'c')",
"('x', 'y', 'z')",
)),
]
for summary, kwargs, expected in tests:
Expand All @@ -134,43 +153,57 @@ def test_init_all_coerced(self):
def test_iterable(self):
static = Variable(**self.VALID_KWARGS)

id, vartype = static
id, storage, vartype = static

values = (id, vartype)
values = (id, storage, vartype)
for value, expected in zip(values, self.VALID_EXPECTED):
self.assertEqual(value, expected)

def test_fields(self):
static = Variable(('a', 'b', 'z'), 'x')
static = Variable(('a', 'b', 'z'), 'x', 'y')

self.assertEqual(static.id, ('a', 'b', 'z'))
self.assertEqual(static.vartype, 'x')
self.assertEqual(static.storage, 'x')
self.assertEqual(static.vartype, 'y')

def test___getattr__(self):
static = Variable(('a', 'b', 'z'), 'x')
static = Variable(('a', 'b', 'z'), 'x', 'y')

self.assertEqual(static.filename, 'a')
self.assertEqual(static.funcname, 'b')
self.assertEqual(static.name, 'z')

def test_validate_typical(self):
static = Variable(
id=ID(
filename='x/y/z/spam.c',
funcname='func',
name='eggs',
),
vartype='int',
)
validstorage = ('static', 'extern', 'implicit', 'local')
self.assertEqual(set(validstorage), set(Variable.STORAGE))

for storage in validstorage:
with self.subTest(storage):
static = Variable(
id=ID(
filename='x/y/z/spam.c',
funcname='func',
name='eggs',
),
storage=storage,
vartype='int',
)

static.validate() # This does not fail.
static.validate() # This does not fail.

def test_validate_missing_field(self):
for field in Variable._fields:
with self.subTest(field):
static = Variable(**self.VALID_KWARGS)
static = static._replace(**{field: None})

with self.assertRaises(TypeError):
static.validate()
for field in ('storage', 'vartype'):
with self.subTest(field):
static = Variable(**self.VALID_KWARGS)
static = static._replace(**{field: UNKNOWN})

with self.assertRaises(TypeError):
static.validate()

Expand All @@ -185,6 +218,7 @@ def test_validate_bad_field(self):
) + badch
tests = [
('id', ()), # Any non-empty str is okay.
('storage', ('external', 'global') + notnames),
('vartype', ()), # Any non-empty str is okay.
]
seen = set()
Expand All @@ -199,6 +233,8 @@ def test_validate_bad_field(self):
static.validate()

for field, invalid in tests:
if field == 'id':
continue
valid = seen - set(invalid)
for value in valid:
with self.subTest(f'{field}={value!r}'):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import os.path
from test.support import load_package_tests


def load_tests(*args):
return load_package_tests(os.path.dirname(__file__), *args)
4 changes: 2 additions & 2 deletions Tools/c-analyzer/c_analyzer_common/_generate.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ def _known(symbol):
raise
if symbol.name not in decl:
decl = decl + symbol.name
return Variable(varid, decl)
return Variable(varid, 'static', decl)


def known_row(varid, decl):
Expand Down Expand Up @@ -291,7 +291,7 @@ def known_rows(symbols, *,
except KeyError:
found = _find_match(symbol, cache, filenames)
if found is None:
found = Variable(symbol.id, UNKNOWN)
found = Variable(symbol.id, UNKNOWN, UNKNOWN)
yield _as_known(found.id, found.vartype)
else:
raise NotImplementedError # XXX incorporate KNOWN
Expand Down
53 changes: 30 additions & 23 deletions Tools/c-analyzer/c_analyzer_common/known.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,34 +34,41 @@ def from_file(infile, *,
id = ID(filename, funcname, name)
if kind == 'variable':
values = known['variables']
value = Variable(id, declaration)
value._isglobal = _is_global(declaration) or id.funcname is None
if funcname:
storage = _get_storage(declaration) or 'local'
else:
storage = _get_storage(declaration) or 'implicit'
value = Variable(id, storage, declaration)
else:
raise ValueError(f'unsupported kind in row {row}')
if value.name == 'id' and declaration == UNKNOWN:
# None of these are variables.
declaration = 'int id';
else:
value.validate()
value.validate()
# if value.name == 'id' and declaration == UNKNOWN:
# # None of these are variables.
# declaration = 'int id';
# else:
# value.validate()
values[id] = value
return known


def _is_global(vartype):
def _get_storage(decl):
# statics
if vartype.startswith('static '):
return True
if vartype.startswith(('Py_LOCAL(', 'Py_LOCAL_INLINE(')):
return True
if vartype.startswith(('_Py_IDENTIFIER(', '_Py_static_string(')):
return True
if vartype.startswith('PyDoc_VAR('):
return True
if vartype.startswith(('SLOT1BINFULL(', 'SLOT1BIN(')):
return True
if vartype.startswith('WRAP_METHOD('):
return True
if decl.startswith('static '):
return 'static'
if decl.startswith(('Py_LOCAL(', 'Py_LOCAL_INLINE(')):
return 'static'
if decl.startswith(('_Py_IDENTIFIER(', '_Py_static_string(')):
return 'static'
if decl.startswith('PyDoc_VAR('):
return 'static'
if decl.startswith(('SLOT1BINFULL(', 'SLOT1BIN(')):
return 'static'
if decl.startswith('WRAP_METHOD('):
return 'static'
# public extern
if vartype.startswith('PyAPI_DATA('):
return True
return False
if decl.startswith('extern '):
return 'extern'
if decl.startswith('PyAPI_DATA('):
return 'extern'
# implicit or local
return None
Loading