From d4c9fd1e0a5f2a09d14c06d516716cd2199e90b5 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 31 Aug 2020 10:21:58 +0200 Subject: [PATCH 1/2] Report the first failure, not the last one If test_fail is called multiple times in the same test case, report the location of the first failure, not the last one. With this change, you no longer need to take care in tests that use auxiliary functions not to fail in the main function if the auxiliary function has failed. Signed-off-by: Gilles Peskine --- tests/suites/helpers.function | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/suites/helpers.function b/tests/suites/helpers.function index d2057861f..8f5cef5e0 100644 --- a/tests/suites/helpers.function +++ b/tests/suites/helpers.function @@ -165,6 +165,12 @@ test_info; void test_fail( const char *test, int line_no, const char* filename ) { + if( test_info.failed ) + { + /* We've already recorded the test as having failed. Don't + * overwrite any previous information about the failure. */ + return; + } test_info.failed = 1; test_info.test = test; test_info.line_no = line_no; From cb0ec05717c935e29a3c35a72bbbaf8ba1f51bf8 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 2 Sep 2020 15:18:07 +0200 Subject: [PATCH 2/2] Initialize ret from test code The test function mbedtls_mpi_lt_mpi_ct did not initialize ret in test code. If there was a bug in library code whereby the library function mbedtls_mpi_lt_mpi_ct() did not set ret when it should, we might have missed it if ret happened to contain the expected value. So initialize ret to a value that we never expect. In Mbed TLS 2.7.17, the lack of initialization also caused Valgrind to fail on a Clang 3.8 build with -O1 or more (not with -O0). As far as I can tell, this is an instance of a known bug/feature in Clang which sometimes generates code that contains a conditional jump based on memory which is not initialized at the C level. This is not really a bug in Clang as a C compiler since the code has the same behavior whether the branch is taken or not, and therefore the branch is not observable at the C level. However, the branch on C-uninitialized memory causes a false positive from Valgrind. Here are some reports of this Clang behavior: * https://lists.llvm.org/pipermail/llvm-dev/2016-November/107428.html * https://bugs.llvm.org/show_bug.cgi?id=32604 Signed-off-by: Gilles Peskine --- tests/suites/test_suite_mpi.function | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_mpi.function b/tests/suites/test_suite_mpi.function index 6f5abf3b8..d023466eb 100644 --- a/tests/suites/test_suite_mpi.function +++ b/tests/suites/test_suite_mpi.function @@ -337,7 +337,7 @@ void mbedtls_mpi_lt_mpi_ct( int size_X, char * input_X, int size_Y, char * input_Y, int input_ret, int input_err ) { - unsigned ret; + unsigned ret = -1; unsigned input_uret = input_ret; mbedtls_mpi X, Y; mbedtls_mpi_init( &X ); mbedtls_mpi_init( &Y );