From 5bf15b6d637ac2f2d49debb7f45521b478cedf0b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 26 Aug 2020 21:53:33 +0200 Subject: [PATCH 1/3] Reduce the use of grep Avoid using the external command grep for simple string-based checks. Prefer a case statement. This improves performance. The performance improvement is moderate but noticeable when skipping most tests. When a test is run, the cost of the associated grep calls is negligible. In this commit, I focused on the uses of grep that can be easily replaced and that are executed a large number of times. Signed-off-by: Gilles Peskine --- tests/ssl-opt.sh | 65 +++++++++++++++++++++++++++++------------------- 1 file changed, 39 insertions(+), 26 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index b9652ef42..e40c3150f 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -184,6 +184,14 @@ get_options() { done } +# Read boolean configuration options from config.h for easy and quick +# testing. Skip non-boolean options (with something other than spaces +# and a comment after "#define SYMBOL"). The variable contains a +# space-separated list of symbols. +CONFIGS_ENABLED=" $(<"$CONFIG_H" \ + sed -n 's!^ *#define *\([A-Za-z][0-9A-Z_a-z]*\) *\(/*\)*!\1!p' | + tr '\n' ' ')" + # Skip next test; use this macro to skip tests which are legitimate # in theory and expected to be re-introduced at some point, but # aren't expected to succeed at the moment due to problems outside @@ -194,16 +202,17 @@ skip_next_test() { # skip next test if the flag is not enabled in config.h requires_config_enabled() { - if grep "^#define $1" $CONFIG_H > /dev/null; then :; else - SKIP_NEXT="YES" - fi + case $CONFIGS_ENABLED in + *" $1 "*) :;; + *) SKIP_NEXT="YES";; + esac } # skip next test if the flag is enabled in config.h requires_config_disabled() { - if grep "^#define $1" $CONFIG_H > /dev/null; then - SKIP_NEXT="YES" - fi + case $CONFIGS_ENABLED in + *" $1 "*) SKIP_NEXT="YES";; + esac } get_config_value_or_default() { @@ -422,17 +431,21 @@ fail() { # is_polar is_polar() { - echo "$1" | grep 'ssl_server2\|ssl_client2' > /dev/null + case "$1" in + *ssl_client2*) true;; + *ssl_server2*) true;; + *) false;; + esac } # openssl s_server doesn't have -www with DTLS check_osrv_dtls() { - if echo "$SRV_CMD" | grep 's_server.*-dtls' >/dev/null; then - NEEDS_INPUT=1 - SRV_CMD="$( echo $SRV_CMD | sed s/-www// )" - else - NEEDS_INPUT=0 - fi + case "$SRV_CMD" in + *s_server*-dtls*) + NEEDS_INPUT=1 + SRV_CMD="$( echo $SRV_CMD | sed s/-www// )";; + *) NEEDS_INPUT=0;; + esac } # provide input to commands that need it @@ -548,11 +561,10 @@ wait_client_done() { # check if the given command uses dtls and sets global variable DTLS detect_dtls() { - if echo "$1" | grep 'dtls=1\|-dtls1\|-u' >/dev/null; then - DTLS=1 - else - DTLS=0 - fi + case "$1" in + *dtls=1*|-dtls|-u) DTLS=1;; + *) DTLS=0;; + esac } # Usage: run_test name [-p proxy_cmd] srv_cmd cli_cmd cli_exit [option [...]] @@ -577,10 +589,11 @@ run_test() { print_name "$NAME" # Do we only run numbered tests? - if [ "X$RUN_TEST_NUMBER" = "X" ]; then : - elif echo ",$RUN_TEST_NUMBER," | grep ",$TESTS," >/dev/null; then : - else - SKIP_NEXT="YES" + if [ -n "$RUN_TEST_NUMBER" ]; then + case ",$RUN_TEST_NUMBER," in + *",$TESTS,"*) :;; + *) SKIP_NEXT="YES";; + esac fi # should we skip? @@ -606,10 +619,10 @@ run_test() { shift 3 # Check if test uses files - TEST_USES_FILES=$(echo "$SRV_CMD $CLI_CMD" | grep "\.\(key\|crt\|pem\)" ) - if [ ! -z "$TEST_USES_FILES" ]; then - requires_config_enabled MBEDTLS_FS_IO - fi + case "$SRV_CMD $CLI_CMD" in + *data_files/*) + requires_config_enabled MBEDTLS_FS_IO;; + esac # should we skip? if [ "X$SKIP_NEXT" = "XYES" ]; then From b7bb068b843cdea9f19ef2449d21e23d740924d1 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 26 Aug 2020 22:35:46 +0200 Subject: [PATCH 2/3] Avoid using grep for test case names if possible If `$FILTER` (`-f`) and `$EXCLUDE` (`-e`) are simple selections that can be expressed as shell patterns, use a case statement instead of calling grep to determine whether a test case should be executed. Using a case statement significantly reduces the time it takes to determine that a test case is excluded (but the improvement is small compared to running the test). This noticeably speeds up running a single test or a small number of tests. Before: ``` tests/ssl-opt.sh -f Default 1.75s user 0.54s system 79% cpu 2.885 total ``` After: ``` tests/ssl-opt.sh -f Default 0.37s user 0.14s system 29% cpu 1.715 total ``` There is no perceptible difference when running a large number of tests. Signed-off-by: Gilles Peskine --- tests/ssl-opt.sh | 47 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 43 insertions(+), 4 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index e40c3150f..1be110da6 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -130,8 +130,8 @@ print_usage() { echo "Usage: $0 [options]" printf " -h|--help\tPrint this help.\n" printf " -m|--memcheck\tCheck memory leaks and errors.\n" - printf " -f|--filter\tOnly matching tests are executed (BRE)\n" - printf " -e|--exclude\tMatching tests are excluded (BRE)\n" + printf " -f|--filter\tOnly matching tests are executed (substring or BRE)\n" + printf " -e|--exclude\tMatching tests are excluded (substring or BRE)\n" printf " -n|--number\tExecute only numbered test (comma-separated, e.g. '245,256')\n" printf " -s|--show-numbers\tShow test numbers in front of test names\n" printf " -p|--preserve-logs\tPreserve logs of successful tests as well\n" @@ -580,8 +580,7 @@ run_test() { NAME="$1" shift 1 - if echo "$NAME" | grep "$FILTER" | grep -v "$EXCLUDE" >/dev/null; then : - else + if is_excluded "$NAME"; then SKIP_NEXT="NO" return fi @@ -850,6 +849,46 @@ cleanup() { get_options "$@" +# Optimize filters: if $FILTER and $EXCLUDE can be expressed as shell +# patterns rather than regular expressions, use a case statement instead +# of calling grep. To keep the optimizer simple, it is incomplete and only +# detects simple cases: plain substring, everything, nothing. +# +# As an exception, the character '.' is treated as an ordinary character +# if it is the only special character in the string. This is because it's +# rare to need "any one character", but needing a literal '.' is common +# (e.g. '-f "DTLS 1.2"'). +need_grep= +case "$FILTER" in + '^$') simple_filter=;; + '.*') simple_filter='*';; + *[][\$^+*?{|}]*) # Regexp special characters (other than .), we need grep + need_grep=1;; + *) # No regexp or shell-pattern special character + simple_filter="*$FILTER*";; +esac +case "$EXCLUDE" in + '^$') simple_exclude=;; + '.*') simple_exclude='*';; + *[][\$^+*?{|}]*) # Regexp special characters (other than .), we need grep + need_grep=1;; + *) # No regexp or shell-pattern special character + simple_exclude="*$EXCLUDE*";; +esac +if [ -n "$need_grep" ]; then + is_excluded () { + ! echo "$1" | grep "$FILTER" | grep -q -v "$EXCLUDE" + } +else + is_excluded () { + case "$1" in + $simple_exclude) true;; + $simple_filter) false;; + *) true;; + esac + } +fi + # sanity checks, avoid an avalanche of errors P_SRV_BIN="${P_SRV%%[ ]*}" P_CLI_BIN="${P_CLI%%[ ]*}" From c5714bb4eadc0b51d0c3087e8095ca38ab634e0e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 29 Sep 2020 23:48:39 +0200 Subject: [PATCH 3/3] Fix regexp detection In a case exprssion, `|` separates patterns so it needs to be quoted. Also `\` was not actually part of the set since it was quoting another character. Signed-off-by: Gilles Peskine --- tests/ssl-opt.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 1be110da6..f97f0870d 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -862,7 +862,7 @@ need_grep= case "$FILTER" in '^$') simple_filter=;; '.*') simple_filter='*';; - *[][\$^+*?{|}]*) # Regexp special characters (other than .), we need grep + *[][$+*?\\^{\|}]*) # Regexp special characters (other than .), we need grep need_grep=1;; *) # No regexp or shell-pattern special character simple_filter="*$FILTER*";; @@ -870,7 +870,7 @@ esac case "$EXCLUDE" in '^$') simple_exclude=;; '.*') simple_exclude='*';; - *[][\$^+*?{|}]*) # Regexp special characters (other than .), we need grep + *[][$+*?\\^{\|}]*) # Regexp special characters (other than .), we need grep need_grep=1;; *) # No regexp or shell-pattern special character simple_exclude="*$EXCLUDE*";;