Merge pull request #6594 from gilles-peskine-arm/generate_test_code-function_comments

Allow comments in test function prototypes

tests/Makefile

@@ -165,6 +165,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

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
+        This method overrides base class's __next__ method and extends it
-        next method and extends the next method to count the line
+        method to count the line numbers as each line is read.
-        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.
 
         :return: Line read from file.
         """
-        parent = super(FileWrapper, self)
+        line = super().__next__()
-        if hasattr(parent, '__next__'):
-            line = parent.__next__()  # Python 3
-        else:
-            line = parent.next()  # Python 2 # pylint: disable=no-member
         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.
@@ -530,6 +519,50 @@ 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
+            # 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)
+                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:
+            # This errors out if the file ends with an unfinished block
+            # comment. That's acceptable to not complicate the code further.
+            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 and line breaks, this way positions in error
+        # messages remain correct.
+        line = (line[: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):
     """
@@ -549,6 +582,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)
@@ -557,7 +591,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(

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,
+    def test_function_name_on_newline(self, parse_function_arguments_mock,
-                                     gen_function_wrapper_mock,
+                                      gen_function_wrapper_mock,
-                                     gen_dependencies_mock,
+                                      gen_dependencies_mock,
-                                     gen_dispatch_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 = ([], '', [])
@@ -724,6 +724,194 @@ 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)
+
+    @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)
 

tests/suites/test_suite_psa_crypto.function

@@ -1452,6 +1452,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 )
 {
@@ -1508,6 +1509,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;
@@ -1553,6 +1557,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;
@@ -1657,7 +1662,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 lifetime_arg,
                                int export_size_delta,