From ca4234c4b550618df2194e0617c43bb12524f820 Mon Sep 17 00:00:00 2001
From: Sergei Trofimovich <siarheit@google.com>
Date: Sun, 11 Sep 2022 21:28:59 +0100
Subject: [PATCH] [SV 63047] Fix shuffle of SECONDEXPANSION prerequisites

Commit 07eea3aa4 `make --shuffle` prevented shuffling prerequisites
that use .SECONDEXPANSION, since shuffle happens before expansion.
This has two problems:
1. No shuffling happens for such prerequisites.
2. Use-after-free when outdated '->shuf' links are used.

Add a reshuffle into expansion phase right after dependency changes.

* src/file.c (expand_deps): Add reshuffle if dependencies change.
* src/shuffle.c (identity_shuffle_array): Fix comment typo.
* tests/scripts/options/shuffle: Add new SECONDEXPANSION test.
---
 src/file.c                    | 10 ++++++++++
 src/shuffle.c                 |  2 +-
 tests/scripts/options/shuffle |  9 +++++++++
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/src/file.c b/src/file.c
index 7596ff10..2cb013ef 100644
--- a/src/file.c
+++ b/src/file.c
@@ -25,6 +25,7 @@ this program.  If not, see <http://www.gnu.org/licenses/>.  */
 #include "variable.h"
 #include "debug.h"
 #include "hash.h"
+#include "shuffle.h"
 
 
 /* Remember whether snap_deps has been invoked: we need this to be sure we
@@ -576,6 +577,7 @@ expand_deps (struct file *f)
   struct dep **dp;
   const char *fstem;
   int initialized = 0;
+  int changed_dep = 0;
 
   if (f->snapped)
     return;
@@ -664,6 +666,7 @@ expand_deps (struct file *f)
       if (new == 0)
         {
           *dp = d->next;
+          changed_dep = 1;
           free_dep (d);
           d = *dp;
           continue;
@@ -672,6 +675,7 @@ expand_deps (struct file *f)
       /* Add newly parsed prerequisites.  */
       fstem = d->stem;
       next = d->next;
+      changed_dep = 1;
       free_dep (d);
       *dp = new;
       for (dp = &new, d = new; d != 0; dp = &d->next, d = d->next)
@@ -688,6 +692,12 @@ expand_deps (struct file *f)
       *dp = next;
       d = *dp;
     }
+
+    /* Shuffle mode assumes '->next' and '->shuf' links both traverse the same
+       dependencies (in different sequences).  Regenerate '->shuf' so we don't
+       refer to stale data.  */
+    if (changed_dep)
+      shuffle_deps_recursive (f->deps);
 }
 
 /* Add extra prereqs to the file in question.  */
diff --git a/src/shuffle.c b/src/shuffle.c
index d78bfd8f..ca54e528 100644
--- a/src/shuffle.c
+++ b/src/shuffle.c
@@ -146,7 +146,7 @@ identity_shuffle_array (void **a UNUSED, size_t len UNUSED)
   /* No-op!  */
 }
 
-/* Shuffle list of dependencies by populating '->next'
+/* Shuffle list of dependencies by populating '->shuf'
    field in each 'struct dep'.  */
 static void
 shuffle_deps (struct dep *deps)
diff --git a/tests/scripts/options/shuffle b/tests/scripts/options/shuffle
index e037ed1a..5661683c 100644
--- a/tests/scripts/options/shuffle
+++ b/tests/scripts/options/shuffle
@@ -116,4 +116,13 @@ run_make_test('
 ',
               '--shuffle=reverse a_ b_ c_', "a_\nb_\nc_");
 
+# Check if SECONDEXPANSION targets also get reshuffled.
+run_make_test('
+.SECONDEXPANSION:
+all: $$(var)
+%_: ; @echo $@
+var = a_ b_ c_
+',
+              '--shuffle=reverse', "c_\nb_\na_");
+
 1;