[SV 67046] Don't modify the hash table when inside hash_map*()
It's illegal for the user-supplied function to modify the hash table
while executing hash_map*() methods. snap_file() may add elements
due to EXTRA_PREREQS, so we can't call it via hash_map_args().
Diagnosis and test by Shim Manning <shimmanning@gmail.com>
Possible fix by Dmitry Goncharov <dgoncharov@users.sf.net>
* src/hash.h (hash_table): Remember when we're in hash_map*().
* src/hash.c (hash_init): Initialize the in-map value to false.
(hash_map): Set the in-map value to true when walking the map.
(hash_map_arg): Ditto.
(hash_insert_at): Assert we're not in hash_map*()
(hash_free_items): Ditto.
(hash_delete_items): Ditto.
(hash_free): Ditto.
* src/file.c (snap_deps): Use hash_dump() to create a copy of the
hash and walk that to call snap_file(), so we can add more files.
(snap_file): Since we're calling it directly, don't use void* args.
diff --git a/src/file.c b/src/file.c
index 65517d0..447662a 100644
--- a/src/file.c
+++ b/src/file.c
@@ -723,9 +723,8 @@
/* Perform per-file snap operations. */
static void
-snap_file (const void *item, void *arg)
+snap_file (struct file *f, const struct dep *deps)
{
- struct file *f = (struct file*)item;
struct dep *prereqs = NULL;
struct dep *d;
@@ -759,7 +758,7 @@
}
}
else if (f->is_target)
- prereqs = copy_dep_chain (arg);
+ prereqs = copy_dep_chain (deps);
if (prereqs)
{
@@ -914,9 +913,16 @@
{
struct dep *prereqs = expand_extra_prereqs (lookup_variable (STRING_SIZE_TUPLE(".EXTRA_PREREQS")));
- /* Perform per-file snap operations. */
- hash_map_arg(&files, snap_file, prereqs);
+ /* Perform per-file snap operations.
+ We can't use hash_map*() here because snap_file may add new elements
+ into the files hash, which is not allowed in the map. Instead make a
+ dump of the files and walk through that. */
+ void** filedump = hash_dump (&files, NULL, 0);
+ for (void** filep = filedump; *filep; ++filep)
+ snap_file (*filep, prereqs);
+
+ free (filedump);
free_dep_chain (prereqs);
}
diff --git a/src/hash.c b/src/hash.c
index a118552..a0b370f 100644
--- a/src/hash.c
+++ b/src/hash.c
@@ -59,6 +59,7 @@
ht->ht_collisions = 0;
ht->ht_lookups = 0;
ht->ht_rehashes = 0;
+ ht->ht_in_map = 0;
ht->ht_hash_1 = hash_1;
ht->ht_hash_2 = hash_2;
ht->ht_compare = hash_cmp;
@@ -138,6 +139,10 @@
hash_insert_at (struct hash_table *ht, const void *item, const void *slot)
{
const void *old_item = *(void **) slot;
+
+ /* It's illegal to insert while in hash_map*(). */
+ assert (! ht->ht_in_map);
+
if (HASH_VACANT (old_item))
{
ht->ht_fill++;
@@ -181,6 +186,10 @@
{
void **vec = ht->ht_vec;
void **end = &vec[ht->ht_size];
+
+ /* It's illegal to free items while in hash_map*(). */
+ assert (! ht->ht_in_map);
+
for (; vec < end; vec++)
{
void *item = *vec;
@@ -197,6 +206,10 @@
{
void **vec = ht->ht_vec;
void **end = &vec[ht->ht_size];
+
+ /* It's illegal to delete all items while in hash_map*(). */
+ assert (! ht->ht_in_map);
+
for (; vec < end; vec++)
*vec = 0;
ht->ht_fill = 0;
@@ -209,6 +222,9 @@
void
hash_free (struct hash_table *ht, int free_items)
{
+ /* It's illegal to free while in hash_map*(). */
+ assert (! ht->ht_in_map);
+
if (free_items)
hash_free_items (ht);
else
@@ -227,11 +243,15 @@
void **slot;
void **end = &ht->ht_vec[ht->ht_size];
+ ht->ht_in_map = 1;
+
for (slot = ht->ht_vec; slot < end; slot++)
{
if (!HASH_VACANT (*slot))
(*map) (*slot);
}
+
+ ht->ht_in_map = 0;
}
void
@@ -240,11 +260,15 @@
void **slot;
void **end = &ht->ht_vec[ht->ht_size];
+ ht->ht_in_map = 1;
+
for (slot = ht->ht_vec; slot < end; slot++)
{
if (!HASH_VACANT (*slot))
(*map) (*slot, arg);
}
+
+ ht->ht_in_map = 0;
}
/* Double the size of the hash table in the event of overflow... */
diff --git a/src/hash.h b/src/hash.h
index 1bbda0f..27b202a 100644
--- a/src/hash.h
+++ b/src/hash.h
@@ -51,6 +51,7 @@
unsigned long ht_collisions; /* # of failed calls to comparison function */
unsigned long ht_lookups; /* # of queries */
unsigned int ht_rehashes; /* # of times we've expanded table */
+ unsigned int ht_in_map:1; /* 1 if we're inside a hash_map*() function */
};
typedef int (*qsort_cmp_t) __P((void const *, void const *));
diff --git a/tests/scripts/variables/EXTRA_PREREQS b/tests/scripts/variables/EXTRA_PREREQS
index 2f72be3..a34abd9 100644
--- a/tests/scripts/variables/EXTRA_PREREQS
+++ b/tests/scripts/variables/EXTRA_PREREQS
@@ -178,7 +178,16 @@
',
'', "hello.x.d\nhello.x\nworld.x.d\nworld.x\n");
+# SV 67046. Rehashing of hash table files during iteration.
+my $files = "";
+for my $k (1 .. 1000) {
+ $files = "$files file$k";
+}
+run_make_test(qq!
+all: .EXTRA_PREREQS := $files
+all:;
+file%:;
+!,
+ '', "#MAKE#: 'all' is up to date.");
-
-# This tells the test driver that the perl test script executed properly.
1;
