From 62cf2e8e9f62db18514da3392a04cd842515990b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 27 Mar 2020 16:35:23 +0100 Subject: [PATCH 01/27] Switch all.sh to bash This will let us use bash features that are not found in some other sh implementations, such as DEBUG and ERR traps, "set -o pipefail", etc. Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index f8e43c8714..b6f39e96fa 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1,4 +1,4 @@ -#! /usr/bin/env sh +#! /usr/bin/env bash # all.sh # @@ -175,8 +175,8 @@ pre_initialize_variables () { # Gather the list of available components. These are the functions # defined in this script whose name starts with "component_". - # Parse the script with sed, because in sh there is no way to list - # defined functions. + # Parse the script with sed. This way we get the functions in the order + # they are defined. ALL_COMPONENTS=$(sed -n 's/^ *component_\([0-9A-Z_a-z]*\) *().*/\1/p' <"$0") # Exclude components that are not supported on this platform. From 5d99682a8c701cff581ee3f33e4e7fbb541010f3 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 28 Mar 2020 21:09:21 +0100 Subject: [PATCH 02/27] Add --error-test option to test error detection and reporting Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index b6f39e96fa..20a20a3151 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -231,6 +231,8 @@ General options: Prefix for a cross-compiler for arm-none-eabi (default: "${ARM_NONE_EABI_GCC_PREFIX}") --armcc Run ARM Compiler builds (on by default). + --error-test Error test mode: run a failing function in addition + to any specified component. --except Exclude the COMPONENTs listed on the command line, instead of running only those. --no-append-outcome Write a new outcome file and analyze it (default). @@ -378,6 +380,7 @@ check_headers_in_cpp () { pre_parse_command_line () { COMMAND_LINE_COMPONENTS= all_except=0 + error_test=0 no_armcc= # Note that legacy options are ignored instead of being omitted from this @@ -390,6 +393,7 @@ pre_parse_command_line () { --armcc) no_armcc=;; --armc5-bin-dir) shift; ARMC5_BIN_DIR="$1";; --armc6-bin-dir) shift; ARMC6_BIN_DIR="$1";; + --error-test) error_test=$((error_test + 1));; --except) all_except=1;; --force|-f) FORCE=1;; --gnutls-cli) shift; GNUTLS_CLI="$1";; @@ -2636,6 +2640,19 @@ post_report () { #### Run all the things ################################################################ +# Function invoked by --error-test to test error reporting. +pseudo_component_error_test () { + msg "Testing error reporting $error_test" + if [ $KEEP_GOING -ne 0 ]; then + echo "Expect three failing commands." + fi + error_test='this should not be used since the component runs in a subshell' + grep non_existent /dev/null + not grep -q . "$0" + make unknown_target + false "this should not be executed" +} + # Run one component and clean up afterwards. run_component () { # Back up the configuration in case the component modifies it. @@ -2685,6 +2702,10 @@ cleanup pre_generate_files # Run the requested tests. +while [ $error_test -gt 0 ]; do + run_component pseudo_component_error_test + error_test=$((error_test - 1)) +done for component in $RUN_COMPONENTS; do run_component "component_$component" done From ce266c48bb8214a061c906392b8fbcdf53ac6617 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 28 Mar 2020 18:50:43 +0100 Subject: [PATCH 03/27] Run each component in a subshell and handle errors more robustly This commit completely rewrites keep-going mode. Instead of relying solely on "set -e", which has some subtle limitations (such as being off anywhere inside a conditional), use an ERR trap to record errors. Run each component in a subshell. This way a component can set environment variables, change the current directory, etc., without affecting other components. Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 151 ++++++++++++++++++++++++++----------------- 1 file changed, 93 insertions(+), 58 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 20a20a3151..f3494bdd97 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -59,6 +59,15 @@ # This script must be invoked from the toplevel directory of a git # working copy of Mbed TLS. # +# The behavior on an error depends on whether --keep-going (alias -k) +# is in effect. +# * Without --keep-going: the script stops on the first error without +# cleaning up. This lets you work in the configuration of the failing +# component. +# * With --keep-going: the script runs all requested components and +# reports failures at the end. In particular the script always cleans +# up on exit. +# # Note that the output is not saved. You may want to run # script -c tests/scripts/all.sh # or @@ -81,6 +90,9 @@ # # Each component must start by invoking `msg` with a short informative message. # +# Each component is executed in a separate shell process. The component +# fails if any command in it returns a non-zero status. +# # The framework performs some cleanup tasks after each component. This # means that components can assume that the working directory is in a # cleaned-up state, and don't need to perform the cleanup themselves. @@ -91,19 +103,6 @@ # `tests/Makefile` and `programs/fuzz/Makefile` from git. # This cleans up after an in-tree use of CMake. # -# Any command that is expected to fail must be protected so that the -# script keeps running in --keep-going mode despite `set -e`. In keep-going -# mode, if a protected command fails, this is logged as a failure and the -# script will exit with a failure status once it has run all components. -# Commands can be protected in any of the following ways: -# * `make` is a function which runs the `make` command with protection. -# Note that you must write `make VAR=value`, not `VAR=value make`, -# because the `VAR=value make` syntax doesn't work with functions. -# * Put `report_status` before the command to protect it. -# * Put `if_build_successful` before a command. This protects it, and -# additionally skips it if a prior invocation of `make` in the same -# component failed. -# # The tests are roughly in order from fastest to slowest. This doesn't # have to be exact, but in general you should add slower tests towards # the end and fast checks near the beginning. @@ -477,8 +476,9 @@ pre_check_git () { } pre_setup_keep_going () { - failure_summary= - failure_count=0 + failure_count=0 # Number of failed components + last_failure_status=0 # Last failure status in this component + start_red= end_color= if [ -t 1 ]; then @@ -489,57 +489,73 @@ pre_setup_keep_going () { ;; esac fi - record_status () { - if "$@"; then - last_status=0 - else - last_status=$? - text="$current_section: $* -> $last_status" - failure_summary="$failure_summary -$text" - failure_count=$((failure_count + 1)) - echo "${start_red}^^^^$text^^^^${end_color}" >&2 - fi - } - make () { - case "$*" in - *test|*check) - if [ $build_status -eq 0 ]; then - record_status command make "$@" - else - echo "(skipped because the build failed)" - fi - ;; - *) - record_status command make "$@" - build_status=$last_status - ;; + + # Keep a summary of failures in a file. We'll print it out at the end. + failure_summary_file=$PWD/all-sh-failures-$$.log + : >"$failure_summary_file" + + # Whether it makes sense to keep a component going after the specified + # command fails (test command) or not (configure or build). + # This doesn't have to be 100% accurate: all failures are recorded anyway. + can_keep_going_after_failure () { + case "$1" in + "msg "*) false;; + *[!A-Za-z]"test"|*[!A-Za-z]"test"[!A-Za-z]*) true;; + "tests/"*) true;; + "grep "*|"not grep "*) true;; + *) false;; esac } + + # This function runs if there is any error in a component. + # It must either exit with a nonzero status, or set + # last_failure_status to a nonzero value. + err_trap () { + # Save $? (status of the failing command). This must be the very + # first thing, before $? is overridden. + last_failure_status=$? + failed_command=$BASH_COMMAND + + text="$current_section: $failed_command -> $last_failure_status" + echo "${start_red}^^^^$text^^^^${end_color}" >&2 + echo "$text" >>"$failure_summary_file" + + # If the command is fatal (configure or build command), stop this + # component. Otherwise (test command) keep the component running + # (run more tests from the same build). + if ! can_keep_going_after_failure "$failed_command"; then + exit $last_failure_status + fi + } + final_report () { if [ $failure_count -gt 0 ]; then echo echo "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!" - echo "${start_red}FAILED: $failure_count${end_color}$failure_summary" + echo "${start_red}FAILED: $failure_count components${end_color}" + cat "$failure_summary_file" echo "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!" - exit 1 elif [ -z "${1-}" ]; then echo "SUCCESS :)" fi if [ -n "${1-}" ]; then echo "Killed by SIG$1." fi + rm -f "$failure_summary_file" + if [ $failure_count -gt 0 ]; then + exit 1 + fi } } -if_build_succeeded () { - if [ $build_status -eq 0 ]; then - record_status "$@" - fi +# These functions are kept temporarily for backward compatibility. +# Don't use them in new components. +record_status () { + "$@" +} +if_build_succeeded () { + "$@" } - -# to be used instead of ! for commands run with -# record_status or if_build_succeeded not() { ! "$@" } @@ -2667,12 +2683,35 @@ run_component () { # have messed it up or shortened it. redirect_err dd if=/dev/urandom of=./tests/seedfile bs=64 count=1 - # Run the component code. - if [ $QUIET -eq 1 ]; then - # msg() is silenced, so just print the component name here - echo "${current_component#component_}" + # Run the component in a subshell + if [ $KEEP_GOING -eq 1 ]; then + set +e + fi + ( + if [ $QUIET -eq 1 ]; then + # msg() will be silenced, so just print the component name here. + echo "${current_component#component_}" + exec >/dev/null + fi + if [ $KEEP_GOING -eq 1 ]; then + # Keep "set -e" off, and run an ERR trap instead to record failures. + set -E + trap err_trap ERR + fi + # The next line is what runs the component + "$@" + if [ $KEEP_GOING -eq 1 ]; then + trap - ERR + exit $last_failure_status + fi + ) + component_status=$? + if [ $KEEP_GOING -eq 1 ]; then + set -e + if [ $component_status -ne 0 ]; then + failure_count=$((failure_count + 1)) + fi fi - redirect_out "$@" # Restore the build tree to a clean state. cleanup @@ -2689,10 +2728,6 @@ pre_check_git build_status=0 if [ $KEEP_GOING -eq 1 ]; then pre_setup_keep_going -else - record_status () { - "$@" - } fi pre_setup_quiet_redirect pre_prepare_outcome_file From 3664780f98357b211fe6fafb4fb090c6fe630c84 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 28 Mar 2020 18:50:49 +0100 Subject: [PATCH 04/27] Detect errors on the left-hand side of a pipeline Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index f3494bdd97..f80b8ff1ea 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -113,8 +113,9 @@ #### Initialization and command line parsing ################################################################ -# Abort on errors (and uninitialised variables) -set -eu +# Abort on errors (even on the left-hand side of a pipe). +# Treat uninitialised variables as errors. +set -e -o pipefail -u pre_check_environment () { if [ -d library -a -d include -a -d tests ]; then :; else From f7e956c85c5204d253db043128ea4260040e158d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 28 Mar 2020 18:56:09 +0100 Subject: [PATCH 05/27] component_test_cmake_out_of_source: simplify and fix error handling Remove ssl-opt.err even if it's empty. Call cat unconditionally: it'll have no visible effect if the file is empty. Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index f80b8ff1ea..5a0ae4b0fa 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -2548,11 +2548,10 @@ component_test_cmake_out_of_source () { # file is missing (ssl-opt.sh tolerates the absence of some files so # may exit with status 0 but emit errors). if_build_succeeded ./tests/ssl-opt.sh -f 'Fallback SCSV: beginning of list' 2>ssl-opt.err - if [ -s ssl-opt.err ]; then - cat ssl-opt.err >&2 - record_status [ ! -s ssl-opt.err ] - rm ssl-opt.err - fi + cat ssl-opt.err >&2 + # If ssl-opt.err is non-empty, record an error and keep going. + record_status [ ! -s ssl-opt.err ] + rm ssl-opt.err cd "$MBEDTLS_ROOT_DIR" rm -rf "$OUT_OF_SOURCE_DIR" unset MBEDTLS_ROOT_DIR From 1f0cdaf3af3dbedeb0ae9dd7094df375472d129f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 8 Jul 2021 18:41:16 +0200 Subject: [PATCH 06/27] Stop dispatching through obsolete functions Remove the obsolete functions record_status and if_build_succeeded. They didn't affect error detection, but they made error reporting worse since $BASH_COMMAND would be the unexpanded "$@". Keep the function definitions for the sake of pull requests using them that may still be in flight. Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 209 ++++++++++++++++++++++--------------------- 1 file changed, 105 insertions(+), 104 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 5a0ae4b0fa..8b8e3dd590 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -549,14 +549,15 @@ pre_setup_keep_going () { } } -# These functions are kept temporarily for backward compatibility. -# Don't use them in new components. +# record_status() and if_build_succeeded() are kept temporarily for backward +# compatibility. Don't use them in new components. record_status () { "$@" } if_build_succeeded () { "$@" } + not() { ! "$@" } @@ -707,24 +708,24 @@ pre_generate_files() { component_check_recursion () { msg "Check: recursion.pl" # < 1s - record_status tests/scripts/recursion.pl library/*.c + tests/scripts/recursion.pl library/*.c } component_check_generated_files () { msg "Check: check-generated-files, files generated with make" # 2s make generated_files - record_status tests/scripts/check-generated-files.sh + tests/scripts/check-generated-files.sh msg "Check: check-generated-files -u, files present" # 2s - record_status tests/scripts/check-generated-files.sh -u + tests/scripts/check-generated-files.sh -u # Check that the generated files are considered up to date. - record_status tests/scripts/check-generated-files.sh + tests/scripts/check-generated-files.sh msg "Check: check-generated-files -u, files absent" # 2s command make neat - record_status tests/scripts/check-generated-files.sh -u + tests/scripts/check-generated-files.sh -u # Check that the generated files are considered up to date. - record_status tests/scripts/check-generated-files.sh + tests/scripts/check-generated-files.sh # This component ends with the generated files present in the source tree. # This is necessary for subsequent components! @@ -732,18 +733,18 @@ component_check_generated_files () { component_check_doxy_blocks () { msg "Check: doxygen markup outside doxygen blocks" # < 1s - record_status tests/scripts/check-doxy-blocks.pl + tests/scripts/check-doxy-blocks.pl } component_check_files () { msg "Check: file sanity checks (permissions, encodings)" # < 1s - record_status tests/scripts/check_files.py + tests/scripts/check_files.py } component_check_changelog () { msg "Check: changelog entries" # < 1s rm -f ChangeLog.new - record_status scripts/assemble_changelog.py -o ChangeLog.new + scripts/assemble_changelog.py -o ChangeLog.new if [ -e ChangeLog.new ]; then # Show the diff for information. It isn't an error if the diff is # non-empty. @@ -754,7 +755,7 @@ component_check_changelog () { component_check_names () { msg "Check: declared and exported names (builds the library)" # < 3s - record_status tests/scripts/check-names.sh -v + tests/scripts/check-names.sh -v } component_check_test_cases () { @@ -764,13 +765,13 @@ component_check_test_cases () { else opt='' fi - record_status tests/scripts/check_test_cases.py $opt + tests/scripts/check_test_cases.py $opt unset opt } component_check_doxygen_warnings () { msg "Check: doxygen warnings (builds the documentation)" # ~ 3s - record_status tests/scripts/doxygen.sh + tests/scripts/doxygen.sh } @@ -790,7 +791,7 @@ component_test_default_out_of_box () { make test msg "selftest: make, default config (out-of-box)" # ~10s - if_build_succeeded programs/test/selftest + programs/test/selftest export MBEDTLS_TEST_OUTCOME_FILE="$SAVE_MBEDTLS_TEST_OUTCOME_FILE" unset SAVE_MBEDTLS_TEST_OUTCOME_FILE @@ -805,16 +806,16 @@ component_test_default_cmake_gcc_asan () { make test msg "test: selftest (ASan build)" # ~ 10s - if_build_succeeded programs/test/selftest + programs/test/selftest msg "test: ssl-opt.sh (ASan build)" # ~ 1 min - if_build_succeeded tests/ssl-opt.sh + tests/ssl-opt.sh msg "test: compat.sh (ASan build)" # ~ 6 min - if_build_succeeded tests/compat.sh + tests/compat.sh msg "test: context-info.sh (ASan build)" # ~ 15 sec - if_build_succeeded tests/context-info.sh + tests/context-info.sh } component_test_full_cmake_gcc_asan () { @@ -827,16 +828,16 @@ component_test_full_cmake_gcc_asan () { make test msg "test: selftest (ASan build)" # ~ 10s - if_build_succeeded programs/test/selftest + programs/test/selftest msg "test: ssl-opt.sh (full config, ASan build)" - if_build_succeeded tests/ssl-opt.sh + tests/ssl-opt.sh msg "test: compat.sh (full config, ASan build)" - if_build_succeeded tests/compat.sh + tests/compat.sh msg "test: context-info.sh (full config, ASan build)" # ~ 15 sec - if_build_succeeded tests/context-info.sh + tests/context-info.sh } component_test_psa_crypto_key_id_encodes_owner () { @@ -874,7 +875,7 @@ component_build_psa_crypto_spm () { # Check that if a symbol is renamed by crypto_spe.h, the non-renamed # version is not present. echo "Checking for renamed symbols in the library" - if_build_succeeded check_renamed_symbols tests/include/spe/crypto_spe.h library/libmbedcrypto.a + check_renamed_symbols tests/include/spe/crypto_spe.h library/libmbedcrypto.a } component_test_psa_crypto_client () { @@ -900,7 +901,7 @@ component_test_psa_crypto_rsa_no_genprime() { component_test_ref_configs () { msg "test/build: ref-configs (ASan build)" # ~ 6 min 20s CC=gcc cmake -D CMAKE_BUILD_TYPE:String=Asan . - record_status tests/scripts/test-ref-configs.pl + tests/scripts/test-ref-configs.pl } component_test_no_renegotiation () { @@ -913,7 +914,7 @@ component_test_no_renegotiation () { make test msg "test: !MBEDTLS_SSL_RENEGOTIATION - ssl-opt.sh (ASan build)" # ~ 6 min - if_build_succeeded tests/ssl-opt.sh + tests/ssl-opt.sh } component_test_no_pem_no_fs () { @@ -929,7 +930,7 @@ component_test_no_pem_no_fs () { make test msg "test: !MBEDTLS_PEM_PARSE_C !MBEDTLS_FS_IO - ssl-opt.sh (ASan build)" # ~ 6 min - if_build_succeeded tests/ssl-opt.sh + tests/ssl-opt.sh } component_test_rsa_no_crt () { @@ -942,13 +943,13 @@ component_test_rsa_no_crt () { make test msg "test: RSA_NO_CRT - RSA-related part of ssl-opt.sh (ASan build)" # ~ 5s - if_build_succeeded tests/ssl-opt.sh -f RSA + tests/ssl-opt.sh -f RSA msg "test: RSA_NO_CRT - RSA-related part of compat.sh (ASan build)" # ~ 3 min - if_build_succeeded tests/compat.sh -t RSA + tests/compat.sh -t RSA msg "test: RSA_NO_CRT - RSA-related part of context-info.sh (ASan build)" # ~ 15 sec - if_build_succeeded tests/context-info.sh + tests/context-info.sh } component_test_no_ctr_drbg_classic () { @@ -967,10 +968,10 @@ component_test_no_ctr_drbg_classic () { # The SSL tests are slow, so run a small subset, just enough to get # confidence that the SSL code copes with HMAC_DRBG. msg "test: Full minus CTR_DRBG, classic crypto - ssl-opt.sh (subset)" - if_build_succeeded tests/ssl-opt.sh -f 'Default\|SSL async private.*delay=\|tickets enabled on server' + tests/ssl-opt.sh -f 'Default\|SSL async private.*delay=\|tickets enabled on server' msg "test: Full minus CTR_DRBG, classic crypto - compat.sh (subset)" - if_build_succeeded tests/compat.sh -m tls1_2 -t 'ECDSA PSK' -V NO -p OpenSSL + tests/compat.sh -m tls1_2 -t 'ECDSA PSK' -V NO -p OpenSSL } component_test_no_ctr_drbg_use_psa () { @@ -989,10 +990,10 @@ component_test_no_ctr_drbg_use_psa () { # The SSL tests are slow, so run a small subset, just enough to get # confidence that the SSL code copes with HMAC_DRBG. msg "test: Full minus CTR_DRBG, USE_PSA_CRYPTO - ssl-opt.sh (subset)" - if_build_succeeded tests/ssl-opt.sh -f 'Default\|SSL async private.*delay=\|tickets enabled on server' + tests/ssl-opt.sh -f 'Default\|SSL async private.*delay=\|tickets enabled on server' msg "test: Full minus CTR_DRBG, USE_PSA_CRYPTO - compat.sh (subset)" - if_build_succeeded tests/compat.sh -m tls1_2 -t 'ECDSA PSK' -V NO -p OpenSSL + tests/compat.sh -m tls1_2 -t 'ECDSA PSK' -V NO -p OpenSSL } component_test_no_hmac_drbg_classic () { @@ -1014,12 +1015,12 @@ component_test_no_hmac_drbg_classic () { # Test SSL with non-deterministic ECDSA. Only test features that # might be affected by how ECDSA signature is performed. msg "test: Full minus HMAC_DRBG, classic crypto - ssl-opt.sh (subset)" - if_build_succeeded tests/ssl-opt.sh -f 'Default\|SSL async private: sign' + tests/ssl-opt.sh -f 'Default\|SSL async private: sign' # To save time, only test one protocol version, since this part of # the protocol is identical in (D)TLS up to 1.2. msg "test: Full minus HMAC_DRBG, classic crypto - compat.sh (ECDSA)" - if_build_succeeded tests/compat.sh -m tls1_2 -t 'ECDSA' + tests/compat.sh -m tls1_2 -t 'ECDSA' } component_test_no_hmac_drbg_use_psa () { @@ -1041,12 +1042,12 @@ component_test_no_hmac_drbg_use_psa () { # Test SSL with non-deterministic ECDSA. Only test features that # might be affected by how ECDSA signature is performed. msg "test: Full minus HMAC_DRBG, USE_PSA_CRYPTO - ssl-opt.sh (subset)" - if_build_succeeded tests/ssl-opt.sh -f 'Default\|SSL async private: sign' + tests/ssl-opt.sh -f 'Default\|SSL async private: sign' # To save time, only test one protocol version, since this part of # the protocol is identical in (D)TLS up to 1.2. msg "test: Full minus HMAC_DRBG, USE_PSA_CRYPTO - compat.sh (ECDSA)" - if_build_succeeded tests/compat.sh -m tls1_2 -t 'ECDSA' + tests/compat.sh -m tls1_2 -t 'ECDSA' } component_test_psa_external_rng_no_drbg_classic () { @@ -1069,7 +1070,7 @@ component_test_psa_external_rng_no_drbg_classic () { make test msg "test: PSA_CRYPTO_EXTERNAL_RNG minus *_DRBG, classic crypto - ssl-opt.sh (subset)" - if_build_succeeded tests/ssl-opt.sh -f 'Default' + tests/ssl-opt.sh -f 'Default' } component_test_psa_external_rng_no_drbg_use_psa () { @@ -1088,7 +1089,7 @@ component_test_psa_external_rng_no_drbg_use_psa () { make test msg "test: PSA_CRYPTO_EXTERNAL_RNG minus *_DRBG, PSA crypto - ssl-opt.sh (subset)" - if_build_succeeded tests/ssl-opt.sh -f 'Default\|opaque' + tests/ssl-opt.sh -f 'Default\|opaque' } component_test_psa_external_rng_use_psa_crypto () { @@ -1103,7 +1104,7 @@ component_test_psa_external_rng_use_psa_crypto () { make test msg "test: full + PSA_CRYPTO_EXTERNAL_RNG + USE_PSA_CRYPTO minus CTR_DRBG" - if_build_succeeded tests/ssl-opt.sh -f 'Default\|opaque' + tests/ssl-opt.sh -f 'Default\|opaque' } component_test_everest () { @@ -1116,11 +1117,11 @@ component_test_everest () { make test msg "test: Everest ECDH context - ECDH-related part of ssl-opt.sh (ASan build)" # ~ 5s - if_build_succeeded tests/ssl-opt.sh -f ECDH + tests/ssl-opt.sh -f ECDH msg "test: Everest ECDH context - compat.sh with some ECDH ciphersuites (ASan build)" # ~ 3 min # Exclude some symmetric ciphers that are redundant here to gain time. - if_build_succeeded tests/compat.sh -f ECDH -V NO -e 'ARIA\|CAMELLIA\|CHACHA\|DES' + tests/compat.sh -f ECDH -V NO -e 'ARIA\|CAMELLIA\|CHACHA\|DES' } component_test_everest_curve25519_only () { @@ -1150,7 +1151,7 @@ component_test_small_ssl_out_content_len () { make msg "test: small SSL_OUT_CONTENT_LEN - ssl-opt.sh MFL and large packet tests" - if_build_succeeded tests/ssl-opt.sh -f "Max fragment\|Large packet" + tests/ssl-opt.sh -f "Max fragment\|Large packet" } component_test_small_ssl_in_content_len () { @@ -1161,7 +1162,7 @@ component_test_small_ssl_in_content_len () { make msg "test: small SSL_IN_CONTENT_LEN - ssl-opt.sh MFL tests" - if_build_succeeded tests/ssl-opt.sh -f "Max fragment" + tests/ssl-opt.sh -f "Max fragment" } component_test_small_ssl_dtls_max_buffering () { @@ -1171,7 +1172,7 @@ component_test_small_ssl_dtls_max_buffering () { make msg "test: small MBEDTLS_SSL_DTLS_MAX_BUFFERING #0 - ssl-opt.sh specific reordering test" - if_build_succeeded tests/ssl-opt.sh -f "DTLS reordering: Buffer out-of-order hs msg before reassembling next, free buffered msg" + tests/ssl-opt.sh -f "DTLS reordering: Buffer out-of-order hs msg before reassembling next, free buffered msg" } component_test_small_mbedtls_ssl_dtls_max_buffering () { @@ -1181,15 +1182,15 @@ component_test_small_mbedtls_ssl_dtls_max_buffering () { make msg "test: small MBEDTLS_SSL_DTLS_MAX_BUFFERING #1 - ssl-opt.sh specific reordering test" - if_build_succeeded tests/ssl-opt.sh -f "DTLS reordering: Buffer encrypted Finished message, drop for fragmented NewSessionTicket" + tests/ssl-opt.sh -f "DTLS reordering: Buffer encrypted Finished message, drop for fragmented NewSessionTicket" } component_test_psa_collect_statuses () { msg "build+test: psa_collect_statuses" # ~30s scripts/config.py full - record_status tests/scripts/psa_collect_statuses.py + tests/scripts/psa_collect_statuses.py # Check that psa_crypto_init() succeeded at least once - record_status grep -q '^0:psa_crypto_init:' tests/statuses.log + grep -q '^0:psa_crypto_init:' tests/statuses.log rm -f tests/statuses.log } @@ -1203,16 +1204,16 @@ component_test_full_cmake_clang () { make test msg "test: psa_constant_names (full config, clang)" # ~ 1s - record_status tests/scripts/test_psa_constant_names.py + tests/scripts/test_psa_constant_names.py msg "test: ssl-opt.sh default, ECJPAKE, SSL async (full config)" # ~ 1s - if_build_succeeded tests/ssl-opt.sh -f 'Default\|ECJPAKE\|SSL async private' + tests/ssl-opt.sh -f 'Default\|ECJPAKE\|SSL async private' msg "test: compat.sh DES, 3DES & NULL (full config)" # ~ 2 min - if_build_succeeded env OPENSSL_CMD="$OPENSSL_LEGACY" GNUTLS_CLI="$GNUTLS_LEGACY_CLI" GNUTLS_SERV="$GNUTLS_LEGACY_SERV" tests/compat.sh -e '^$' -f 'NULL\|DES' + env OPENSSL_CMD="$OPENSSL_LEGACY" GNUTLS_CLI="$GNUTLS_LEGACY_CLI" GNUTLS_SERV="$GNUTLS_LEGACY_SERV" tests/compat.sh -e '^$' -f 'NULL\|DES' msg "test: compat.sh ARIA + ChachaPoly" - if_build_succeeded env OPENSSL_CMD="$OPENSSL_NEXT" tests/compat.sh -e '^$' -f 'ARIA\|CHACHA' + env OPENSSL_CMD="$OPENSSL_NEXT" tests/compat.sh -e '^$' -f 'ARIA\|CHACHA' } component_test_memsan_constant_flow () { @@ -1319,59 +1320,59 @@ component_build_crypto_default () { msg "build: make, crypto only" scripts/config.py crypto make CFLAGS='-O1 -Werror' - if_build_succeeded are_empty_libraries library/libmbedx509.* library/libmbedtls.* + are_empty_libraries library/libmbedx509.* library/libmbedtls.* } component_build_crypto_full () { msg "build: make, crypto only, full config" scripts/config.py crypto_full make CFLAGS='-O1 -Werror' - if_build_succeeded are_empty_libraries library/libmbedx509.* library/libmbedtls.* + are_empty_libraries library/libmbedx509.* library/libmbedtls.* } component_build_crypto_baremetal () { msg "build: make, crypto only, baremetal config" scripts/config.py crypto_baremetal make CFLAGS='-O1 -Werror' - if_build_succeeded are_empty_libraries library/libmbedx509.* library/libmbedtls.* + are_empty_libraries library/libmbedx509.* library/libmbedtls.* } component_test_depends_curves () { msg "test/build: curves.pl (gcc)" # ~ 4 min - record_status tests/scripts/curves.pl + tests/scripts/curves.pl } component_test_depends_curves_psa () { msg "test/build: curves.pl with MBEDTLS_USE_PSA_CRYPTO defined (gcc)" scripts/config.py set MBEDTLS_USE_PSA_CRYPTO - record_status tests/scripts/curves.pl + tests/scripts/curves.pl } component_test_depends_hashes () { msg "test/build: depends-hashes.pl (gcc)" # ~ 2 min - record_status tests/scripts/depends-hashes.pl + tests/scripts/depends-hashes.pl } component_test_depends_hashes_psa () { msg "test/build: depends-hashes.pl with MBEDTLS_USE_PSA_CRYPTO defined (gcc)" scripts/config.py set MBEDTLS_USE_PSA_CRYPTO - record_status tests/scripts/depends-hashes.pl + tests/scripts/depends-hashes.pl } component_test_depends_pkalgs () { msg "test/build: depends-pkalgs.pl (gcc)" # ~ 2 min - record_status tests/scripts/depends-pkalgs.pl + tests/scripts/depends-pkalgs.pl } component_test_depends_pkalgs_psa () { msg "test/build: depends-pkalgs.pl with MBEDTLS_USE_PSA_CRYPTO defined (gcc)" scripts/config.py set MBEDTLS_USE_PSA_CRYPTO - record_status tests/scripts/depends-pkalgs.pl + tests/scripts/depends-pkalgs.pl } component_build_key_exchanges () { msg "test/build: key-exchanges (gcc)" # ~ 1 min - record_status tests/scripts/key-exchanges.pl + tests/scripts/key-exchanges.pl } component_build_default_make_gcc_and_cxx () { @@ -1379,7 +1380,7 @@ component_build_default_make_gcc_and_cxx () { make CC=gcc CFLAGS='-Werror -Wall -Wextra -Os' msg "test: verify header list in cpp_dummy_build.cpp" - record_status check_headers_in_cpp + check_headers_in_cpp msg "build: Unix make, incremental g++" make TEST_CPP=1 @@ -1434,16 +1435,16 @@ component_test_no_use_psa_crypto_full_cmake_asan() { make test msg "test: ssl-opt.sh (full minus MBEDTLS_USE_PSA_CRYPTO)" - if_build_succeeded tests/ssl-opt.sh + tests/ssl-opt.sh msg "test: compat.sh default (full minus MBEDTLS_USE_PSA_CRYPTO)" - if_build_succeeded tests/compat.sh + tests/compat.sh msg "test: compat.sh DES & NULL (full minus MBEDTLS_USE_PSA_CRYPTO)" - if_build_succeeded env OPENSSL_CMD="$OPENSSL_LEGACY" GNUTLS_CLI="$GNUTLS_LEGACY_CLI" GNUTLS_SERV="$GNUTLS_LEGACY_SERV" tests/compat.sh -e '3DES\|DES-CBC3' -f 'NULL\|DES' + env OPENSSL_CMD="$OPENSSL_LEGACY" GNUTLS_CLI="$GNUTLS_LEGACY_CLI" GNUTLS_SERV="$GNUTLS_LEGACY_SERV" tests/compat.sh -e '3DES\|DES-CBC3' -f 'NULL\|DES' msg "test: compat.sh ARIA + ChachaPoly (full minus MBEDTLS_USE_PSA_CRYPTO)" - if_build_succeeded env OPENSSL_CMD="$OPENSSL_NEXT" tests/compat.sh -e '^$' -f 'ARIA\|CHACHA' + env OPENSSL_CMD="$OPENSSL_NEXT" tests/compat.sh -e '^$' -f 'ARIA\|CHACHA' } component_test_psa_crypto_config_basic() { @@ -1916,7 +1917,7 @@ component_test_memory_buffer_allocator () { msg "test: ssl-opt.sh, MBEDTLS_MEMORY_BUFFER_ALLOC_C" # MBEDTLS_MEMORY_BUFFER_ALLOC is slow. Skip tests that tend to time out. - if_build_succeeded tests/ssl-opt.sh -e '^DTLS proxy' + tests/ssl-opt.sh -e '^DTLS proxy' } component_test_no_max_fragment_length () { @@ -1927,7 +1928,7 @@ component_test_no_max_fragment_length () { make msg "test: ssl-opt.sh, MFL-related tests" - if_build_succeeded tests/ssl-opt.sh -f "Max fragment length" + tests/ssl-opt.sh -f "Max fragment length" } component_test_asan_remove_peer_certificate () { @@ -1940,13 +1941,13 @@ component_test_asan_remove_peer_certificate () { make test msg "test: ssl-opt.sh, !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE" - if_build_succeeded tests/ssl-opt.sh + tests/ssl-opt.sh msg "test: compat.sh, !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE" - if_build_succeeded tests/compat.sh + tests/compat.sh msg "test: context-info.sh, !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE" - if_build_succeeded tests/context-info.sh + tests/context-info.sh } component_test_no_max_fragment_length_small_ssl_out_content_len () { @@ -1958,10 +1959,10 @@ component_test_no_max_fragment_length_small_ssl_out_content_len () { make msg "test: MFL tests (disabled MFL extension case) & large packet tests" - if_build_succeeded tests/ssl-opt.sh -f "Max fragment length\|Large buffer" + tests/ssl-opt.sh -f "Max fragment length\|Large buffer" msg "test: context-info.sh (disabled MFL extension case)" - if_build_succeeded tests/context-info.sh + tests/context-info.sh } component_test_variable_ssl_in_out_buffer_len () { @@ -1974,10 +1975,10 @@ component_test_variable_ssl_in_out_buffer_len () { make test msg "test: ssl-opt.sh, MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH enabled" - if_build_succeeded tests/ssl-opt.sh + tests/ssl-opt.sh msg "test: compat.sh, MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH enabled" - if_build_succeeded tests/compat.sh + tests/compat.sh } component_test_variable_ssl_in_out_buffer_len_CID () { @@ -1992,10 +1993,10 @@ component_test_variable_ssl_in_out_buffer_len_CID () { make test msg "test: ssl-opt.sh, MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH and MBEDTLS_SSL_DTLS_CONNECTION_ID enabled" - if_build_succeeded tests/ssl-opt.sh + tests/ssl-opt.sh msg "test: compat.sh, MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH and MBEDTLS_SSL_DTLS_CONNECTION_ID enabled" - if_build_succeeded tests/compat.sh + tests/compat.sh } component_test_ssl_alloc_buffer_and_mfl () { @@ -2012,7 +2013,7 @@ component_test_ssl_alloc_buffer_and_mfl () { make test msg "test: MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH, MBEDTLS_MEMORY_BUFFER_ALLOC_C, MBEDTLS_MEMORY_DEBUG and MBEDTLS_SSL_MAX_FRAGMENT_LENGTH" - if_build_succeeded tests/ssl-opt.sh -f "Handshake memory usage" + tests/ssl-opt.sh -f "Handshake memory usage" } component_test_when_no_ciphersuites_have_mac () { @@ -2026,7 +2027,7 @@ component_test_when_no_ciphersuites_have_mac () { make test msg "test ssl-opt.sh: !MBEDTLS_SSL_SOME_MODES_USE_MAC" - if_build_succeeded tests/ssl-opt.sh -f 'Default\|EtM' -e 'without EtM' + tests/ssl-opt.sh -f 'Default\|EtM' -e 'without EtM' } component_test_no_date_time () { @@ -2062,7 +2063,7 @@ component_test_malloc_0_null () { msg "selftest: malloc(0) returns NULL (ASan+UBSan build)" # Just the calloc selftest. "make test" ran the others as part of the # test suites. - if_build_succeeded programs/test/selftest calloc + programs/test/selftest calloc msg "test ssl-opt.sh: malloc(0) returns NULL (ASan+UBSan build)" # Run a subset of the tests. The choice is a balance between coverage @@ -2070,7 +2071,7 @@ component_test_malloc_0_null () { # The current choice is to skip tests whose description includes # "proxy", which is an approximation of skipping tests that use the # UDP proxy, which tend to be slower and flakier. - if_build_succeeded tests/ssl-opt.sh -e 'proxy' + tests/ssl-opt.sh -e 'proxy' } component_test_aes_fewer_tables () { @@ -2261,7 +2262,7 @@ component_test_m32_o1 () { make test msg "test ssl-opt.sh, i386, make, gcc-O1" - if_build_succeeded tests/ssl-opt.sh + tests/ssl-opt.sh } support_test_m32_o1 () { support_test_m32_o0 "$@" @@ -2276,11 +2277,11 @@ component_test_m32_everest () { make test msg "test: i386, Everest ECDH context - ECDH-related part of ssl-opt.sh (ASan build)" # ~ 5s - if_build_succeeded tests/ssl-opt.sh -f ECDH + tests/ssl-opt.sh -f ECDH msg "test: i386, Everest ECDH context - compat.sh with some ECDH ciphersuites (ASan build)" # ~ 3 min # Exclude some symmetric ciphers that are redundant here to gain time. - if_build_succeeded tests/compat.sh -f ECDH -V NO -e 'ARIA\|CAMELLIA\|CHACHA\|DES' + tests/compat.sh -f ECDH -V NO -e 'ARIA\|CAMELLIA\|CHACHA\|DES' } support_test_m32_everest () { support_test_m32_o0 "$@" @@ -2378,7 +2379,7 @@ component_test_no_x509_info () { make test msg "test: ssl-opt.sh, full + MBEDTLS_X509_REMOVE_INFO" # ~ 1 min - if_build_succeeded tests/ssl-opt.sh + tests/ssl-opt.sh } component_build_arm_none_eabi_gcc () { @@ -2419,7 +2420,7 @@ component_build_arm_none_eabi_gcc_no_udbl_division () { scripts/config.py set MBEDTLS_NO_UDBL_DIVISION make CC="${ARM_NONE_EABI_GCC_PREFIX}gcc" AR="${ARM_NONE_EABI_GCC_PREFIX}ar" LD="${ARM_NONE_EABI_GCC_PREFIX}ld" CFLAGS='-std=c99 -Werror -Wall -Wextra' lib echo "Checking that software 64-bit division is not required" - if_build_succeeded not grep __aeabi_uldiv library/*.o + not grep __aeabi_uldiv library/*.o } component_build_arm_none_eabi_gcc_no_64bit_multiplication () { @@ -2428,7 +2429,7 @@ component_build_arm_none_eabi_gcc_no_64bit_multiplication () { scripts/config.py set MBEDTLS_NO_64BIT_MULTIPLICATION make CC="${ARM_NONE_EABI_GCC_PREFIX}gcc" AR="${ARM_NONE_EABI_GCC_PREFIX}ar" LD="${ARM_NONE_EABI_GCC_PREFIX}ld" CFLAGS='-std=c99 -Werror -O1 -march=armv6-m -mthumb' lib echo "Checking that software 64-bit multiplication is not required" - if_build_succeeded not grep __aeabi_lmul library/*.o + not grep __aeabi_lmul library/*.o } component_build_armcc () { @@ -2496,13 +2497,13 @@ component_test_memsan () { make test msg "test: ssl-opt.sh (MSan)" # ~ 1 min - if_build_succeeded tests/ssl-opt.sh + tests/ssl-opt.sh # Optional part(s) if [ "$MEMORY" -gt 0 ]; then msg "test: compat.sh (MSan)" # ~ 6 min 20s - if_build_succeeded tests/compat.sh + tests/compat.sh fi } @@ -2518,17 +2519,17 @@ component_test_valgrind () { # seem to receive signals under valgrind on OS X). if [ "$MEMORY" -gt 0 ]; then msg "test: ssl-opt.sh --memcheck (Release)" - if_build_succeeded tests/ssl-opt.sh --memcheck + tests/ssl-opt.sh --memcheck fi if [ "$MEMORY" -gt 1 ]; then msg "test: compat.sh --memcheck (Release)" - if_build_succeeded tests/compat.sh --memcheck + tests/compat.sh --memcheck fi if [ "$MEMORY" -gt 0 ]; then msg "test: context-info.sh --memcheck (Release)" - if_build_succeeded tests/context-info.sh --memcheck + tests/context-info.sh --memcheck fi } @@ -2547,10 +2548,10 @@ component_test_cmake_out_of_source () { # "No such file or directory", which would indicate that some required # file is missing (ssl-opt.sh tolerates the absence of some files so # may exit with status 0 but emit errors). - if_build_succeeded ./tests/ssl-opt.sh -f 'Fallback SCSV: beginning of list' 2>ssl-opt.err + ./tests/ssl-opt.sh -f 'Fallback SCSV: beginning of list' 2>ssl-opt.err cat ssl-opt.err >&2 # If ssl-opt.err is non-empty, record an error and keep going. - record_status [ ! -s ssl-opt.err ] + [ ! -s ssl-opt.err ] rm ssl-opt.err cd "$MBEDTLS_ROOT_DIR" rm -rf "$OUT_OF_SOURCE_DIR" @@ -2564,7 +2565,7 @@ component_test_cmake_as_subdirectory () { cd programs/test/cmake_subproject cmake . make - if_build_succeeded ./cmake_subproject + ./cmake_subproject cd "$MBEDTLS_ROOT_DIR" unset MBEDTLS_ROOT_DIR @@ -2577,7 +2578,7 @@ component_test_cmake_as_package () { cd programs/test/cmake_package cmake . make - if_build_succeeded ./cmake_package + ./cmake_package cd "$MBEDTLS_ROOT_DIR" unset MBEDTLS_ROOT_DIR @@ -2590,7 +2591,7 @@ component_test_cmake_as_package_install () { cd programs/test/cmake_package_install cmake . make - if_build_succeeded ./cmake_package_install + ./cmake_package_install cd "$MBEDTLS_ROOT_DIR" unset MBEDTLS_ROOT_DIR @@ -2615,9 +2616,9 @@ component_test_zeroize () { for compiler in clang gcc; do msg "test: $compiler $optimization_flag, mbedtls_platform_zeroize()" make programs CC="$compiler" DEBUG=1 CFLAGS="$optimization_flag" - if_build_succeeded gdb -ex "$gdb_disable_aslr" -x tests/scripts/test_zeroize.gdb -nw -batch -nx 2>&1 | tee test_zeroize.log - if_build_succeeded grep "The buffer was correctly zeroized" test_zeroize.log - if_build_succeeded not grep -i "error" test_zeroize.log + gdb -ex "$gdb_disable_aslr" -x tests/scripts/test_zeroize.gdb -nw -batch -nx 2>&1 | tee test_zeroize.log + grep "The buffer was correctly zeroized" test_zeroize.log + not grep -i "error" test_zeroize.log rm -f test_zeroize.log make clean done @@ -2628,7 +2629,7 @@ component_test_zeroize () { component_check_python_files () { msg "Lint: Python scripts" - record_status tests/scripts/check-python-files.sh + tests/scripts/check-python-files.sh } component_check_generate_test_code () { @@ -2636,7 +2637,7 @@ component_check_generate_test_code () { # unittest writes out mundane stuff like number or tests run on stderr. # Our convention is to reserve stderr for actual errors, and write # harmless info on stdout so it can be suppress with --quiet. - record_status ./tests/scripts/test_generate_test_code.py 2>&1 + ./tests/scripts/test_generate_test_code.py 2>&1 } ################################################################ From fec30cbe8c800dcc3615601eb2d6859de8d78da5 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 28 Mar 2020 19:34:23 +0100 Subject: [PATCH 07/27] Fix double reporting when the last command of a function fails Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 8b8e3dd590..cb3c8f2fb9 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -480,6 +480,11 @@ pre_setup_keep_going () { failure_count=0 # Number of failed components last_failure_status=0 # Last failure status in this component + # See err_trap + previous_failure_status=0 + previous_failed_command= + previous_failure_funcall_depth=0 + start_red= end_color= if [ -t 1 ]; then @@ -517,6 +522,21 @@ pre_setup_keep_going () { last_failure_status=$? failed_command=$BASH_COMMAND + if [[ $last_failure_status -eq $previous_failure_status && + "$failed_command" == "$previous_failed_command" && + ${#FUNCNAME[@]} == $((previous_failure_funcall_depth - 1)) ]] + then + # The same command failed twice in a row, but this time one level + # less deep in the function call stack. This happens when the last + # command of a function returns a nonzero status, and the function + # returns that same status. Ignore the second failure. + previous_failure_funcall_depth=${#FUNCNAME[@]} + return + fi + previous_failure_status=$last_failure_status + previous_failed_command=$failed_command + previous_failure_funcall_depth=${#FUNCNAME[@]} + text="$current_section: $failed_command -> $last_failure_status" echo "${start_red}^^^^$text^^^^${end_color}" >&2 echo "$text" >>"$failure_summary_file" From a681c59d348f362bd18fcff72c694fc8ede98546 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 28 Mar 2020 21:27:40 +0100 Subject: [PATCH 08/27] Better not function In the `not` function, in keep-going mode, arrange to report the failing command (rather than `"$@"`). Note that the `!` keyword should not be used, because failures with `!` are not reported properly. Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index cb3c8f2fb9..d95ca7b897 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -90,6 +90,9 @@ # # Each component must start by invoking `msg` with a short informative message. # +# Warning: due to the way bash detects errors, the failure of a command +# inside 'if' or '!' is not detected. Use the 'not' function instead of '!'. +# # Each component is executed in a separate shell process. The component # fails if any command in it returns a non-zero status. # @@ -484,6 +487,7 @@ pre_setup_keep_going () { previous_failure_status=0 previous_failed_command= previous_failure_funcall_depth=0 + unset report_failed_command start_red= end_color= @@ -508,7 +512,7 @@ pre_setup_keep_going () { "msg "*) false;; *[!A-Za-z]"test"|*[!A-Za-z]"test"[!A-Za-z]*) true;; "tests/"*) true;; - "grep "*|"not grep "*) true;; + "grep "*|"! grep "*) true;; *) false;; esac } @@ -520,7 +524,7 @@ pre_setup_keep_going () { # Save $? (status of the failing command). This must be the very # first thing, before $? is overridden. last_failure_status=$? - failed_command=$BASH_COMMAND + failed_command=${report_failed_command-$BASH_COMMAND} if [[ $last_failure_status -eq $previous_failure_status && "$failed_command" == "$previous_failed_command" && @@ -578,8 +582,14 @@ if_build_succeeded () { "$@" } -not() { - ! "$@" +# '! true' does not trigger the ERR trap. Arrange to trigger it, with +# a reasonably informative error message (not just "$@"). +not () { + if "$@"; then + report_failed_command="! $*" + false + unset report_failed_command + fi } pre_setup_quiet_redirect () { From b80f0d20ea063ff0a87ac52e58c8155faf53f8bd Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 28 Mar 2020 21:37:59 +0100 Subject: [PATCH 09/27] Complain if an unsupported component is explicitly requested In all.sh, when an explicit list of components is specified, error out if one of the components is not known or not supported. Patterns that happen to match zero components are still effectively ignored. Fix #2783 Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index d95ca7b897..61b17e1022 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -444,6 +444,24 @@ pre_parse_command_line () { COMMAND_LINE_COMPONENTS="$COMMAND_LINE_COMPONENTS *_armcc*" fi + if [ $all_except -eq 0 ]; then + unsupported=0 + for component in $COMMAND_LINE_COMPONENTS; do + case $component in + *[*?\[]*) continue;; + esac + case " $SUPPORTED_COMPONENTS " in + *" $component "*) :;; + *) + echo >&2 "Component $component was explicitly requested, but is not known or not supported." + unsupported=$((unsupported + 1));; + esac + done + if [ $unsupported -ne 0 ]; then + exit 2 + fi + fi + # Build the list of components to run. RUN_COMPONENTS= for component in $SUPPORTED_COMPONENTS; do From c2e22ee27177c2041cdb047325db20df500796d4 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sat, 28 Mar 2020 22:02:50 +0100 Subject: [PATCH 10/27] Remove code that is useless now that components run in a subshell Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 61b17e1022..6249977c82 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -267,10 +267,6 @@ EOF # remove built files as well as the cmake cache/config cleanup() { - if [ -n "${MBEDTLS_ROOT_DIR+set}" ]; then - cd "$MBEDTLS_ROOT_DIR" - fi - command make clean # Remove CMake artefacts @@ -2603,7 +2599,6 @@ component_test_cmake_out_of_source () { rm ssl-opt.err cd "$MBEDTLS_ROOT_DIR" rm -rf "$OUT_OF_SOURCE_DIR" - unset MBEDTLS_ROOT_DIR } component_test_cmake_as_subdirectory () { From aca0b32132533acec4cbb9c64b5e64338bb04145 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 20 Apr 2020 13:21:27 +0200 Subject: [PATCH 11/27] Keep going after a shell "[" a.k.a. "test" fails This is necessary to actually keep going and finish the component-specific cleanup in component_test_cmake_out_of_source if ssl-opt.err is non-empty. Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 6249977c82..afc1a49185 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -527,6 +527,7 @@ pre_setup_keep_going () { *[!A-Za-z]"test"|*[!A-Za-z]"test"[!A-Za-z]*) true;; "tests/"*) true;; "grep "*|"! grep "*) true;; + "test "*|"[ "*) true;; *) false;; esac } From 88a07457c7e6dbe0c170a75110f84bad39f156c6 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 8 Jul 2021 19:03:50 +0200 Subject: [PATCH 12/27] Remove barely-used redirect functions redirect_out was no longer used and redirect_err was only used to quiet dd. Change the dd invocation to only print diagnostics on error (on platforms where this is possible). Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index afc1a49185..4614029ad0 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -607,24 +607,6 @@ not () { fi } -pre_setup_quiet_redirect () { - if [ $QUIET -ne 1 ]; then - redirect_out () { - "$@" - } - redirect_err () { - "$@" - } - else - redirect_out () { - "$@" >/dev/null - } - redirect_err () { - "$@" 2>/dev/null - } - fi -} - pre_prepare_outcome_file () { case "$MBEDTLS_TEST_OUTCOME_FILE" in [!/]*) MBEDTLS_TEST_OUTCOME_FILE="$PWD/$MBEDTLS_TEST_OUTCOME_FILE";; @@ -2726,7 +2708,12 @@ run_component () { # Unconditionally create a seedfile that's sufficiently long. # Do this before each component, because a previous component may # have messed it up or shortened it. - redirect_err dd if=/dev/urandom of=./tests/seedfile bs=64 count=1 + local dd_cmd + dd_cmd=(dd if=/dev/urandom of=./tests/seedfile bs=64 count=1) + case $OSTYPE in + linux*|freebsd*|openbsd*|darwin*) dd_cmd+=(status=none) + esac + "${dd_cmd[@]}" # Run the component in a subshell if [ $KEEP_GOING -eq 1 ]; then @@ -2774,7 +2761,6 @@ build_status=0 if [ $KEEP_GOING -eq 1 ]; then pre_setup_keep_going fi -pre_setup_quiet_redirect pre_prepare_outcome_file pre_print_configuration pre_check_tools From 72385036420e9a072c0588ccd685f8f72cf6458d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 8 Jul 2021 19:07:07 +0200 Subject: [PATCH 13/27] Heed --quiet when running make generated_files Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 4614029ad0..c3df05b724 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -713,7 +713,11 @@ pre_generate_files() { # since make doesn't have proper dependencies, remove any possibly outdate # file that might be around before generating fresh ones make neat - make generated_files + if [ $QUIET -eq 1 ]; then + make -s generated_files + else + make generated_files + fi } From 03ab544832fceeb1d618fef1d4f2e44641a9eadc Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 9 Jul 2021 15:19:28 +0200 Subject: [PATCH 14/27] Generate cpp_cummy_build.cpp dynamically Generate programs/test/cpp_dummy_build.cpp dynamically instead of maintaining it manually. This removes the need to update it when the list of headers changes. Include all the headers unconditionally except for the ones that cannot be included directly. Support this dynamic generation both with make and with cmake. Adapt all.sh accordingly. Remove the redundant C build from component_build_default_make_gcc_and_cxx (it was also done in component_test_default_out_of_box), leaving a component_test_make_cxx. Also run the C++ program, because why not. Do this in the full configuration which may catch a bit more problems in headers. Fixes #2570 for good. Signed-off-by: Gilles Peskine --- programs/.gitignore | 1 + programs/Makefile | 7 +- programs/test/CMakeLists.txt | 12 ++- programs/test/cpp_dummy_build.cpp | 98 ----------------------- programs/test/generate_cpp_dummy_build.sh | 85 ++++++++++++++++++++ tests/scripts/all.sh | 22 ++--- 6 files changed, 109 insertions(+), 116 deletions(-) delete mode 100644 programs/test/cpp_dummy_build.cpp create mode 100755 programs/test/generate_cpp_dummy_build.sh diff --git a/programs/.gitignore b/programs/.gitignore index 83521a792b..d8eb6baa03 100644 --- a/programs/.gitignore +++ b/programs/.gitignore @@ -56,6 +56,7 @@ ssl/ssl_server ssl/ssl_server2 test/benchmark test/cpp_dummy_build +test/cpp_dummy_build.cpp test/ecp-bench test/query_compile_time_config test/selftest diff --git a/programs/Makefile b/programs/Makefile index 997c198716..977ae7e8b4 100644 --- a/programs/Makefile +++ b/programs/Makefile @@ -331,6 +331,10 @@ test/benchmark$(EXEXT): test/benchmark.c $(DEP) echo " CC test/benchmark.c" $(CC) $(LOCAL_CFLAGS) $(CFLAGS) test/benchmark.c $(LOCAL_LDFLAGS) $(LDFLAGS) -o $@ +test/cpp_dummy_build.cpp: test/generate_cpp_dummy_build.sh + echo " Gen test/cpp_dummy_build.cpp" + test/generate_cpp_dummy_build.sh + test/cpp_dummy_build$(EXEXT): test/cpp_dummy_build.cpp $(DEP) echo " CXX test/cpp_dummy_build.cpp" $(CXX) $(LOCAL_CXXFLAGS) $(CXXFLAGS) test/cpp_dummy_build.cpp $(LOCAL_LDFLAGS) $(LDFLAGS) -o $@ @@ -391,10 +395,11 @@ clean: ifndef WINDOWS rm -f $(EXES) -rm -f ssl/ssl_pthread_server$(EXEXT) - -rm -f test/cpp_dummy_build$(EXEXT) + -rm -f test/cpp_dummy_build.cpp test/cpp_dummy_build$(EXEXT) else if exist *.o del /Q /F *.o if exist *.exe del /Q /F *.exe + if exist test\cpp_dummy_build.cpp del /Q /F test\cpp_dummy_build.cpp endif $(MAKE) -C fuzz clean diff --git a/programs/test/CMakeLists.txt b/programs/test/CMakeLists.txt index 807d1bc10b..a0a1b763cc 100644 --- a/programs/test/CMakeLists.txt +++ b/programs/test/CMakeLists.txt @@ -14,7 +14,17 @@ set(executables_mbedcrypto ) if(TEST_CPP) - list(APPEND executables_mbedcrypto cpp_dummy_build) + set(cpp_dummy_build_cpp "${CMAKE_CURRENT_BINARY_DIR}/cpp_dummy_build.cpp") + set(generate_cpp_dummy_build "${CMAKE_CURRENT_SOURCE_DIR}/generate_cpp_dummy_build.sh") + add_custom_command( + OUTPUT "${cpp_dummy_build_cpp}" + COMMAND "${generate_cpp_dummy_build}" "${cpp_dummy_build_cpp}" + DEPENDS "${generate_cpp_dummy_build}" + WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}" + ) + add_executable(cpp_dummy_build "${cpp_dummy_build_cpp}") + target_include_directories(cpp_dummy_build PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/../../include) + target_link_libraries(cpp_dummy_build ${mbedcrypto_target}) endif() foreach(exe IN LISTS executables_libs executables_mbedcrypto) diff --git a/programs/test/cpp_dummy_build.cpp b/programs/test/cpp_dummy_build.cpp deleted file mode 100644 index 7f1efe8dba..0000000000 --- a/programs/test/cpp_dummy_build.cpp +++ /dev/null @@ -1,98 +0,0 @@ -/* - * This program is a dummy C++ program to ensure Mbed TLS library header files - * can be included and built with a C++ compiler. - * - * 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. - */ - -#include "mbedtls/build_info.h" - -#include "mbedtls/aes.h" -#include "mbedtls/aria.h" -#include "mbedtls/asn1.h" -#include "mbedtls/asn1write.h" -#include "mbedtls/base64.h" -#include "mbedtls/bignum.h" -#include "mbedtls/camellia.h" -#include "mbedtls/ccm.h" -#include "mbedtls/chacha20.h" -#include "mbedtls/chachapoly.h" -#include "mbedtls/check_config.h" -#include "mbedtls/cipher.h" -#include "mbedtls/cmac.h" -#include "mbedtls/config_psa.h" -#include "mbedtls/ctr_drbg.h" -#include "mbedtls/debug.h" -#include "mbedtls/des.h" -#include "mbedtls/dhm.h" -#include "mbedtls/ecdh.h" -#include "mbedtls/ecdsa.h" -#include "mbedtls/ecjpake.h" -#include "mbedtls/ecp.h" -#include "mbedtls/entropy.h" -#include "mbedtls/error.h" -#include "mbedtls/gcm.h" -#include "mbedtls/hkdf.h" -#include "mbedtls/hmac_drbg.h" -#include "mbedtls/md.h" -#include "mbedtls/md5.h" -#include "mbedtls/net_sockets.h" -#include "mbedtls/nist_kw.h" -#include "mbedtls/oid.h" -#include "mbedtls/pem.h" -#include "mbedtls/pk.h" -#include "mbedtls/pkcs12.h" -#include "mbedtls/pkcs5.h" -#include "mbedtls/platform_time.h" -#include "mbedtls/platform_util.h" -#include "mbedtls/poly1305.h" -#include "mbedtls/psa_util.h" -#include "mbedtls/ripemd160.h" -#include "mbedtls/rsa.h" -#include "mbedtls/sha1.h" -#include "mbedtls/sha256.h" -#include "mbedtls/sha512.h" -#include "mbedtls/ssl.h" -#include "mbedtls/ssl_cache.h" -#include "mbedtls/ssl_ciphersuites.h" -#include "mbedtls/ssl_cookie.h" -#include "mbedtls/ssl_ticket.h" -#include "mbedtls/threading.h" -#include "mbedtls/timing.h" -#include "mbedtls/version.h" -#include "mbedtls/x509.h" -#include "mbedtls/x509_crl.h" -#include "mbedtls/x509_crt.h" -#include "mbedtls/x509_csr.h" - -#if defined(MBEDTLS_PLATFORM_C) -#include "mbedtls/platform.h" -#endif - -#if defined(MBEDTLS_MEMORY_BUFFER_ALLOC_C) -#include "mbedtls/memory_buffer_alloc.h" -#endif - -#include "psa/crypto.h" -#include "psa/crypto_se_driver.h" - -int main() -{ - mbedtls_platform_context *ctx = NULL; - mbedtls_platform_setup(ctx); - mbedtls_printf("CPP Build test\n"); - mbedtls_platform_teardown(ctx); -} diff --git a/programs/test/generate_cpp_dummy_build.sh b/programs/test/generate_cpp_dummy_build.sh new file mode 100755 index 0000000000..41adf149eb --- /dev/null +++ b/programs/test/generate_cpp_dummy_build.sh @@ -0,0 +1,85 @@ +#!/bin/sh + +# 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 -e + +# Ensure a reproducible order for *.h +export LC_ALL=C + +print_cpp () { + cat <<'EOF' +/* Automatically generated file. Do not edit. + * + * This program is a dummy C++ program to ensure Mbed TLS library header files + * can be included and built with a C++ compiler. + * + * 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. + */ + +#include "mbedtls/build_info.h" + +EOF + + for header in include/mbedtls/*.h include/psa/*.h; do + case ${header#include/} in + mbedtls/mbedtls_config.h) :;; # not meant for direct inclusion + psa/crypto_config.h) :;; # not meant for direct inclusion + # Some of the psa/crypto_*.h headers are not meant to be included directly. + # They do have include guards that make them no-ops if psa/crypto.h + # has been included before. Since psa/crypto.h comes before psa/crypto_*.h + # in the wildcard enumeration, we don't need to skip those headers. + *) echo "#include \"${header#include/}\"";; + esac + done + + cat <<'EOF' + +int main() +{ + mbedtls_platform_context *ctx = NULL; + mbedtls_platform_setup(ctx); + mbedtls_printf("CPP Build test passed\n"); + mbedtls_platform_teardown(ctx); +} +EOF +} + +if [ -d include/mbedtls ]; then + : +elif [ -d ../include/mbedtls ]; then + cd .. +elif [ -d ../../include/mbedtls ]; then + cd ../.. +else + echo >&2 "This script must be run from an Mbed TLS source tree." + exit 3 +fi + +print_cpp >"${1:-programs/test/cpp_dummy_build.cpp}" diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index c3df05b724..8c75c9f052 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -368,14 +368,6 @@ check_tools() done } -check_headers_in_cpp () { - ls include/mbedtls | grep "\.h$" >headers.txt - Date: Mon, 30 Mar 2020 20:11:39 +0200 Subject: [PATCH 15/27] Don't restore *config.h before backing it up Back up the config files at the beginning of all.sh, rather than before each component. In particular, create the backup before running cleanup for the first time. This fixes #3139 (all.sh using a config.h.bak from a previous job), and makes all.sh more robust against accidentally using a modified config.h midway through because a component messed with the backup. Use a different extension (*.all.bak rather than *.bak) for the backups. This is necessary to ensure that auxiliary scripts such as depends*.pl that make their own backup don't remove all.sh's backup, which the code from this commit does not support. Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 41 +++++++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 8c75c9f052..5c2ab2778d 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -129,9 +129,14 @@ pre_check_environment () { pre_initialize_variables () { CONFIG_H='include/mbedtls/mbedtls_config.h' - CONFIG_BAK="$CONFIG_H.bak" CRYPTO_CONFIG_H='include/psa/crypto_config.h' - CRYPTO_CONFIG_BAK="$CRYPTO_CONFIG_H.bak" + + # Files that are clobbered by some jobs will be backed up. Use a different + # suffix from auxiliary scripts so that all.sh and auxiliary scripts can + # independently decide when to remove the backup file. + backup_suffix='.all.bak' + # Files clobbered by config.py + files_to_back_up="$CONFIG_H $CRYPTO_CONFIG_H" append_outcome=0 MEMORY=0 @@ -295,13 +300,18 @@ cleanup() rm -f programs/test/cmake_package_install/Makefile rm -f programs/test/cmake_package_install/cmake_package_install - if [ -f "$CONFIG_BAK" ]; then - mv "$CONFIG_BAK" "$CONFIG_H" - fi + # Restore files that may have been clobbered by the job + for x in $files_to_back_up; do + cp -p "$x$backup_suffix" "$x" + done +} - if [ -f "$CRYPTO_CONFIG_BAK" ]; then - mv "$CRYPTO_CONFIG_BAK" "$CRYPTO_CONFIG_H" - fi +final_cleanup () { + cleanup + + for x in $files_to_back_up; do + rm -f "$x$backup_suffix" + done } # Executed on exit. May be redefined depending on command line options. @@ -310,7 +320,7 @@ final_report () { } fatal_signal () { - cleanup + final_cleanup final_report $1 trap - $1 kill -$1 $$ @@ -485,6 +495,12 @@ pre_check_git () { fi } +pre_back_up () { + for x in $files_to_back_up; do + cp -p "$x" "$x$backup_suffix" + done +} + pre_setup_keep_going () { failure_count=0 # Number of failed components last_failure_status=0 # Last failure status in this component @@ -2666,7 +2682,7 @@ component_check_generate_test_code () { post_report () { msg "Done, cleaning up" - cleanup + final_cleanup final_report } @@ -2692,10 +2708,6 @@ pseudo_component_error_test () { # Run one component and clean up afterwards. run_component () { - # Back up the configuration in case the component modifies it. - # The cleanup function will restore it. - cp -p "$CONFIG_H" "$CONFIG_BAK" - cp -p "$CRYPTO_CONFIG_H" "$CRYPTO_CONFIG_BAK" current_component="$1" export MBEDTLS_TEST_CONFIGURATION="$current_component" @@ -2750,6 +2762,7 @@ pre_initialize_variables pre_parse_command_line "$@" pre_check_git +pre_back_up build_status=0 if [ $KEEP_GOING -eq 1 ]; then From 568f53a9d8734f8dc28f2a9c8fe86ef5267df0af Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 12 Jul 2021 18:16:01 +0200 Subject: [PATCH 16/27] Don't unconditionally restore **/Makefile all.sh restores **/Makefile from git in case the version in the worktree was from doing a cmake in-tree build. Instead of doing this unconditionally, do it only if the toplevel Makefile seems to have been automatically generated (by cmake or otherwise, e.g. by mbedtls-prepare-build). This way all.sh no longer silently wipes changes made to Makefile but not committed yet. Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 5c2ab2778d..7db9488eee 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -137,6 +137,8 @@ pre_initialize_variables () { backup_suffix='.all.bak' # Files clobbered by config.py files_to_back_up="$CONFIG_H $CRYPTO_CONFIG_H" + # Files clobbered by in-tree cmake + files_to_back_up="$files_to_back_up Makefile library/Makefile programs/Makefile tests/Makefile programs/fuzz/Makefile" append_outcome=0 MEMORY=0 @@ -282,8 +284,6 @@ cleanup() -iname CMakeCache.txt \) -exec rm -f {} \+ # Recover files overwritten by in-tree CMake builds rm -f include/Makefile include/mbedtls/Makefile programs/*/Makefile - git update-index --no-skip-worktree Makefile library/Makefile programs/Makefile tests/Makefile programs/fuzz/Makefile - git checkout -- Makefile library/Makefile programs/Makefile tests/Makefile programs/fuzz/Makefile # Remove any artifacts from the component_test_cmake_as_subdirectory test. rm -rf programs/test/cmake_subproject/build @@ -495,6 +495,20 @@ pre_check_git () { fi } +pre_restore_files () { + # If the makefiles have been generated by a framework such as cmake, + # restore them from git. If the makefiles look like modifications from + # the ones checked into git, take care not to modify them. Whatever + # this function leaves behind is what the script will restore before + # each component. + case "$(head -n1 Makefile)" in + *[Gg]enerated*) + git update-index --no-skip-worktree Makefile library/Makefile programs/Makefile tests/Makefile programs/fuzz/Makefile + git checkout -- Makefile library/Makefile programs/Makefile tests/Makefile programs/fuzz/Makefile + ;; + esac +} + pre_back_up () { for x in $files_to_back_up; do cp -p "$x" "$x$backup_suffix" @@ -2762,6 +2776,7 @@ pre_initialize_variables pre_parse_command_line "$@" pre_check_git +pre_restore_files pre_back_up build_status=0 From ec135544c81aa4c292f8b715244ac1b38682580b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 2 Aug 2021 23:14:03 +0200 Subject: [PATCH 17/27] Clarify some comments Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 7db9488eee..73630c122e 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -2714,8 +2714,11 @@ pseudo_component_error_test () { echo "Expect three failing commands." fi error_test='this should not be used since the component runs in a subshell' + # Expected error: 'grep non_existent /dev/null -> 1' grep non_existent /dev/null + # Expected error: '! grep -q . tests/scripts/all.sh -> 1' not grep -q . "$0" + # Expected error: 'make unknown_target -> 2' make unknown_target false "this should not be executed" } @@ -2735,8 +2738,11 @@ run_component () { esac "${dd_cmd[@]}" - # Run the component in a subshell + # Run the component in a subshell, with error trapping and output + # redirection set up based on the relevant options. if [ $KEEP_GOING -eq 1 ]; then + # We want to keep running if the subshell fails, so 'set -e' must + # be off when the subshell runs. set +e fi ( From 88a7c2b32e6cf289d8287186a20370aead8e6eb1 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 2 Aug 2021 23:28:00 +0200 Subject: [PATCH 18/27] Improve --error-test reporting Count invocations from 1 to n instead of n to 1. Explain how changing the loop variable would cause an error if the function was not executed in a subshell. Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 73630c122e..54b28b870f 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -2709,11 +2709,13 @@ post_report () { # Function invoked by --error-test to test error reporting. pseudo_component_error_test () { - msg "Testing error reporting $error_test" + msg "Testing error reporting $error_test_i" if [ $KEEP_GOING -ne 0 ]; then echo "Expect three failing commands." fi - error_test='this should not be used since the component runs in a subshell' + # If the component doesn't run in a subshell, changing error_test_i to an + # invalid integer will cause an error in the loop that runs this function. + error_test_i=this_should_not_be_used_since_the_component_runs_in_a_subshell # Expected error: 'grep non_existent /dev/null -> 1' grep non_existent /dev/null # Expected error: '! grep -q . tests/scripts/all.sh -> 1' @@ -2796,10 +2798,10 @@ cleanup pre_generate_files # Run the requested tests. -while [ $error_test -gt 0 ]; do +for ((error_test_i=1; error_test_i <= error_test; error_test_i++)); do run_component pseudo_component_error_test - error_test=$((error_test - 1)) done +unset error_test_i for component in $RUN_COMPONENTS; do run_component "component_$component" done From c111e2429287ba5f20708a4853541fd3cb4f0ae1 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 2 Aug 2021 23:29:53 +0200 Subject: [PATCH 19/27] Improve the detection of keep-going commands Have simpler patterns related to 'test' (the central objective being to keep going if 'make test' or 'tests/...' fails, but not if 'make tests' fails). Add 'cd' as a can't-keep-going command. Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 54b28b870f..6cdc922fe3 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -543,13 +543,19 @@ pre_setup_keep_going () { # Whether it makes sense to keep a component going after the specified # command fails (test command) or not (configure or build). # This doesn't have to be 100% accurate: all failures are recorded anyway. + # False positives result in running things that can't be expected to + # work. False negatives result in things not running after something else + # failed even though they might have given useful feedback. can_keep_going_after_failure () { case "$1" in "msg "*) false;; - *[!A-Za-z]"test"|*[!A-Za-z]"test"[!A-Za-z]*) true;; - "tests/"*) true;; - "grep "*|"! grep "*) true;; - "test "*|"[ "*) true;; + "cd "*) false;; + *make*[\ /]tests*) false;; # make tests, make CFLAGS=-I../tests, ... + *test*) true;; # make test, tests/stuff, env V=v tests/stuff, ... + *make*check*) true;; + "grep "*) true;; + "[ "*) true;; + "! "*) true;; *) false;; esac } From 1d475b63981742afe9a90b2e70330111c8b65c3d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 3 Aug 2021 13:43:36 +0200 Subject: [PATCH 20/27] Disable wildcards when checking for unsupported components Otherwise $COMMAND_LINE_COMPONENTS would try to expand wildcard patterns based on files in the current directory. Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 6cdc922fe3..4dfbaec668 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -444,7 +444,9 @@ pre_parse_command_line () { if [ $all_except -eq 0 ]; then unsupported=0 + set -f for component in $COMMAND_LINE_COMPONENTS; do + set +f case $component in *[*?\[]*) continue;; esac @@ -455,6 +457,7 @@ pre_parse_command_line () { unsupported=$((unsupported + 1));; esac done + set +f if [ $unsupported -ne 0 ]; then exit 2 fi From bf66e2cc8ffdd17c954d10fcc1431e41214aff7f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 3 Aug 2021 13:44:28 +0200 Subject: [PATCH 21/27] Documentation improvements Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 4dfbaec668..2dc13756a8 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -271,7 +271,9 @@ Tool path options: EOF } -# remove built files as well as the cmake cache/config +# Cleanup before/after running a component. +# Remove built files as well as the cmake cache/config. +# Does not remove generated source files. cleanup() { command make clean @@ -306,6 +308,8 @@ cleanup() done } +# Final cleanup when this script exits (except when exiting on a failure +# in non-keep-going mode). final_cleanup () { cleanup @@ -442,11 +446,14 @@ pre_parse_command_line () { COMMAND_LINE_COMPONENTS="$COMMAND_LINE_COMPONENTS *_armcc*" fi + # Error out if an explicitly requested component doesn't exist. if [ $all_except -eq 0 ]; then unsupported=0 set -f for component in $COMMAND_LINE_COMPONENTS; do set +f + # If the requested name includes a wildcard character, don't + # check it. Accept wildcard patterns that don't match anything. case $component in *[*?\[]*) continue;; esac From 3cbd69c4d42df872ba3bec1fe511a0183255da74 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 5 Aug 2021 15:10:27 +0200 Subject: [PATCH 22/27] Switch to 4-space indentation Signed-off-by: Gilles Peskine --- programs/test/generate_cpp_dummy_build.sh | 37 ++++++++++++----------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/programs/test/generate_cpp_dummy_build.sh b/programs/test/generate_cpp_dummy_build.sh index 41adf149eb..9e16348b52 100755 --- a/programs/test/generate_cpp_dummy_build.sh +++ b/programs/test/generate_cpp_dummy_build.sh @@ -21,7 +21,7 @@ set -e export LC_ALL=C print_cpp () { - cat <<'EOF' + cat <<'EOF' /* Automatically generated file. Do not edit. * * This program is a dummy C++ program to ensure Mbed TLS library header files @@ -47,19 +47,20 @@ print_cpp () { EOF - for header in include/mbedtls/*.h include/psa/*.h; do - case ${header#include/} in - mbedtls/mbedtls_config.h) :;; # not meant for direct inclusion - psa/crypto_config.h) :;; # not meant for direct inclusion - # Some of the psa/crypto_*.h headers are not meant to be included directly. - # They do have include guards that make them no-ops if psa/crypto.h - # has been included before. Since psa/crypto.h comes before psa/crypto_*.h - # in the wildcard enumeration, we don't need to skip those headers. - *) echo "#include \"${header#include/}\"";; - esac - done + for header in include/mbedtls/*.h include/psa/*.h; do + case ${header#include/} in + mbedtls/mbedtls_config.h) :;; # not meant for direct inclusion + psa/crypto_config.h) :;; # not meant for direct inclusion + # Some of the psa/crypto_*.h headers are not meant to be included + # directly. They do have include guards that make them no-ops if + # psa/crypto.h has been included before. Since psa/crypto.h comes + # before psa/crypto_*.h in the wildcard enumeration, we don't need + # to skip those headers. + *) echo "#include \"${header#include/}\"";; + esac + done - cat <<'EOF' + cat <<'EOF' int main() { @@ -72,14 +73,14 @@ EOF } if [ -d include/mbedtls ]; then - : + : elif [ -d ../include/mbedtls ]; then - cd .. + cd .. elif [ -d ../../include/mbedtls ]; then - cd ../.. + cd ../.. else - echo >&2 "This script must be run from an Mbed TLS source tree." - exit 3 + echo >&2 "This script must be run from an Mbed TLS source tree." + exit 3 fi print_cpp >"${1:-programs/test/cpp_dummy_build.cpp}" From 7530163f3b003e4851a48cb5661b1b1d4dbf6e6a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 5 Aug 2021 15:10:47 +0200 Subject: [PATCH 23/27] Make --quiet more effective when running make generated_files Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 2dc13756a8..7e8b2c3c20 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -752,7 +752,7 @@ pre_generate_files() { # file that might be around before generating fresh ones make neat if [ $QUIET -eq 1 ]; then - make -s generated_files + make generated_files >/dev/null else make generated_files fi From 86f6129067b7f2f6f6ff7aa1e1af69660e8f2d9a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 5 Aug 2021 15:11:33 +0200 Subject: [PATCH 24/27] Documentation improvement Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 7e8b2c3c20..6c322e29fc 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -242,7 +242,7 @@ General options: (default: "${ARM_NONE_EABI_GCC_PREFIX}") --armcc Run ARM Compiler builds (on by default). --error-test Error test mode: run a failing function in addition - to any specified component. + to any specified component. May be repeated. --except Exclude the COMPONENTs listed on the command line, instead of running only those. --no-append-outcome Write a new outcome file and analyze it (default). From 91e890e2fcefca43836dc174ad37906a6d6c996f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 5 Aug 2021 15:13:57 +0200 Subject: [PATCH 25/27] Add documentation Signed-off-by: Gilles Peskine --- programs/test/generate_cpp_dummy_build.sh | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/programs/test/generate_cpp_dummy_build.sh b/programs/test/generate_cpp_dummy_build.sh index 9e16348b52..94e911515d 100755 --- a/programs/test/generate_cpp_dummy_build.sh +++ b/programs/test/generate_cpp_dummy_build.sh @@ -1,5 +1,18 @@ #!/bin/sh +DEFAULT_OUTPUT_FILE=programs/test/cpp_dummy_build.cpp + +if [ "$1" = "--help" ]; then + cat <"${1:-programs/test/cpp_dummy_build.cpp}" +print_cpp >"${1:-$DEFAULT_OUTPUT_FILE}" From 03af67891194788791572fe892eb5d49c0ab9202 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 6 Aug 2021 11:35:17 +0200 Subject: [PATCH 26/27] Documentation improvements Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 6c322e29fc..26a5b7e5de 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -203,6 +203,8 @@ pre_initialize_variables () { # Test whether the component $1 is included in the command line patterns. is_component_included() { + # Temporarily disable wildcard expansion so that $COMMAND_LINE_COMPONENTS + # only does word splitting. set -f for pattern in $COMMAND_LINE_COMPONENTS; do set +f @@ -449,6 +451,8 @@ pre_parse_command_line () { # Error out if an explicitly requested component doesn't exist. if [ $all_except -eq 0 ]; then unsupported=0 + # Temporarily disable wildcard expansion so that $COMMAND_LINE_COMPONENTS + # only does word splitting. set -f for component in $COMMAND_LINE_COMPONENTS; do set +f @@ -552,6 +556,9 @@ pre_setup_keep_going () { # Whether it makes sense to keep a component going after the specified # command fails (test command) or not (configure or build). + # This function normally receives the failing simple command + # ($BASH_COMMAND) as an argument, but if $report_failed_command is set, + # this is passed instead. # This doesn't have to be 100% accurate: all failures are recorded anyway. # False positives result in running things that can't be expected to # work. False negatives result in things not running after something else From 80ddb991c2eab48bf5ed1d91ac0cb4a05facd143 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 6 Aug 2021 11:51:59 +0200 Subject: [PATCH 27/27] Add --restore option to clean up but not necessarily run components Signed-off-by: Gilles Peskine --- tests/scripts/all.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 26a5b7e5de..f963e4bf03 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -243,6 +243,9 @@ General options: Prefix for a cross-compiler for arm-none-eabi (default: "${ARM_NONE_EABI_GCC_PREFIX}") --armcc Run ARM Compiler builds (on by default). + --restore First clean up the build tree, restoring backed up + files. Do not run any components unless they are + explicitly specified. --error-test Error test mode: run a failing function in addition to any specified component. May be repeated. --except Exclude the COMPONENTs listed on the command line, @@ -388,6 +391,7 @@ pre_parse_command_line () { COMMAND_LINE_COMPONENTS= all_except=0 error_test=0 + restore_first=0 no_armcc= # Note that legacy options are ignored instead of being omitted from this @@ -426,6 +430,7 @@ pre_parse_command_line () { --quiet|-q) QUIET=1;; --random-seed) unset SEED;; --release-test|-r) SEED=$RELEASE_SEED;; + --restore) restore_first=1;; --seed|-s) shift; SEED="$1";; -*) echo >&2 "Unknown option: $1" @@ -438,7 +443,7 @@ pre_parse_command_line () { done # With no list of components, run everything. - if [ -z "$COMMAND_LINE_COMPONENTS" ]; then + if [ -z "$COMMAND_LINE_COMPONENTS" ] && [ $restore_first -eq 0 ]; then all_except=1 fi