[PATCH] Fix --shuffle for SECONDEXPANSION prerequisites

[Sergei Trofimovich]

From: Sergei Trofimovich <siarheit@google.com>

Since commit 07eea3aa4 ("[SV 62706] Only second-expand targets that
might be built") `make --shuffle` stopped shuffling prerequisites that
use .SECONDEXPANSION feature for Makefiles like:

    .SECONDEXPANSION:
    all: $$(var)
    %_: ; @echo $@
    var = a_ b_ c_

Shuffle does not happen anymore because second expansion now happens
after shuffle phase. This has two problems:
1. No shuffling happens for such prerequisites.
2. Already freed data is accessed when when oudated '->shuf' links
   are used.

The fix inserts reshuffle into expansion phase right after dependency
changes to fix both problems.

* src/file.c (expand_deps): Add reshuffling phase if dependency updates.
* 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..83aa593c 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).  Here we
+       regenerate '->shuf'.  Otherwise we risk referring 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 ea6e836d..23c9c224 100644
--- a/src/shuffle.c
+++ b/src/shuffle.c
@@ -154,7 +154,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;
-- 
2.37.2