From 6542f6c597b8c03a5f40850244976fb2aedc7e5a Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Sun, 9 Dec 2018 22:09:59 +0000 Subject: [PATCH] Change the use of setjmp/longjmp in parameter failure callback Change the use of setjmp and longjmp in signalling parameter validation failures when using the MBEDTLS_CHECK_PARAMS config.h option. This change allows all calls which might result in a call to the parameter validation failure handler to always be caught, even without use of the new macros, by placing a setjmp() in the outer function which calls the test function, which the handler can jump to. This has several benefits: * it allows us to remove the clang compiler warning (-Wclobbered) caused by local auto variables being in the same function as the call to setjmp. * removes the need to wrap all function calls in the test functions with the TEST_ASSERT() macro. Now all parameter validation function calls should be caught. --- tests/suites/helpers.function | 163 +++++++++++++------------------- tests/suites/host_test.function | 1 + tests/suites/main_test.function | 34 ++++++- 3 files changed, 97 insertions(+), 101 deletions(-) diff --git a/tests/suites/helpers.function b/tests/suites/helpers.function index 2d1f6922e..3ae547184 100644 --- a/tests/suites/helpers.function +++ b/tests/suites/helpers.function @@ -27,10 +27,6 @@ #include #define MBEDTLS_PARAM_FAILED(x) mbedtls_param_failed( #x ) -#if defined(__GNUC__) && !defined(__clang__) -#pragma GCC diagnostic ignored "-Wno-uninitialized" -#endif - #endif /* MBEDTLS_CHECK_PARAMS */ #ifdef _MSC_VER @@ -75,12 +71,19 @@ typedef struct data_tag #define DISPATCH_UNSUPPORTED_SUITE -5 /* Test suite not supported by the build */ +typedef enum +{ + PARAMFAIL_TESTSTATE_IDLE = 0, /* No parameter failure call test */ + PARAMFAIL_TESTSTATE_PENDING, /* Test call to the parameter failure + * is pending */ + PARAMFAIL_TESTSTATE_CALLED /* The test call to the parameter + * failure function has been made */ +} paramfail_test_state_t; + /*----------------------------------------------------------------------------*/ /* Macros */ -#if defined(MBEDTLS_CHECK_PARAMS) - /** * \brief This macro tests the expression passed to it as a test step or * individual test in a test case. @@ -96,53 +99,17 @@ typedef struct data_tag * * \param TEST The test expression to be tested. */ -#define TEST_ASSERT( TEST ) \ - do { \ - if ( setjmp( param_fail_jmp ) == 0 ) \ - { \ - if( ! (TEST) ) \ - { \ - test_fail( #TEST, __LINE__, __FILE__ ); \ - goto exit; \ - } \ - } \ - else \ - { \ - test_fail( #TEST, __LINE__, __FILE__ ); \ - goto exit; \ - } \ - memset( param_fail_jmp, 0, sizeof(jmp_buf) ); \ - } while( 0 ) - -/** - * \brief This macro tests and individual function call as a test step or - * individual test in a test case. - * - * It does not require a library function to return a value, and cannot - tets a return error code that can be tested. - * - * When MBEDTLS_CHECK_PARAMS is enabled, calls to the parameter failure - * callback, MBEDTLS_PARAM_FAIL, will be assumed to be a test failure. - * - * This macro is not suitable for negative parameter validation tests - * as it assumes the test step will not create an error. - * - * \param TEST The test statement to be executed. - */ -#define TEST_FN( TEST ) \ - do { \ - if ( setjmp( param_fail_jmp ) == 0 ) \ - { \ - TEST; \ - } \ - else \ - { \ - test_fail( #TEST, __LINE__, __FILE__ ); \ - goto exit; \ - } \ - memset( param_fail_jmp, 0, sizeof(jmp_buf) ); \ + +#define TEST_ASSERT( TEST ) \ + do { \ + if( ! (TEST) ) \ + { \ + test_fail( #TEST, __LINE__, __FILE__ ); \ + goto exit; \ + } \ } while( 0 ) +#if defined(MBEDTLS_CHECK_PARAMS) /** * \brief This macro tests the statement passed to it as a test step or * individual test in a test case. The macro assumes the test will fail @@ -163,18 +130,16 @@ typedef struct data_tag * * \param TEST The test expression to be tested. */ -#define TEST_INVALID_PARAM_RET( PARAM_ERR_VALUE, TEST ) \ - do { \ - if ( setjmp( param_fail_jmp ) == 0 ) \ - { \ - if( (TEST) != PARAM_ERR_VALUE) \ - { \ - test_fail( #TEST, __LINE__, __FILE__ ); \ - goto exit; \ - } \ - } \ - memset( param_fail_jmp, 0, sizeof(jmp_buf) ); \ - } while( 0 ) +#define TEST_INVALID_PARAM_RET( PARAM_ERR_VALUE, TEST ) \ + do { \ + test_info.paramfail_test_state = PARAMFAIL_TESTSTATE_PENDING; \ + if( (TEST) != (PARAM_ERR_VALUE) && \ + test_info.paramfail_test_state != PARAMFAIL_TESTSTATE_CALLED ) \ + { \ + test_fail( #TEST, __LINE__, __FILE__ ); \ + goto exit; \ + } \ + } while( 0 ) /** * \brief This macro tests the statement passed to it as a test step or @@ -196,33 +161,20 @@ typedef struct data_tag * * \param TEST The test expression to be tested. */ -#define TEST_INVALID_PARAM( TEST ) \ - do { \ - if ( setjmp( param_fail_jmp ) == 0 ) \ - { \ - TEST; \ - test_fail( #TEST, __LINE__, __FILE__ ); \ - goto exit; \ - } \ - memset( param_fail_jmp, 0, sizeof(jmp_buf) ); \ +#define TEST_INVALID_PARAM( TEST ) \ + do { \ + memcpy(jmp_tmp, param_fail_jmp, sizeof(jmp_buf)); \ + if ( setjmp( param_fail_jmp ) == 0 ) \ + { \ + TEST; \ + test_fail( #TEST, __LINE__, __FILE__ ); \ + goto exit; \ + } \ + memcpy(param_fail_jmp, jmp_tmp, sizeof(jmp_buf)); \ } while( 0 ) #else -#define TEST_ASSERT( TEST ) \ - do { \ - if( ! (TEST) ) \ - { \ - test_fail( #TEST, __LINE__, __FILE__ ); \ - goto exit; \ - } \ - } while( 0 ) - -#define TEST_FN( TEST ) \ - do { \ - TEST; \ - } while( 0 ) - #define TEST_INVALID_PARAM_RET( PARAM_ERR_VALUE, TEST ) \ do { \ if( (TEST) != (PARAM_ERR_VALUE) ) \ @@ -273,9 +225,9 @@ typedef struct data_tag /*----------------------------------------------------------------------------*/ /* Global variables */ - static struct { + paramfail_test_state_t paramfail_test_state; int failed; const char *test; const char *filename; @@ -289,6 +241,7 @@ mbedtls_platform_context platform_ctx; #if defined(MBEDTLS_CHECK_PARAMS) jmp_buf param_fail_jmp; +jmp_buf jmp_tmp; #endif /*----------------------------------------------------------------------------*/ @@ -308,6 +261,15 @@ jmp_buf param_fail_jmp; /*----------------------------------------------------------------------------*/ /* Helper Functions */ + +static void test_fail( const char *test, int line_no, const char* filename ) +{ + test_info.failed = 1; + test_info.test = test; + test_info.line_no = line_no; + test_info.filename = filename; +} + static int platform_setup() { int ret = 0; @@ -327,11 +289,22 @@ static void platform_teardown() #if defined(MBEDTLS_CHECK_PARAMS) void mbedtls_param_failed( char* failure_condition, char* file, int line ) { - (void)failure_condition; - (void)file; - (void)line; + /* If we are testing the callback function... */ + if ( test_info.paramfail_test_state == PARAMFAIL_TESTSTATE_PENDING ) + { + test_info.paramfail_test_state = PARAMFAIL_TESTSTATE_CALLED; + } + else + { + /* ...else we treat this as an error */ - longjmp( param_fail_jmp, 1 ); + /* Record the location of the failure, but not as a failure yet, in case + * it was part of the test */ + test_fail( failure_condition, line, file ); + test_info.failed = 0; + + longjmp( param_fail_jmp, 1 ); + } } #endif @@ -623,14 +596,6 @@ static int rnd_pseudo_rand( void *rng_state, unsigned char *output, size_t len ) return( 0 ); } -static void test_fail( const char *test, int line_no, const char* filename ) -{ - test_info.failed = 1; - test_info.test = test; - test_info.line_no = line_no; - test_info.filename = filename; -} - int hexcmp( uint8_t * a, uint8_t * b, uint32_t a_len, uint32_t b_len ) { int ret = 0; diff --git a/tests/suites/host_test.function b/tests/suites/host_test.function index b354af473..3c4303208 100644 --- a/tests/suites/host_test.function +++ b/tests/suites/host_test.function @@ -546,6 +546,7 @@ int execute_tests( int argc , const char ** argv ) if( unmet_dep_count == 0 ) { test_info.failed = 0; + test_info.paramfail_test_state = PARAMFAIL_TESTSTATE_IDLE; #if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__)) /* Suppress all output from the library unless we're verbose diff --git a/tests/suites/main_test.function b/tests/suites/main_test.function index 2ba919ce0..ca4783dcf 100644 --- a/tests/suites/main_test.function +++ b/tests/suites/main_test.function @@ -134,9 +134,39 @@ $dispatch_code #line $line_no "suites/main_test.function" }; +/** + * \brief Execute the test function. + * + * This is a wrapper function around the test function execution + * to allow the setjmp() call used to catch any calls to the + * parameter failure callback, to be used. Calls to setjmp() + * can invalidate the state of any local auto variables. + * + * \param fp Function pointer to the test function + * \param params Parameters to pass + * + */ +void execute_function_ptr(TestWrapper_t fp, void **params) +{ +#if defined(MBEDTLS_CHECK_PARAMS) + if ( setjmp( param_fail_jmp ) == 0 ) + { + fp( params ); + } + else + { + /* Unexpected parameter validation error */ + test_info.failed = 1; + } + + memset( param_fail_jmp, 0, sizeof(jmp_buf) ); +#else + fp( params ); +#endif +} /** - * \brief Dispatches test functions based on function index. + * \brief Dispatches test functions based on function index. * * \param exp_id Test function index. * @@ -153,7 +183,7 @@ int dispatch_test( int func_idx, void ** params ) { fp = test_funcs[func_idx]; if ( fp ) - fp( params ); + execute_function_ptr(fp, params); else ret = DISPATCH_UNSUPPORTED_SUITE; }