From 5f0057d861b2622acd89540368ac55b3a420c6f9 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 10 Nov 2022 19:33:25 +0100 Subject: [PATCH 01/10] Remove some Python 2 compatibility code Signed-off-by: Gilles Peskine --- tests/scripts/generate_test_code.py | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/tests/scripts/generate_test_code.py b/tests/scripts/generate_test_code.py index 938f24cf4..d994f6bf9 100755 --- a/tests/scripts/generate_test_code.py +++ b/tests/scripts/generate_test_code.py @@ -220,25 +220,17 @@ class FileWrapper(io.FileIO): :param file_name: File path to open. """ - super(FileWrapper, self).__init__(file_name, 'r') + super().__init__(file_name, 'r') self._line_no = 0 - def next(self): + def __next__(self): """ - Python 2 iterator method. This method overrides base class's - next method and extends the next method to count the line - numbers as each line is read. - - It works for both Python 2 and Python 3 by checking iterator - method name in the base iterator object. + This method overrides base class's __next__ method and extends it + method to count the line numbers as each line is read. :return: Line read from file. """ - parent = super(FileWrapper, self) - if hasattr(parent, '__next__'): - line = parent.__next__() # Python 3 - else: - line = parent.next() # Python 2 # pylint: disable=no-member + line = super().__next__() if line is not None: self._line_no += 1 # Convert byte array to string with correct encoding and @@ -246,9 +238,6 @@ class FileWrapper(io.FileIO): return line.decode(sys.getdefaultencoding()).rstrip() + '\n' return None - # Python 3 iterator method - __next__ = next - def get_line_no(self): """ Gives current line number. From 9e509fc3166e08563a22773bff27e7bd32d434b7 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 10 Nov 2022 19:50:34 +0100 Subject: [PATCH 02/10] Add target to generated all .c (and .datax) files Signed-off-by: Gilles Peskine --- tests/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Makefile b/tests/Makefile index 6e232c974..44ff43c67 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -108,6 +108,7 @@ src/drivers/%.o : src/drivers/%.c $(CC) $(LOCAL_CFLAGS) $(CFLAGS) -o $@ -c $< C_FILES := $(addsuffix .c,$(APPS)) +c: $(C_FILES) # Wildcard target for test code generation: # A .c file is generated for each .data file in the suites/ directory. Each .c From 4711731455752528b7ed070480e6c94a8cad7313 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 11 Nov 2022 16:36:51 +0100 Subject: [PATCH 03/10] Fix typo and copypasta Signed-off-by: Gilles Peskine --- tests/scripts/test_generate_test_code.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/scripts/test_generate_test_code.py b/tests/scripts/test_generate_test_code.py index 9bf66f1cc..9796463e4 100755 --- a/tests/scripts/test_generate_test_code.py +++ b/tests/scripts/test_generate_test_code.py @@ -682,12 +682,12 @@ exit: @patch("generate_test_code.gen_dependencies") @patch("generate_test_code.gen_function_wrapper") @patch("generate_test_code.parse_function_arguments") - def test_functio_name_on_newline(self, parse_function_arguments_mock, - gen_function_wrapper_mock, - gen_dependencies_mock, - gen_dispatch_mock): + def test_function_name_on_newline(self, parse_function_arguments_mock, + gen_function_wrapper_mock, + gen_dependencies_mock, + gen_dispatch_mock): """ - Test when exit label is present. + Test with line break before the function name. :return: """ parse_function_arguments_mock.return_value = ([], '', []) From d3ad55e496d43fee3805fffb46987c275e515665 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 11 Nov 2022 16:37:16 +0100 Subject: [PATCH 04/10] Allow comments in prototypes of unit test functions Signed-off-by: Gilles Peskine --- tests/scripts/generate_test_code.py | 38 +++++++- tests/scripts/test_generate_test_code.py | 96 +++++++++++++++++++++ tests/suites/test_suite_psa_crypto.function | 7 +- 3 files changed, 139 insertions(+), 2 deletions(-) diff --git a/tests/scripts/generate_test_code.py b/tests/scripts/generate_test_code.py index d994f6bf9..a1eaa957f 100755 --- a/tests/scripts/generate_test_code.py +++ b/tests/scripts/generate_test_code.py @@ -519,6 +519,41 @@ def generate_function_code(name, code, local_vars, args_dispatch, gen_dependencies(dependencies) return preprocessor_check_start + code + preprocessor_check_end +COMMENT_START_REGEX = re.compile(r'/[*/]') + +def skip_comments(line, stream): + """Remove comments in line. + + If the line contains an unfinished comment, read more lines from stream + until the line that contains the comment. + + :return: The original line with inner comments replaced by spaces. + Trailing comments and whitespace may be removed completely. + """ + pos = 0 + while True: + opening = COMMENT_START_REGEX.search(line, pos) + if not opening: + break + if line[opening.start(0) + 1] == '/': # //... + continuation = line + while continuation.endswith('\\\n'): + # This errors out if the file ends with an unfinished line + # comment. That's acceptable to not complicat the code further. + continuation = next(stream) + return line[:opening.start(0)].rstrip() + '\n' + # Parsing /*...*/, looking for the end + closing = line.find('*/', opening.end(0)) + while closing == -1: + # This errors out if the file ends with an unfinished block + # comment. That's acceptable to not complicat the code further. + line += next(stream) + closing = line.find('*/', opening.end(0)) + pos = closing + 2 + line = (line[:opening.start(0)] + + ' ' * (pos - opening.start(0)) + + line[pos:]) + return re.sub(r' +(\n|\Z)', r'\1', line) def parse_function_code(funcs_f, dependencies, suite_dependencies): """ @@ -538,6 +573,7 @@ def parse_function_code(funcs_f, dependencies, suite_dependencies): # across multiple lines. Here we try to find the start of # arguments list, then remove '\n's and apply the regex to # detect function start. + line = skip_comments(line, funcs_f) up_to_arg_list_start = code + line[:line.find('(') + 1] match = re.match(TEST_FUNCTION_VALIDATION_REGEX, up_to_arg_list_start.replace('\n', ' '), re.I) @@ -546,7 +582,7 @@ def parse_function_code(funcs_f, dependencies, suite_dependencies): name = match.group('func_name') if not re.match(FUNCTION_ARG_LIST_END_REGEX, line): for lin in funcs_f: - line += lin + line += skip_comments(lin, funcs_f) if re.search(FUNCTION_ARG_LIST_END_REGEX, line): break args, local_vars, args_dispatch = parse_function_arguments( diff --git a/tests/scripts/test_generate_test_code.py b/tests/scripts/test_generate_test_code.py index 9796463e4..f36675397 100755 --- a/tests/scripts/test_generate_test_code.py +++ b/tests/scripts/test_generate_test_code.py @@ -724,6 +724,102 @@ exit: yes sir yes sir 3 bags full } +''' + self.assertEqual(code, expected) + + @patch("generate_test_code.gen_dispatch") + @patch("generate_test_code.gen_dependencies") + @patch("generate_test_code.gen_function_wrapper") + @patch("generate_test_code.parse_function_arguments") + def test_case_starting_with_comment(self, parse_function_arguments_mock, + gen_function_wrapper_mock, + gen_dependencies_mock, + gen_dispatch_mock): + """ + Test with comments before the function signature + :return: + """ + parse_function_arguments_mock.return_value = ([], '', []) + gen_function_wrapper_mock.return_value = '' + gen_dependencies_mock.side_effect = gen_dependencies + gen_dispatch_mock.side_effect = gen_dispatch + data = '''/* comment */ +/* more + * comment */ +// this is\ +still \ +a comment +void func() +{ + ba ba black sheep + have you any wool +exit: + yes sir yes sir + 3 bags full +} +/* END_CASE */ +''' + stream = StringIOWrapper('test_suite_ut.function', data) + _, _, code, _ = parse_function_code(stream, [], []) + + expected = '''#line 1 "test_suite_ut.function" + + + +void test_func() +{ + ba ba black sheep + have you any wool +exit: + yes sir yes sir + 3 bags full +} +''' + self.assertEqual(code, expected) + + @patch("generate_test_code.gen_dispatch") + @patch("generate_test_code.gen_dependencies") + @patch("generate_test_code.gen_function_wrapper") + @patch("generate_test_code.parse_function_arguments") + def test_comment_in_prototype(self, parse_function_arguments_mock, + gen_function_wrapper_mock, + gen_dependencies_mock, + gen_dispatch_mock): + """ + Test with comments in the function prototype + :return: + """ + parse_function_arguments_mock.return_value = ([], '', []) + gen_function_wrapper_mock.return_value = '' + gen_dependencies_mock.side_effect = gen_dependencies + gen_dispatch_mock.side_effect = gen_dispatch + data = ''' +void func( int x, // (line \\ + comment) + int y /* lone closing parenthesis) */ ) +{ + ba ba black sheep + have you any wool +exit: + yes sir yes sir + 3 bags full +} +/* END_CASE */ +''' + stream = StringIOWrapper('test_suite_ut.function', data) + _, _, code, _ = parse_function_code(stream, [], []) + + expected = '''#line 1 "test_suite_ut.function" + +void test_func( int x, + int y ) +{ + ba ba black sheep + have you any wool +exit: + yes sir yes sir + 3 bags full +} ''' self.assertEqual(code, expected) diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 0f4e31333..5f489f97c 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -450,6 +450,7 @@ exit: /* END_CASE */ /* BEGIN_CASE */ +/* Construct and attempt to import a large unstructured key. */ void import_large_key( int type_arg, int byte_size_arg, int expected_status_arg ) { @@ -506,6 +507,9 @@ exit: /* END_CASE */ /* BEGIN_CASE depends_on:MBEDTLS_ASN1_WRITE_C */ +/* Import an RSA key with a valid structure (but not valid numbers + * inside, beyond having sensible size and parity). This is expected to + * fail for large keys. */ void import_rsa_made_up( int bits_arg, int keypair, int expected_status_arg ) { mbedtls_svc_key_id_t key = MBEDTLS_SVC_KEY_ID_INIT; @@ -550,6 +554,7 @@ void import_export( data_t *data, int expected_bits, int export_size_delta, int expected_export_status_arg, + /*whether reexport must give the original input exactly*/ int canonical_input ) { mbedtls_svc_key_id_t key = MBEDTLS_SVC_KEY_ID_INIT; @@ -649,7 +654,7 @@ exit: /* BEGIN_CASE */ void import_export_public_key( data_t *data, - int type_arg, + int type_arg, // key pair or public key int alg_arg, int export_size_delta, int expected_export_status_arg, From e54f63e4f37f3cca65d079d68878138c99cc81bb Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 18 Nov 2022 22:24:56 +0100 Subject: [PATCH 05/10] Fix intended backslash in test data Signed-off-by: Gilles Peskine --- tests/scripts/test_generate_test_code.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/scripts/test_generate_test_code.py b/tests/scripts/test_generate_test_code.py index f36675397..ae1aa2af4 100755 --- a/tests/scripts/test_generate_test_code.py +++ b/tests/scripts/test_generate_test_code.py @@ -746,8 +746,8 @@ exit: data = '''/* comment */ /* more * comment */ -// this is\ -still \ +// this is\\ +still \\ a comment void func() { From 8ee3a65f146ee291f060baf819df38f755be1b19 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 18 Nov 2022 22:25:18 +0100 Subject: [PATCH 06/10] Add test cases for comment nesting Add a test case that would fail if all line comments were parsed before block comments, and a test case that would fail if all block comments were parsed before line comments. Signed-off-by: Gilles Peskine --- tests/scripts/test_generate_test_code.py | 88 ++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/tests/scripts/test_generate_test_code.py b/tests/scripts/test_generate_test_code.py index ae1aa2af4..d8b8cf979 100755 --- a/tests/scripts/test_generate_test_code.py +++ b/tests/scripts/test_generate_test_code.py @@ -820,6 +820,94 @@ exit: yes sir yes sir 3 bags full } +''' + self.assertEqual(code, expected) + + @patch("generate_test_code.gen_dispatch") + @patch("generate_test_code.gen_dependencies") + @patch("generate_test_code.gen_function_wrapper") + @patch("generate_test_code.parse_function_arguments") + def test_line_comment_in_block_comment(self, parse_function_arguments_mock, + gen_function_wrapper_mock, + gen_dependencies_mock, + gen_dispatch_mock): + """ + Test with line comment in block comment. + :return: + """ + parse_function_arguments_mock.return_value = ([], '', []) + gen_function_wrapper_mock.return_value = '' + gen_dependencies_mock.side_effect = gen_dependencies + gen_dispatch_mock.side_effect = gen_dispatch + data = ''' +void func( int x /* // */ ) +{ + ba ba black sheep + have you any wool +exit: + yes sir yes sir + 3 bags full +} +/* END_CASE */ +''' + stream = StringIOWrapper('test_suite_ut.function', data) + _, _, code, _ = parse_function_code(stream, [], []) + + expected = '''#line 1 "test_suite_ut.function" + +void test_func( int x ) +{ + ba ba black sheep + have you any wool +exit: + yes sir yes sir + 3 bags full +} +''' + self.assertEqual(code, expected) + + @patch("generate_test_code.gen_dispatch") + @patch("generate_test_code.gen_dependencies") + @patch("generate_test_code.gen_function_wrapper") + @patch("generate_test_code.parse_function_arguments") + def test_block_comment_in_line_comment(self, parse_function_arguments_mock, + gen_function_wrapper_mock, + gen_dependencies_mock, + gen_dispatch_mock): + """ + Test with block comment in line comment. + :return: + """ + parse_function_arguments_mock.return_value = ([], '', []) + gen_function_wrapper_mock.return_value = '' + gen_dependencies_mock.side_effect = gen_dependencies + gen_dispatch_mock.side_effect = gen_dispatch + data = ''' +// /* +void func( int x ) +{ + ba ba black sheep + have you any wool +exit: + yes sir yes sir + 3 bags full +} +/* END_CASE */ +''' + stream = StringIOWrapper('test_suite_ut.function', data) + _, _, code, _ = parse_function_code(stream, [], []) + + expected = '''#line 1 "test_suite_ut.function" + + +void test_func( int x ) +{ + ba ba black sheep + have you any wool +exit: + yes sir yes sir + 3 bags full +} ''' self.assertEqual(code, expected) From 43febf28904aab68f9ffb10c83700e920fa1b319 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 18 Nov 2022 22:26:03 +0100 Subject: [PATCH 07/10] Typos in comments Signed-off-by: Gilles Peskine --- tests/scripts/generate_test_code.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/scripts/generate_test_code.py b/tests/scripts/generate_test_code.py index a1eaa957f..e70fffb93 100755 --- a/tests/scripts/generate_test_code.py +++ b/tests/scripts/generate_test_code.py @@ -539,14 +539,14 @@ def skip_comments(line, stream): continuation = line while continuation.endswith('\\\n'): # This errors out if the file ends with an unfinished line - # comment. That's acceptable to not complicat the code further. + # comment. That's acceptable to not complicate the code further. continuation = next(stream) return line[:opening.start(0)].rstrip() + '\n' # Parsing /*...*/, looking for the end closing = line.find('*/', opening.end(0)) while closing == -1: # This errors out if the file ends with an unfinished block - # comment. That's acceptable to not complicat the code further. + # comment. That's acceptable to not complicate the code further. line += next(stream) closing = line.find('*/', opening.end(0)) pos = closing + 2 From 7e8d4b6afff438e390a04ce2124e191520ae1523 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 18 Nov 2022 22:27:37 +0100 Subject: [PATCH 08/10] Explain space preservation Signed-off-by: Gilles Peskine --- tests/scripts/generate_test_code.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/scripts/generate_test_code.py b/tests/scripts/generate_test_code.py index e70fffb93..c1d1c1cbf 100755 --- a/tests/scripts/generate_test_code.py +++ b/tests/scripts/generate_test_code.py @@ -550,6 +550,13 @@ def skip_comments(line, stream): line += next(stream) closing = line.find('*/', opening.end(0)) pos = closing + 2 + # Replace inner comment by spaces. There needs to be at least one space + # for things like 'int/*ihatespaces*/foo'. Go further and preserve the + # width of the comment, this way column positions in error messages + # remain correct. + # TODO: It would be better to preserve line breaks, to get accurate + # line numbers if there's something interesting after a comment on + # the same line. line = (line[:opening.start(0)] + ' ' * (pos - opening.start(0)) + line[pos:]) From 07995fdd2f7ec29d52264ab3846e6aedd90070a8 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 29 Nov 2022 22:03:32 +0100 Subject: [PATCH 09/10] Preserve line breaks in comments before test functions This way line numbers match better in error messages. Signed-off-by: Gilles Peskine --- tests/scripts/generate_test_code.py | 10 ++++------ tests/scripts/test_generate_test_code.py | 1 + 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/scripts/generate_test_code.py b/tests/scripts/generate_test_code.py index c1d1c1cbf..b7c8e2854 100755 --- a/tests/scripts/generate_test_code.py +++ b/tests/scripts/generate_test_code.py @@ -552,14 +552,12 @@ def skip_comments(line, stream): pos = closing + 2 # Replace inner comment by spaces. There needs to be at least one space # for things like 'int/*ihatespaces*/foo'. Go further and preserve the - # width of the comment, this way column positions in error messages - # remain correct. - # TODO: It would be better to preserve line breaks, to get accurate - # line numbers if there's something interesting after a comment on - # the same line. + # width of the comment and line breaks, this way positions in error + # messages remain correct. line = (line[:opening.start(0)] + - ' ' * (pos - opening.start(0)) + + re.sub(r'.', r' ', line[opening.start(0):pos]) + line[pos:]) + # Strip whitespace at the end of lines (it's irrelevant to error messages). return re.sub(r' +(\n|\Z)', r'\1', line) def parse_function_code(funcs_f, dependencies, suite_dependencies): diff --git a/tests/scripts/test_generate_test_code.py b/tests/scripts/test_generate_test_code.py index d8b8cf979..8b2bf78d8 100755 --- a/tests/scripts/test_generate_test_code.py +++ b/tests/scripts/test_generate_test_code.py @@ -766,6 +766,7 @@ exit: + void test_func() { ba ba black sheep From aec4bec53a836d8d9b05204fcaf42f1ee775f1fb Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 30 Nov 2022 16:38:49 +0100 Subject: [PATCH 10/10] Preserve line breaks from continued line comments The commit "Preserve line breaks in comments before test functions" only handled block comments. This commit handles line comments. Signed-off-by: Gilles Peskine --- tests/scripts/generate_test_code.py | 6 +++++- tests/scripts/test_generate_test_code.py | 3 +++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/scripts/generate_test_code.py b/tests/scripts/generate_test_code.py index b7c8e2854..f19d30b61 100755 --- a/tests/scripts/generate_test_code.py +++ b/tests/scripts/generate_test_code.py @@ -537,11 +537,15 @@ def skip_comments(line, stream): break if line[opening.start(0) + 1] == '/': # //... continuation = line + # Count the number of line breaks, to keep line numbers aligned + # in the output. + line_count = 1 while continuation.endswith('\\\n'): # This errors out if the file ends with an unfinished line # comment. That's acceptable to not complicate the code further. continuation = next(stream) - return line[:opening.start(0)].rstrip() + '\n' + line_count += 1 + return line[:opening.start(0)].rstrip() + '\n' * line_count # Parsing /*...*/, looking for the end closing = line.find('*/', opening.end(0)) while closing == -1: diff --git a/tests/scripts/test_generate_test_code.py b/tests/scripts/test_generate_test_code.py index 8b2bf78d8..d23d74219 100755 --- a/tests/scripts/test_generate_test_code.py +++ b/tests/scripts/test_generate_test_code.py @@ -767,6 +767,8 @@ exit: + + void test_func() { ba ba black sheep @@ -813,6 +815,7 @@ exit: expected = '''#line 1 "test_suite_ut.function" void test_func( int x, + int y ) { ba ba black sheep