Merge pull request #6793 from tom-cosgrove-arm/update-mbedtls_mpi_mod_sub-tests-to-match-mod_add-tests
Update mbedtls_mpi_mod_sub() tests to incorporate mod_add test feedback
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 64c1058..519604b 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -304,22 +304,15 @@
# additional convenience targets for Unix only
if(UNIX)
- ADD_CUSTOM_TARGET(covtest
- COMMAND make test
- COMMAND programs/test/selftest
- COMMAND tests/compat.sh
- COMMAND tests/ssl-opt.sh
- )
-
+ # For coverage testing:
+ # 1. Build with:
+ # cmake -D CMAKE_BUILD_TYPE=Coverage /path/to/source && make
+ # 2. Run the relevant tests for the part of the code you're interested in.
+ # For the reference coverage measurement, see
+ # tests/scripts/basic-build-test.sh
+ # 3. Run scripts/lcov.sh to generate an HTML report.
ADD_CUSTOM_TARGET(lcov
- COMMAND rm -rf Coverage
- COMMAND lcov --capture --initial --directory library/CMakeFiles/mbedtls.dir -o files.info
- COMMAND lcov --capture --directory library/CMakeFiles/mbedtls.dir -o tests.info
- COMMAND lcov --add-tracefile files.info --add-tracefile tests.info -o all.info
- COMMAND lcov --remove all.info -o final.info '*.h'
- COMMAND gendesc tests/Descriptions.txt -o descriptions
- COMMAND genhtml --title "mbed TLS" --description-file descriptions --keep-descriptions --legend --no-branch-coverage -o Coverage final.info
- COMMAND rm -f files.info tests.info all.info final.info descriptions
+ COMMAND scripts/lcov.sh
)
ADD_CUSTOM_TARGET(memcheck
diff --git a/Makefile b/Makefile
index 5b2ad16..2f1be65 100644
--- a/Makefile
+++ b/Makefile
@@ -4,7 +4,7 @@
.SILENT:
-.PHONY: all no_test programs lib tests install uninstall clean test check covtest lcov apidoc apidoc_clean
+.PHONY: all no_test programs lib tests install uninstall clean test check lcov apidoc apidoc_clean
all: programs tests
$(MAKE) post_build
@@ -136,23 +136,15 @@
test: check
ifndef WINDOWS
-# note: for coverage testing, build with:
-# make CFLAGS='--coverage -g3 -O0'
-covtest:
- $(MAKE) check
- programs/test/selftest
- tests/compat.sh
- tests/ssl-opt.sh
-
+# For coverage testing:
+# 1. Build with:
+# make CFLAGS='--coverage -g3 -O0' LDFLAGS='--coverage'
+# 2. Run the relevant tests for the part of the code you're interested in.
+# For the reference coverage measurement, see
+# tests/scripts/basic-build-test.sh
+# 3. Run scripts/lcov.sh to generate an HTML report.
lcov:
- rm -rf Coverage
- lcov --capture --initial --directory library -o files.info
- lcov --rc lcov_branch_coverage=1 --capture --directory library -o tests.info
- lcov --rc lcov_branch_coverage=1 --add-tracefile files.info --add-tracefile tests.info -o all.info
- lcov --rc lcov_branch_coverage=1 --remove all.info -o final.info '*.h'
- gendesc tests/Descriptions.txt -o descriptions
- genhtml --title "mbed TLS" --description-file descriptions --keep-descriptions --legend --branch-coverage -o Coverage final.info
- rm -f files.info tests.info all.info final.info descriptions
+ scripts/lcov.sh
apidoc:
mkdir -p apidoc
diff --git a/scripts/lcov.sh b/scripts/lcov.sh
new file mode 100755
index 0000000..8d141ee
--- /dev/null
+++ b/scripts/lcov.sh
@@ -0,0 +1,81 @@
+#!/bin/sh
+
+help () {
+ cat <<EOF
+Usage: $0 [-r]
+Collect coverage statistics of library code into an HTML report.
+
+General instructions:
+1. Build the library with CFLAGS="--coverage -O0 -g3" and link the test
+ programs with LDFLAGS="--coverage".
+ This can be an out-of-tree build.
+ For example (in-tree):
+ make CFLAGS="--coverage -O0 -g3" LDFLAGS="--coverage"
+ Or (out-of-tree):
+ mkdir build-coverage && cd build-coverage &&
+ cmake -D CMAKE_BUILD_TYPE=Coverage .. && make
+2. Run whatever tests you want.
+3. Run this script from the parent of the directory containing the library
+ object files and coverage statistics files.
+4. Browse the coverage report in Coverage/index.html.
+5. After rework, run "$0 -r", then re-test and run "$0" to get a fresh report.
+
+Options
+ -r Reset traces. Run this before re-testing to get fresh measurements.
+EOF
+}
+
+# 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.
+
+set -eu
+
+# Collect stats and build a HTML report.
+lcov_library_report () {
+ rm -rf Coverage
+ mkdir Coverage Coverage/tmp
+ lcov --capture --initial --directory library -o Coverage/tmp/files.info
+ lcov --rc lcov_branch_coverage=1 --capture --directory library -o Coverage/tmp/tests.info
+ lcov --rc lcov_branch_coverage=1 --add-tracefile Coverage/tmp/files.info --add-tracefile Coverage/tmp/tests.info -o Coverage/tmp/all.info
+ lcov --rc lcov_branch_coverage=1 --remove Coverage/tmp/all.info -o Coverage/tmp/final.info '*.h'
+ gendesc tests/Descriptions.txt -o Coverage/tmp/descriptions
+ genhtml --title "mbed TLS" --description-file Coverage/tmp/descriptions --keep-descriptions --legend --branch-coverage -o Coverage Coverage/tmp/final.info
+ rm -f Coverage/tmp/*.info Coverage/tmp/descriptions
+ echo "Coverage report in: Coverage/index.html"
+}
+
+# Reset the traces to 0.
+lcov_reset_traces () {
+ # Location with plain make
+ rm -f library/*.gcda
+ # Location with CMake
+ rm -f library/CMakeFiles/*.dir/*.gcda
+}
+
+if [ $# -gt 0 ] && [ "$1" = "--help" ]; then
+ help
+ exit
+fi
+
+main=lcov_library_report
+while getopts r OPTLET; do
+ case $OPTLET in
+ r) main=lcov_reset_traces;;
+ *) help 2>&1; exit 120;;
+ esac
+done
+shift $((OPTIND - 1))
+
+"$main" "$@"
diff --git a/scripts/mbedtls_dev/bignum_core.py b/scripts/mbedtls_dev/bignum_core.py
index 118a659..158ada9 100644
--- a/scripts/mbedtls_dev/bignum_core.py
+++ b/scripts/mbedtls_dev/bignum_core.py
@@ -130,24 +130,20 @@
class BignumCoreSub(BignumCoreTarget, bignum_common.OperationCommon):
"""Test cases for bignum core sub."""
count = 0
+ input_style = "arch_split"
symbol = "-"
test_function = "mpi_core_sub"
test_name = "mbedtls_mpi_core_sub"
def result(self) -> List[str]:
if self.int_a >= self.int_b:
- result_4 = result_8 = self.int_a - self.int_b
+ result = self.int_a - self.int_b
carry = 0
else:
- bound_val = max(self.int_a, self.int_b)
- bound_4 = bignum_common.bound_mpi(bound_val, 32)
- result_4 = bound_4 + self.int_a - self.int_b
- bound_8 = bignum_common.bound_mpi(bound_val, 64)
- result_8 = bound_8 + self.int_a - self.int_b
+ result = self.limb_boundary + self.int_a - self.int_b
carry = 1
return [
- "\"{:x}\"".format(result_4),
- "\"{:x}\"".format(result_8),
+ self.format_result(result),
str(carry)
]
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 71dd70b..4a7de82 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -243,6 +243,7 @@
if(EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/seedfile")
link_to_source(seedfile)
endif()
+ link_to_source(Descriptions.txt)
link_to_source(compat.sh)
link_to_source(context-info.sh)
link_to_source(data_files)
diff --git a/tests/Makefile b/tests/Makefile
index 2d2d70a..f037338 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -165,6 +165,7 @@
$(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
diff --git a/tests/scripts/generate_test_code.py b/tests/scripts/generate_test_code.py
index 938f24c..f19d30b 100755
--- a/tests/scripts/generate_test_code.py
+++ b/tests/scripts/generate_test_code.py
@@ -220,25 +220,17 @@
: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 @@
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 @@
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 @@
# 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 @@
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 9bf66f1..d23d742 100755
--- a/tests/scripts/test_generate_test_code.py
+++ b/tests/scripts/test_generate_test_code.py
@@ -682,12 +682,12 @@
@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 = ([], '', [])
@@ -727,6 +727,194 @@
'''
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)
+
class ParseFunction(TestCase):
"""
diff --git a/tests/suites/test_suite_bignum_core.function b/tests/suites/test_suite_bignum_core.function
index 7bf03fb..7872115 100644
--- a/tests/suites/test_suite_bignum_core.function
+++ b/tests/suites/test_suite_bignum_core.function
@@ -533,10 +533,9 @@
/* BEGIN_CASE */
void mpi_core_sub( char * input_A, char * input_B,
- char * input_X4, char * input_X8,
- int carry )
+ char * input_X, int carry )
{
- mbedtls_mpi A, B, X4, X8;
+ mbedtls_mpi A, B, X;
mbedtls_mpi_uint *a = NULL;
mbedtls_mpi_uint *b = NULL;
mbedtls_mpi_uint *x = NULL; /* expected */
@@ -544,29 +543,23 @@
mbedtls_mpi_init( &A );
mbedtls_mpi_init( &B );
- mbedtls_mpi_init( &X4 );
- mbedtls_mpi_init( &X8 );
+ mbedtls_mpi_init( &X );
TEST_EQUAL( 0, mbedtls_test_read_mpi( &A, input_A ) );
TEST_EQUAL( 0, mbedtls_test_read_mpi( &B, input_B ) );
- TEST_EQUAL( 0, mbedtls_test_read_mpi( &X4, input_X4 ) );
- TEST_EQUAL( 0, mbedtls_test_read_mpi( &X8, input_X8 ) );
+ TEST_EQUAL( 0, mbedtls_test_read_mpi( &X, input_X ) );
/* All of the inputs are +ve (or zero) */
TEST_EQUAL( 1, A.s );
TEST_EQUAL( 1, B.s );
- TEST_EQUAL( 1, X4.s );
- TEST_EQUAL( 1, X8.s );
+ TEST_EQUAL( 1, X.s );
/* Get the number of limbs we will need */
size_t limbs = MAX( A.n, B.n );
size_t bytes = limbs * sizeof(mbedtls_mpi_uint);
- /* We only need to work with X4 or X8, depending on sizeof(mbedtls_mpi_uint) */
- mbedtls_mpi *X = ( sizeof(mbedtls_mpi_uint) == 4 ) ? &X4 : &X8;
-
/* The result shouldn't have more limbs than the longest input */
- TEST_LE_U( X->n, limbs );
+ TEST_LE_U( X.n, limbs );
/* Now let's get arrays of mbedtls_mpi_uints, rather than MPI structures */
@@ -582,7 +575,7 @@
*/
memcpy( a, A.p, A.n * sizeof(mbedtls_mpi_uint) );
memcpy( b, B.p, B.n * sizeof(mbedtls_mpi_uint) );
- memcpy( x, X->p, X->n * sizeof(mbedtls_mpi_uint) );
+ memcpy( x, X.p, X.n * sizeof(mbedtls_mpi_uint) );
/* 1a) r = a - b => we should get the correct carry */
TEST_EQUAL( carry, mbedtls_mpi_core_sub( r, a, b, limbs ) );
@@ -621,8 +614,7 @@
mbedtls_mpi_free( &A );
mbedtls_mpi_free( &B );
- mbedtls_mpi_free( &X4 );
- mbedtls_mpi_free( &X8 );
+ mbedtls_mpi_free( &X );
}
/* END_CASE */
diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function
index dbbac76..a4c19b8 100644
--- a/tests/suites/test_suite_psa_crypto.function
+++ b/tests/suites/test_suite_psa_crypto.function
@@ -1452,6 +1452,7 @@
/* 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 @@
/* 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 @@
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 @@
/* 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,