From 15898eec23485eb53141c49800eb864a37ee7154 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 8 Aug 2020 23:14:27 +0200 Subject: [PATCH 01/19] Allow Python files not to be executable .py files may be modules which are not standalone program, so allow them not to be executable. Signed-off-by: Gilles Peskine --- tests/scripts/check_files.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/scripts/check_files.py b/tests/scripts/check_files.py index 13fee9d76..c498117a9 100755 --- a/tests/scripts/check_files.py +++ b/tests/scripts/check_files.py @@ -161,9 +161,13 @@ class PermissionIssueTracker(FileIssueTracker): heading = "Incorrect permissions:" + # .py files can be either full scripts or modules, so they may or may + # not be executable. + suffix_exemptions = frozenset({".py"}) + def check_file_for_issue(self, filepath): is_executable = os.access(filepath, os.X_OK) - should_be_executable = filepath.endswith((".sh", ".pl", ".py")) + should_be_executable = filepath.endswith((".sh", ".pl")) if is_executable != should_be_executable: self.files_with_issues[filepath] = None From 4aebb8d936487ed96fda96b5de24349c1f3f0e88 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 8 Aug 2020 23:15:18 +0200 Subject: [PATCH 02/19] Test shebang lines Executable scripts must have shebang (#!) line to be effectively executable on most Unix-like systems. Enforce this, and conversely enforce that files with a shebang line are executable. Check that the specified interperter is consistent with the file extension. Signed-off-by: Gilles Peskine --- tests/scripts/check_files.py | 48 ++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/scripts/check_files.py b/tests/scripts/check_files.py index c498117a9..09ab615ab 100755 --- a/tests/scripts/check_files.py +++ b/tests/scripts/check_files.py @@ -172,6 +172,53 @@ class PermissionIssueTracker(FileIssueTracker): self.files_with_issues[filepath] = None +class ShebangIssueTracker(FileIssueTracker): + """Track files with a bad, missing or extraneous shebang line. + + Executable scripts must start with a valid shebang (#!) line. + """ + + heading = "Invalid shebang line:" + + # Allow either /bin/sh, /bin/bash, or /usr/bin/env. + # Allow at most one argument (this is a Linux limitation). + # For sh and bash, the argument if present must be options. + # For env, the argument must be the base name of the interpeter. + _shebang_re = re.compile(rb'^#! ?(?:/bin/(bash|sh)(?: -[^\n ]*)?' + rb'|/usr/bin/env ([^\n /]+))$') + _extensions = { + b'bash': 'sh', + b'perl': 'pl', + b'python3': 'py', + b'sh': 'sh', + } + + def is_valid_shebang(self, first_line, filepath): + m = re.match(self._shebang_re, first_line) + if not m: + return False + interpreter = m.group(1) or m.group(2) + if interpreter not in self._extensions: + return False + if not filepath.endswith('.' + self._extensions[interpreter]): + return False + return True + + def check_file_for_issue(self, filepath): + is_executable = os.access(filepath, os.X_OK) + with open(filepath, "rb") as f: + first_line = f.readline() + if first_line.startswith(b'#!'): + if not is_executable: + # Shebang on a non-executable file + self.files_with_issues[filepath] = None + elif not self.is_valid_shebang(first_line, filepath): + self.files_with_issues[filepath] = [1] + elif is_executable: + # Executable without a shebang + self.files_with_issues[filepath] = None + + class EndOfFileNewlineIssueTracker(FileIssueTracker): """Track files that end with an incomplete line (no newline character at the end of the last line).""" @@ -292,6 +339,7 @@ class IntegrityChecker: self.setup_logger(log_file) self.issues_to_check = [ PermissionIssueTracker(), + ShebangIssueTracker(), EndOfFileNewlineIssueTracker(), Utf8BomIssueTracker(), UnixLineEndingIssueTracker(), From ac9e7c0b6e61731306997aab49d074adc0faf58c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 11 Aug 2020 15:11:50 +0200 Subject: [PATCH 03/19] check_files.py: pass mypy Add enough type annotations to pass mypy 0.782 with Python 3.5. The source code will still run normally under older Python versions. Signed-off-by: Gilles Peskine --- tests/scripts/check_files.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/scripts/check_files.py b/tests/scripts/check_files.py index 09ab615ab..8857e0021 100755 --- a/tests/scripts/check_files.py +++ b/tests/scripts/check_files.py @@ -29,6 +29,10 @@ import codecs import re import subprocess import sys +try: + from typing import FrozenSet, Optional, Pattern # pylint: disable=unused-import +except ImportError: + pass class FileIssueTracker: @@ -48,8 +52,8 @@ class FileIssueTracker: ``heading``: human-readable description of the issue """ - suffix_exemptions = frozenset() - path_exemptions = None + suffix_exemptions = frozenset() #type: FrozenSet[str] + path_exemptions = None #type: Optional[Pattern[str]] # heading must be defined in derived classes. # pylint: disable=no-member From 38b66dfc857cb3478dd8dcda61aa8d6783c6fb41 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 12 Aug 2020 02:11:38 +0200 Subject: [PATCH 04/19] test_generate_test_code: remove Python 2 compatibility code This makes the code cleaner. As a bonus, mypy no longer gets confused. Signed-off-by: Gilles Peskine --- tests/scripts/test_generate_test_code.py | 74 ++++-------------------- 1 file changed, 11 insertions(+), 63 deletions(-) diff --git a/tests/scripts/test_generate_test_code.py b/tests/scripts/test_generate_test_code.py index 000c2a701..9bf66f1cc 100755 --- a/tests/scripts/test_generate_test_code.py +++ b/tests/scripts/test_generate_test_code.py @@ -20,21 +20,10 @@ Unit tests for generate_test_code.py """ -# pylint: disable=wrong-import-order -try: - # Python 2 - from StringIO import StringIO -except ImportError: - # Python 3 - from io import StringIO +from io import StringIO from unittest import TestCase, main as unittest_main -try: - # Python 2 - from mock import patch -except ImportError: - # Python 3 - from unittest.mock import patch -# pylint: enable=wrong-import-order +from unittest.mock import patch + from generate_test_code import gen_dependencies, gen_dependencies_one_line from generate_test_code import gen_function_wrapper, gen_dispatch from generate_test_code import parse_until_pattern, GeneratorInputError @@ -317,25 +306,16 @@ class StringIOWrapper(StringIO): :return: Line read from file. """ parent = super(StringIOWrapper, self) - if getattr(parent, 'next', None): - # Python 2 - line = parent.next() - else: - # Python 3 - line = parent.__next__() + line = parent.__next__() return line - # Python 3 - __next__ = next - - def readline(self, length=0): + def readline(self, _length=0): """ Wrap the base class readline. :param length: :return: """ - # pylint: disable=unused-argument line = super(StringIOWrapper, self).readline() if line is not None: self.line_no += 1 @@ -549,38 +529,6 @@ class ParseFunctionCode(TestCase): Test suite for testing parse_function_code() """ - def assert_raises_regex(self, exp, regex, func, *args): - """ - Python 2 & 3 portable wrapper of assertRaisesRegex(p)? function. - - :param exp: Exception type expected to be raised by cb. - :param regex: Expected exception message - :param func: callable object under test - :param args: variable positional arguments - """ - parent = super(ParseFunctionCode, self) - - # Pylint does not appreciate that the super method called - # conditionally can be available in other Python version - # then that of Pylint. - # Workaround is to call the method via getattr. - # Pylint ignores that the method got via getattr is - # conditionally executed. Method has to be a callable. - # Hence, using a dummy callable for getattr default. - dummy = lambda *x: None - # First Python 3 assertRaisesRegex is checked, since Python 2 - # assertRaisesRegexp is also available in Python 3 but is - # marked deprecated. - for name in ('assertRaisesRegex', 'assertRaisesRegexp'): - method = getattr(parent, name, dummy) - if method is not dummy: - method(exp, regex, func, *args) - break - else: - raise AttributeError(" 'ParseFunctionCode' object has no attribute" - " 'assertRaisesRegex' or 'assertRaisesRegexp'" - ) - def test_no_function(self): """ Test no test function found. @@ -593,8 +541,8 @@ function ''' stream = StringIOWrapper('test_suite_ut.function', data) err_msg = 'file: test_suite_ut.function - Test functions not found!' - self.assert_raises_regex(GeneratorInputError, err_msg, - parse_function_code, stream, [], []) + self.assertRaisesRegex(GeneratorInputError, err_msg, + parse_function_code, stream, [], []) def test_no_end_case_comment(self): """ @@ -609,8 +557,8 @@ void test_func() stream = StringIOWrapper('test_suite_ut.function', data) err_msg = r'file: test_suite_ut.function - '\ 'end case pattern .*? not found!' - self.assert_raises_regex(GeneratorInputError, err_msg, - parse_function_code, stream, [], []) + self.assertRaisesRegex(GeneratorInputError, err_msg, + parse_function_code, stream, [], []) @patch("generate_test_code.parse_function_arguments") def test_function_called(self, @@ -727,8 +675,8 @@ exit: data = 'int entropy_threshold( char * a, data_t * h, int result )' err_msg = 'file: test_suite_ut.function - Test functions not found!' stream = StringIOWrapper('test_suite_ut.function', data) - self.assert_raises_regex(GeneratorInputError, err_msg, - parse_function_code, stream, [], []) + self.assertRaisesRegex(GeneratorInputError, err_msg, + parse_function_code, stream, [], []) @patch("generate_test_code.gen_dispatch") @patch("generate_test_code.gen_dependencies") From e6d0ac26ca67d836ced78122135efc5e1af09a7d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 12 Aug 2020 02:28:02 +0200 Subject: [PATCH 05/19] mbedtls_test.py: Tell mypy to ignore mbed_host_tests Since no typing stubs are available for mbed_host_tests.py, mypy errors out on mbedtls_test.py with error: Skipping analyzing 'mbed_host_tests': found module but no type hints or library stubs Ignore this import to get at least some benefit from mypy without spending significant effort to write stubs. Signed-off-by: Gilles Peskine --- tests/scripts/mbedtls_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/mbedtls_test.py b/tests/scripts/mbedtls_test.py index a5d094064..64f12bbb3 100755 --- a/tests/scripts/mbedtls_test.py +++ b/tests/scripts/mbedtls_test.py @@ -38,7 +38,7 @@ import re import os import binascii -from mbed_host_tests import BaseHostTest, event_callback # pylint: disable=import-error +from mbed_host_tests import BaseHostTest, event_callback # type: ignore # pylint: disable=import-error class TestDataParserError(Exception): From 7be4551f236560d487da3dd8543ff93fe3f4cd10 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 12 Aug 2020 02:31:02 +0200 Subject: [PATCH 06/19] Check typing of python scripts if mypy is available Signed-off-by: Gilles Peskine --- tests/scripts/check-python-files.sh | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/tests/scripts/check-python-files.sh b/tests/scripts/check-python-files.sh index 518c423d9..a80860d51 100755 --- a/tests/scripts/check-python-files.sh +++ b/tests/scripts/check-python-files.sh @@ -14,11 +14,13 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -# -# Purpose: -# -# Run 'pylint' on Python files for programming errors and helps enforcing -# PEP8 coding standards. + +# Purpose: check Python files for potential programming errors or maintenance +# hurdles. Run pylint to detect some potential mistakes and enforce PEP8 +# coding standards. If available, run mypy to perform static type checking. + +# We'll keep going on errors and report the status at the end. +ret=0 if type python3 >/dev/null 2>/dev/null; then PYTHON=python3 @@ -26,4 +28,17 @@ else PYTHON=python fi -$PYTHON -m pylint -j 2 scripts/*.py tests/scripts/*.py +$PYTHON -m pylint -j 2 scripts/*.py tests/scripts/*.py || { + echo >&2 "pylint reported errors" + ret=1 +} + +# Check types if mypy is available +if type mypy >/dev/null 2>/dev/null; then + echo + echo 'Running mypy ...' + mypy scripts/*.py tests/scripts/*.py || + ret=1 +fi + +exit $ret From 45d350b9dc97ac73ea72d7b28ae57c2c01a89e90 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 10 Dec 2020 23:11:59 +0100 Subject: [PATCH 07/19] Allow tests/scripts/foo.py to import from scripts Allow Python scripts in tests/scripts to import modules located in the scripts directory. To do this, use ``` import scripts_path # pylint: disable=unused-import ``` Declare the scripts directory to pylint and to mypy. Signed-off-by: Gilles Peskine --- .mypy.ini | 3 +++ .pylintrc | 3 +++ tests/scripts/scripts_path.py | 28 ++++++++++++++++++++++++++++ 3 files changed, 34 insertions(+) create mode 100644 .mypy.ini create mode 100644 tests/scripts/scripts_path.py diff --git a/.mypy.ini b/.mypy.ini new file mode 100644 index 000000000..31d92ccd2 --- /dev/null +++ b/.mypy.ini @@ -0,0 +1,3 @@ +[mypy] +mypy_path = scripts +warn_unused_configs = True diff --git a/.pylintrc b/.pylintrc index ad25a7ca1..5f3d2b235 100644 --- a/.pylintrc +++ b/.pylintrc @@ -1,3 +1,6 @@ +[MASTER] +init-hook='import sys; sys.path.append("scripts")' + [BASIC] # We're ok with short funtion argument names. # [invalid-name] diff --git a/tests/scripts/scripts_path.py b/tests/scripts/scripts_path.py new file mode 100644 index 000000000..10bf6f852 --- /dev/null +++ b/tests/scripts/scripts_path.py @@ -0,0 +1,28 @@ +"""Add our Python library directory to the module search path. + +Usage: + + import scripts_path # pylint: disable=unused-import +""" + +# Copyright The Mbed TLS Contributors +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os +import sys + +sys.path.append(os.path.join(os.path.dirname(__file__), + os.path.pardir, os.path.pardir, + 'scripts')) From fc62211e3b7ab294d767cb796b14991280f374ab Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 11 Dec 2020 00:27:14 +0100 Subject: [PATCH 08/19] Refactor and generalize run_c Generalize the very ad hoc run_c function into a function to generate a C program to print the value of a list of expressions. Refactor the code into several functions to make it more manageable. No intended behavior change. Signed-off-by: Gilles Peskine --- tests/scripts/test_psa_constant_names.py | 133 ++++++++++++++++++----- 1 file changed, 104 insertions(+), 29 deletions(-) diff --git a/tests/scripts/test_psa_constant_names.py b/tests/scripts/test_psa_constant_names.py index c7011a784..68bfe77be 100755 --- a/tests/scripts/test_psa_constant_names.py +++ b/tests/scripts/test_psa_constant_names.py @@ -313,40 +313,97 @@ def remove_file_if_exists(filename): except OSError: pass -def run_c(type_word, expressions, include_path=None, keep_c=False): - """Generate and run a program to print out numerical values for expressions.""" - if include_path is None: - include_path = [] - if type_word == 'status': - cast_to = 'long' - printf_format = '%ld' - else: - cast_to = 'unsigned long' - printf_format = '0x%08lx' - c_name = None - exe_name = None - try: - c_fd, c_name = tempfile.mkstemp(prefix='tmp-{}-'.format(type_word), - suffix='.c', - dir='programs/psa') - exe_suffix = '.exe' if platform.system() == 'Windows' else '' - exe_name = c_name[:-2] + exe_suffix - remove_file_if_exists(exe_name) - c_file = os.fdopen(c_fd, 'w', encoding='ascii') - c_file.write('/* Generated by test_psa_constant_names.py for {} values */' - .format(type_word)) - c_file.write(''' +def create_c_file(file_label): + """Create a temporary C file. + + * ``file_label``: a string that will be included in the file name. + + Return ```(c_file, c_name, exe_name)``` where ``c_file`` is a Python + stream open for writing to the file, ``c_name`` is the name of the file + and ``exe_name`` is the name of the executable that will be produced + by compiling the file. + """ + c_fd, c_name = tempfile.mkstemp(prefix='tmp-{}-'.format(file_label), + suffix='.c') + exe_suffix = '.exe' if platform.system() == 'Windows' else '' + exe_name = c_name[:-2] + exe_suffix + remove_file_if_exists(exe_name) + c_file = os.fdopen(c_fd, 'w', encoding='ascii') + return c_file, c_name, exe_name + +def generate_c_printf_expressions(c_file, cast_to, printf_format, expressions): + """Generate C instructions to print the value of ``expressions``. + + Write the code with ``c_file``'s ``write`` method. + + Each expression is cast to the type ``cast_to`` and printed with the + printf format ``printf_format``. + """ + for expr in expressions: + c_file.write(' printf("{}\\n", ({}) {});\n' + .format(printf_format, cast_to, expr)) + +def generate_c_file(c_file, + caller, header, + main_generator): + """Generate a temporary C source file. + + * ``c_file`` is an open stream on the C source file. + * ``caller``: an informational string written in a comment at the top + of the file. + * ``header``: extra code to insert before any function in the generated + C file. + * ``main_generator``: a function called with ``c_file`` as its sole argument + to generate the body of the ``main()`` function. + """ + c_file.write('/* Generated by {} */' + .format(caller)) + c_file.write(''' #include -#include +''') + c_file.write(header) + c_file.write(''' int main(void) { ''') - for expr in expressions: - c_file.write(' printf("{}\\n", ({}) {});\n' - .format(printf_format, cast_to, expr)) - c_file.write(''' return 0; + main_generator(c_file) + c_file.write(''' return 0; } ''') + +def get_c_expression_values( + cast_to, printf_format, + expressions, + caller=__name__, file_label='', + header='', include_path=None, + keep_c=False, +): # pylint: disable=too-many-arguments + """Generate and run a program to print out numerical values for expressions. + + * ``cast_to``: a C type. + * ``printf_format``: a printf format suitable for the type ``cast_to``. + * ``header``: extra code to insert before any function in the generated + C file. + * ``expressions``: a list of C language expressions that have the type + ``type``. + * ``include_path``: a list of directories containing header files. + * ``keep_c``: if true, keep the temporary C file (presumably for debugging + purposes). + + Return the list of values of the ``expressions``. + """ + if include_path is None: + include_path = [] + c_name = None + exe_name = None + try: + c_file, c_name, exe_name = create_c_file(file_label) + generate_c_file( + c_file, caller, header, + lambda c_file: generate_c_printf_expressions(c_file, + cast_to, printf_format, + expressions) + ) c_file.close() cc = os.getenv('CC', 'cc') subprocess.check_call([cc] + @@ -354,7 +411,7 @@ int main(void) ['-o', exe_name, c_name]) if keep_c: sys.stderr.write('List of {} tests kept at {}\n' - .format(type_word, c_name)) + .format(caller, c_name)) else: os.remove(c_name) output = subprocess.check_output([exe_name]) @@ -362,6 +419,24 @@ int main(void) finally: remove_file_if_exists(exe_name) +def run_c(type_word, expressions, include_path=None, keep_c=False): + """Generate and run a program to print out numerical values for expressions.""" + if type_word == 'status': + cast_to = 'long' + printf_format = '%ld' + else: + cast_to = 'unsigned long' + printf_format = '0x%08lx' + return get_c_expression_values( + cast_to, printf_format, + expressions, + caller='test_psa_constant_names.py for {} values'.format(type_word), + file_label=type_word, + header='#include ', + include_path=include_path, + keep_c=keep_c + ) + NORMALIZE_STRIP_RE = re.compile(r'\s+') def normalize(expr): """Normalize the C expression so as not to care about trivial differences. From 2adebc89dabff2ab5ddf776f365f4e08dfbf6706 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 11 Dec 2020 00:30:53 +0100 Subject: [PATCH 09/19] Move get_c_expression_values into a separate module Create a directory mbedtls_dev intended to contain various Python module for use by Python scripts located anywhere in the Mbed TLS source tree. Move get_c_expression_values and its auxiliary functions into a new Python module mbedtls_dev.c_build_helper. Signed-off-by: Gilles Peskine --- scripts/mbedtls_dev/c_build_helper.py | 138 +++++++++++++++++++++++ tests/scripts/test_psa_constant_names.py | 122 +------------------- 2 files changed, 142 insertions(+), 118 deletions(-) create mode 100644 scripts/mbedtls_dev/c_build_helper.py diff --git a/scripts/mbedtls_dev/c_build_helper.py b/scripts/mbedtls_dev/c_build_helper.py new file mode 100644 index 000000000..680760247 --- /dev/null +++ b/scripts/mbedtls_dev/c_build_helper.py @@ -0,0 +1,138 @@ +"""Generate and run C code. +""" + +# Copyright The Mbed TLS Contributors +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os +import platform +import subprocess +import sys +import tempfile + +def remove_file_if_exists(filename): + """Remove the specified file, ignoring errors.""" + if not filename: + return + try: + os.remove(filename) + except OSError: + pass + +def create_c_file(file_label): + """Create a temporary C file. + + * ``file_label``: a string that will be included in the file name. + + Return ```(c_file, c_name, exe_name)``` where ``c_file`` is a Python + stream open for writing to the file, ``c_name`` is the name of the file + and ``exe_name`` is the name of the executable that will be produced + by compiling the file. + """ + c_fd, c_name = tempfile.mkstemp(prefix='tmp-{}-'.format(file_label), + suffix='.c') + exe_suffix = '.exe' if platform.system() == 'Windows' else '' + exe_name = c_name[:-2] + exe_suffix + remove_file_if_exists(exe_name) + c_file = os.fdopen(c_fd, 'w', encoding='ascii') + return c_file, c_name, exe_name + +def generate_c_printf_expressions(c_file, cast_to, printf_format, expressions): + """Generate C instructions to print the value of ``expressions``. + + Write the code with ``c_file``'s ``write`` method. + + Each expression is cast to the type ``cast_to`` and printed with the + printf format ``printf_format``. + """ + for expr in expressions: + c_file.write(' printf("{}\\n", ({}) {});\n' + .format(printf_format, cast_to, expr)) + +def generate_c_file(c_file, + caller, header, + main_generator): + """Generate a temporary C source file. + + * ``c_file`` is an open stream on the C source file. + * ``caller``: an informational string written in a comment at the top + of the file. + * ``header``: extra code to insert before any function in the generated + C file. + * ``main_generator``: a function called with ``c_file`` as its sole argument + to generate the body of the ``main()`` function. + """ + c_file.write('/* Generated by {} */' + .format(caller)) + c_file.write(''' +#include +''') + c_file.write(header) + c_file.write(''' +int main(void) +{ +''') + main_generator(c_file) + c_file.write(''' return 0; +} +''') + +def get_c_expression_values( + cast_to, printf_format, + expressions, + caller=__name__, file_label='', + header='', include_path=None, + keep_c=False, +): # pylint: disable=too-many-arguments + """Generate and run a program to print out numerical values for expressions. + + * ``cast_to``: a C type. + * ``printf_format``: a printf format suitable for the type ``cast_to``. + * ``header``: extra code to insert before any function in the generated + C file. + * ``expressions``: a list of C language expressions that have the type + ``type``. + * ``include_path``: a list of directories containing header files. + * ``keep_c``: if true, keep the temporary C file (presumably for debugging + purposes). + + Return the list of values of the ``expressions``. + """ + if include_path is None: + include_path = [] + c_name = None + exe_name = None + try: + c_file, c_name, exe_name = create_c_file(file_label) + generate_c_file( + c_file, caller, header, + lambda c_file: generate_c_printf_expressions(c_file, + cast_to, printf_format, + expressions) + ) + c_file.close() + cc = os.getenv('CC', 'cc') + subprocess.check_call([cc] + + ['-I' + dir for dir in include_path] + + ['-o', exe_name, c_name]) + if keep_c: + sys.stderr.write('List of {} tests kept at {}\n' + .format(caller, c_name)) + else: + os.remove(c_name) + output = subprocess.check_output([exe_name]) + return output.decode('ascii').strip().split('\n') + finally: + remove_file_if_exists(exe_name) diff --git a/tests/scripts/test_psa_constant_names.py b/tests/scripts/test_psa_constant_names.py index 68bfe77be..694e2a9db 100755 --- a/tests/scripts/test_psa_constant_names.py +++ b/tests/scripts/test_psa_constant_names.py @@ -26,11 +26,12 @@ import argparse from collections import namedtuple import itertools import os -import platform import re import subprocess import sys -import tempfile + +import scripts_path # pylint: disable=unused-import +from mbedtls_dev import c_build_helper class ReadFileLineException(Exception): def __init__(self, filename, line_number): @@ -304,121 +305,6 @@ def gather_inputs(headers, test_suites, inputs_class=Inputs): inputs.gather_arguments() return inputs -def remove_file_if_exists(filename): - """Remove the specified file, ignoring errors.""" - if not filename: - return - try: - os.remove(filename) - except OSError: - pass - -def create_c_file(file_label): - """Create a temporary C file. - - * ``file_label``: a string that will be included in the file name. - - Return ```(c_file, c_name, exe_name)``` where ``c_file`` is a Python - stream open for writing to the file, ``c_name`` is the name of the file - and ``exe_name`` is the name of the executable that will be produced - by compiling the file. - """ - c_fd, c_name = tempfile.mkstemp(prefix='tmp-{}-'.format(file_label), - suffix='.c') - exe_suffix = '.exe' if platform.system() == 'Windows' else '' - exe_name = c_name[:-2] + exe_suffix - remove_file_if_exists(exe_name) - c_file = os.fdopen(c_fd, 'w', encoding='ascii') - return c_file, c_name, exe_name - -def generate_c_printf_expressions(c_file, cast_to, printf_format, expressions): - """Generate C instructions to print the value of ``expressions``. - - Write the code with ``c_file``'s ``write`` method. - - Each expression is cast to the type ``cast_to`` and printed with the - printf format ``printf_format``. - """ - for expr in expressions: - c_file.write(' printf("{}\\n", ({}) {});\n' - .format(printf_format, cast_to, expr)) - -def generate_c_file(c_file, - caller, header, - main_generator): - """Generate a temporary C source file. - - * ``c_file`` is an open stream on the C source file. - * ``caller``: an informational string written in a comment at the top - of the file. - * ``header``: extra code to insert before any function in the generated - C file. - * ``main_generator``: a function called with ``c_file`` as its sole argument - to generate the body of the ``main()`` function. - """ - c_file.write('/* Generated by {} */' - .format(caller)) - c_file.write(''' -#include -''') - c_file.write(header) - c_file.write(''' -int main(void) -{ -''') - main_generator(c_file) - c_file.write(''' return 0; -} -''') - -def get_c_expression_values( - cast_to, printf_format, - expressions, - caller=__name__, file_label='', - header='', include_path=None, - keep_c=False, -): # pylint: disable=too-many-arguments - """Generate and run a program to print out numerical values for expressions. - - * ``cast_to``: a C type. - * ``printf_format``: a printf format suitable for the type ``cast_to``. - * ``header``: extra code to insert before any function in the generated - C file. - * ``expressions``: a list of C language expressions that have the type - ``type``. - * ``include_path``: a list of directories containing header files. - * ``keep_c``: if true, keep the temporary C file (presumably for debugging - purposes). - - Return the list of values of the ``expressions``. - """ - if include_path is None: - include_path = [] - c_name = None - exe_name = None - try: - c_file, c_name, exe_name = create_c_file(file_label) - generate_c_file( - c_file, caller, header, - lambda c_file: generate_c_printf_expressions(c_file, - cast_to, printf_format, - expressions) - ) - c_file.close() - cc = os.getenv('CC', 'cc') - subprocess.check_call([cc] + - ['-I' + dir for dir in include_path] + - ['-o', exe_name, c_name]) - if keep_c: - sys.stderr.write('List of {} tests kept at {}\n' - .format(caller, c_name)) - else: - os.remove(c_name) - output = subprocess.check_output([exe_name]) - return output.decode('ascii').strip().split('\n') - finally: - remove_file_if_exists(exe_name) - def run_c(type_word, expressions, include_path=None, keep_c=False): """Generate and run a program to print out numerical values for expressions.""" if type_word == 'status': @@ -427,7 +313,7 @@ def run_c(type_word, expressions, include_path=None, keep_c=False): else: cast_to = 'unsigned long' printf_format = '0x%08lx' - return get_c_expression_values( + return c_build_helper.get_c_expression_values( cast_to, printf_format, expressions, caller='test_psa_constant_names.py for {} values'.format(type_word), From 86fc21cd3bdb90ecf368c6febe6e0e39b27f17fe Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 11 Dec 2020 00:33:05 +0100 Subject: [PATCH 10/19] mypy: support mbedtls_dev.foo Tell mypy to support packages without an __init__.py (PEP 420 namespace packages). Python 3.3 and (modern) Pylint support them out of the box, but mypy needs to be told to support them. Signed-off-by: Gilles Peskine --- .mypy.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/.mypy.ini b/.mypy.ini index 31d92ccd2..6b831ddb7 100644 --- a/.mypy.ini +++ b/.mypy.ini @@ -1,3 +1,4 @@ [mypy] mypy_path = scripts +namespace_packages = True warn_unused_configs = True From b13ed70b3268705af934505b38a7f56fb8d9c498 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 11 Dec 2020 00:58:48 +0100 Subject: [PATCH 11/19] Check scripts/mbedtls_dev/*.py with pylint mypy automatically checks the modules when it encounters them as imports. Don't make it check them twice, because it would complain about encountering them through different paths (via the command line as scripts/mbedtls_dev/*.py and via imports as just mbedtls_dev/*.py). Signed-off-by: Gilles Peskine --- tests/scripts/check-python-files.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/check-python-files.sh b/tests/scripts/check-python-files.sh index a80860d51..8f8026eb3 100755 --- a/tests/scripts/check-python-files.sh +++ b/tests/scripts/check-python-files.sh @@ -28,7 +28,7 @@ else PYTHON=python fi -$PYTHON -m pylint -j 2 scripts/*.py tests/scripts/*.py || { +$PYTHON -m pylint -j 2 scripts/mbedtls_dev/*.py scripts/*.py tests/scripts/*.py || { echo >&2 "pylint reported errors" ret=1 } From 1cc6a8ea15893e3afd2dd1d95fb44423169494b7 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 6 Jan 2021 17:02:33 +0100 Subject: [PATCH 12/19] Add --can-pylint and --can-mypy options With just the option --can-pylint or --can-mypy, check whether the requisite tool is available with an acceptable version and exit. Signed-off-by: Gilles Peskine --- tests/scripts/check-python-files.sh | 33 +++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/scripts/check-python-files.sh b/tests/scripts/check-python-files.sh index 8f8026eb3..03a147281 100755 --- a/tests/scripts/check-python-files.sh +++ b/tests/scripts/check-python-files.sh @@ -28,6 +28,39 @@ else PYTHON=python fi +can_pylint () { + # Pylint 1.5.2 from Ubuntu 16.04 is too old. + # Pylint 1.8.3 from Ubuntu 18.04 passed on the first commit containing this line. + $PYTHON -m pylint 2>/dev/null --version | awk ' + BEGIN {status = 1} + /^(pylint[0-9]*|__main__\.py) +[0-9]+\.[0-9]+/ { + split($2, version, /[^0-9]+/); + status = !(version[1] >= 2 || (version[1] == 1 && version[2] >= 8)); + exit; # executes the END block + } + END {exit status} + ' +} + +can_mypy () { + # Just check that mypy is present and looks sane. I don't know what + # minimum version is required. The check is not just "type mypy" + # becaues that passes if a mypy exists but is not installed for the current + # python version. + mypy --version 2>/dev/null >/dev/null +} + +# With just a --can-xxx option, check whether the tool for xxx is available +# with an acceptable version, and exit without running any checks. The exit +# status is true if the tool is available and acceptable and false otherwise. +if [ "$1" = "--can-pylint" ]; then + can_pylint + exit +elif [ "$1" = "--can-mypy" ]; then + can_mypy + exit +fi + $PYTHON -m pylint -j 2 scripts/mbedtls_dev/*.py scripts/*.py tests/scripts/*.py || { echo >&2 "pylint reported errors" ret=1 From 2991b5f6c0cbb17e12aa426f8e072e267076d478 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 19 Jan 2021 21:19:02 +0100 Subject: [PATCH 13/19] Minor documentation fixes Signed-off-by: Gilles Peskine --- scripts/mbedtls_dev/c_build_helper.py | 2 +- tests/scripts/check-python-files.sh | 5 +++-- tests/scripts/test_psa_constant_names.py | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/scripts/mbedtls_dev/c_build_helper.py b/scripts/mbedtls_dev/c_build_helper.py index 680760247..5c587a16b 100644 --- a/scripts/mbedtls_dev/c_build_helper.py +++ b/scripts/mbedtls_dev/c_build_helper.py @@ -103,7 +103,7 @@ def get_c_expression_values( * ``header``: extra code to insert before any function in the generated C file. * ``expressions``: a list of C language expressions that have the type - ``type``. + ``cast_to``. * ``include_path``: a list of directories containing header files. * ``keep_c``: if true, keep the temporary C file (presumably for debugging purposes). diff --git a/tests/scripts/check-python-files.sh b/tests/scripts/check-python-files.sh index 03a147281..11e351b60 100755 --- a/tests/scripts/check-python-files.sh +++ b/tests/scripts/check-python-files.sh @@ -29,7 +29,8 @@ else fi can_pylint () { - # Pylint 1.5.2 from Ubuntu 16.04 is too old. + # Pylint 1.5.2 from Ubuntu 16.04 is too old: + # E: 34, 0: Unable to import 'mbedtls_dev' (import-error) # Pylint 1.8.3 from Ubuntu 18.04 passed on the first commit containing this line. $PYTHON -m pylint 2>/dev/null --version | awk ' BEGIN {status = 1} @@ -45,7 +46,7 @@ can_pylint () { can_mypy () { # Just check that mypy is present and looks sane. I don't know what # minimum version is required. The check is not just "type mypy" - # becaues that passes if a mypy exists but is not installed for the current + # because that passes if a mypy exists but is not installed for the current # python version. mypy --version 2>/dev/null >/dev/null } diff --git a/tests/scripts/test_psa_constant_names.py b/tests/scripts/test_psa_constant_names.py index 694e2a9db..5de59c2f4 100755 --- a/tests/scripts/test_psa_constant_names.py +++ b/tests/scripts/test_psa_constant_names.py @@ -306,7 +306,7 @@ def gather_inputs(headers, test_suites, inputs_class=Inputs): return inputs def run_c(type_word, expressions, include_path=None, keep_c=False): - """Generate and run a program to print out numerical values for expressions.""" + """Generate and run a program to print out numerical values of C expressions.""" if type_word == 'status': cast_to = 'long' printf_format = '%ld' From 6d82a7ef9f1b27795712ddfa1c6cdf6716c2f388 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 19 Jan 2021 21:19:25 +0100 Subject: [PATCH 14/19] Say we're running pylint Now that the script might additionally run mypy, it's more user-friendly to indicate what's going on at the beginning as well. Signed-off-by: Gilles Peskine --- tests/scripts/check-python-files.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/scripts/check-python-files.sh b/tests/scripts/check-python-files.sh index 11e351b60..df4821a33 100755 --- a/tests/scripts/check-python-files.sh +++ b/tests/scripts/check-python-files.sh @@ -62,6 +62,7 @@ elif [ "$1" = "--can-mypy" ]; then exit fi +echo 'Running pylint ...' $PYTHON -m pylint -j 2 scripts/mbedtls_dev/*.py scripts/*.py tests/scripts/*.py || { echo >&2 "pylint reported errors" ret=1 From bdde5d002c8780eef9bde5dc78c28b0f90fc2356 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 19 Jan 2021 21:42:05 +0100 Subject: [PATCH 15/19] Use Python to check the version of pylint This reduces dependencies, doesn't require maintainers to know awk, and makes the version parsing more robust. Signed-off-by: Gilles Peskine --- tests/scripts/check-python-files.sh | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/tests/scripts/check-python-files.sh b/tests/scripts/check-python-files.sh index df4821a33..d82ea746c 100755 --- a/tests/scripts/check-python-files.sh +++ b/tests/scripts/check-python-files.sh @@ -28,19 +28,24 @@ else PYTHON=python fi +check_version () { + $PYTHON - "$2" </dev/null --version | awk ' - BEGIN {status = 1} - /^(pylint[0-9]*|__main__\.py) +[0-9]+\.[0-9]+/ { - split($2, version, /[^0-9]+/); - status = !(version[1] >= 2 || (version[1] == 1 && version[2] >= 8)); - exit; # executes the END block - } - END {exit status} - ' + check_version pylint 1.8.3 } can_mypy () { From c3b178768f37be8d834a1b9122ecd8baa05bad32 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 19 Jan 2021 21:43:24 +0100 Subject: [PATCH 16/19] Use can_mypy rather than just checking for mypy As indicated in the comments in the can_mypy function, we don't just need a mypy executable to be present, we need it to work. Signed-off-by: Gilles Peskine --- tests/scripts/check-python-files.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/check-python-files.sh b/tests/scripts/check-python-files.sh index d82ea746c..322e92b50 100755 --- a/tests/scripts/check-python-files.sh +++ b/tests/scripts/check-python-files.sh @@ -74,7 +74,7 @@ $PYTHON -m pylint -j 2 scripts/mbedtls_dev/*.py scripts/*.py tests/scripts/*.py } # Check types if mypy is available -if type mypy >/dev/null 2>/dev/null; then +if can_mypy; then echo echo 'Running mypy ...' mypy scripts/*.py tests/scripts/*.py || From 4738b96d753957ebe65d3e9b7091ee9ed4ba98d9 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 19 Jan 2021 21:45:32 +0100 Subject: [PATCH 17/19] Use $PYTHON when running mypy Make sure to run mypy with the desired Python version. Signed-off-by: Gilles Peskine --- tests/scripts/check-python-files.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/scripts/check-python-files.sh b/tests/scripts/check-python-files.sh index 322e92b50..5796c29b6 100755 --- a/tests/scripts/check-python-files.sh +++ b/tests/scripts/check-python-files.sh @@ -53,7 +53,7 @@ can_mypy () { # minimum version is required. The check is not just "type mypy" # because that passes if a mypy exists but is not installed for the current # python version. - mypy --version 2>/dev/null >/dev/null + $PYTHON -m mypy --version 2>/dev/null >/dev/null } # With just a --can-xxx option, check whether the tool for xxx is available @@ -77,7 +77,7 @@ $PYTHON -m pylint -j 2 scripts/mbedtls_dev/*.py scripts/*.py tests/scripts/*.py if can_mypy; then echo echo 'Running mypy ...' - mypy scripts/*.py tests/scripts/*.py || + $PYTHON -m mypy scripts/*.py tests/scripts/*.py || ret=1 fi From 0370c1710537064c66fbb919c0924a5596bd160c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 19 Jan 2021 21:58:09 +0100 Subject: [PATCH 18/19] mypy: require at least version 0.780 0.780 works. The previous release, 0.770, does not. Signed-off-by: Gilles Peskine --- tests/scripts/check-python-files.sh | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/scripts/check-python-files.sh b/tests/scripts/check-python-files.sh index 5796c29b6..449803a54 100755 --- a/tests/scripts/check-python-files.sh +++ b/tests/scripts/check-python-files.sh @@ -49,11 +49,10 @@ can_pylint () { } can_mypy () { - # Just check that mypy is present and looks sane. I don't know what - # minimum version is required. The check is not just "type mypy" - # because that passes if a mypy exists but is not installed for the current - # python version. - $PYTHON -m mypy --version 2>/dev/null >/dev/null + # mypy 0.770 is too old: + # tests/scripts/test_psa_constant_names.py:34: error: Cannot find implementation or library stub for module named 'mbedtls_dev' + # mypy 0.780 from pip passed on the first commit containing this line. + check_version mypy.version 0.780 } # With just a --can-xxx option, check whether the tool for xxx is available From f71ff1f0add04a6341f57a5d61f13e2e6802b709 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 19 Jan 2021 21:59:06 +0100 Subject: [PATCH 19/19] Run mypy on Travis `tests/scripts/all.sh check_python_files` now runs mypy (in addition to pylint) if it's available. So install mypy. Install mypy 0.780, which is the earliest version that works on our code at this time. Signed-off-by: Gilles Peskine --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 76cb1c537..9b729ec07 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,7 +17,7 @@ jobs: language: python # Needed to get pip for Python 3 python: 3.5 # version from Ubuntu 16.04 install: - - pip install pylint==2.4.4 + - pip install mypy==0.780 pylint==2.4.4 script: - tests/scripts/all.sh -k 'check_*' - tests/scripts/all.sh -k test_default_out_of_box