From: Sergei Trofimovich <siarh...@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
