From 24bdf02e4e08734d98a87c29af4c19decac86367 Mon Sep 17 00:00:00 2001
From: Gilles Peskine <Gilles.Peskine@arm.com>
Date: Mon, 30 Mar 2020 20:11:39 +0200
Subject: [PATCH] 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 <Gilles.Peskine@arm.com>
---
 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 d4408ea46..016aa0062 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/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
@@ -289,13 +294,18 @@ cleanup()
     rm -f programs/test/cmake_subproject/Makefile
     rm -f programs/test/cmake_subproject/cmake_subproject
 
-    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.
@@ -304,7 +314,7 @@ final_report () {
 }
 
 fatal_signal () {
-    cleanup
+    final_cleanup
     final_report $1
     trap - $1
     kill -$1 $$
@@ -480,6 +490,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
@@ -2825,7 +2841,7 @@ component_check_generate_test_code () {
 
 post_report () {
     msg "Done, cleaning up"
-    cleanup
+    final_cleanup
 
     final_report
 }
@@ -2851,10 +2867,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"
 
@@ -2909,6 +2921,7 @@ pre_initialize_variables
 pre_parse_command_line "$@"
 
 pre_check_git
+pre_back_up
 
 build_status=0
 if [ $KEEP_GOING -eq 1 ]; then