Merge pull request #1462 from MoarVM/fix_fixkey_hash_memory_corruption

Fix corruption of fixkey hash entries
MoarVM · May 8, 2021 · 2d14aa9 · 2d14aa9
2 parents 684b240 + c7a063e
commit 2d14aa9
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 10 deletions.
diff --git a/src/core/fixkey_hash_table.c b/src/core/fixkey_hash_table.c
@@ -10,7 +10,7 @@ MVM_STATIC_INLINE MVMuint32 calc_entries_in_use(const struct MVMFixKeyHashTableC
 void hash_demolish_internal(MVMThreadContext *tc,
                             struct MVMFixKeyHashTableControl *control) {
     size_t allocated_items = MVM_fixkey_hash_allocated_items(control);
-    size_t entries_size = sizeof(MVMString ***) * allocated_items;
+    size_t entries_size = control->entry_size * allocated_items;
     size_t metadata_size = MVM_hash_round_size_up(allocated_items + 1);
     size_t total_size
         = entries_size + sizeof(struct MVMFixKeyHashTableControl) + metadata_size;
@@ -36,7 +36,7 @@ void MVM_fixkey_hash_demolish(MVMThreadContext *tc, MVMFixKeyHashTable *hashtabl
         }
         ++bucket;
         ++metadata;
-        entry_raw -= sizeof(MVMString ***);
+        entry_raw -= control->entry_size;
     }
 
     hash_demolish_internal(tc, control);
@@ -58,7 +58,7 @@ MVM_STATIC_INLINE struct MVMFixKeyHashTableControl *hash_allocate_common(MVMThre
         max_probe_distance_limit = max_items;
     }
     size_t allocated_items = official_size + max_probe_distance_limit - 1;
-    size_t entries_size = sizeof(MVMString ***) * allocated_items;
+    size_t entries_size = entry_size * allocated_items;
     size_t metadata_size = MVM_hash_round_size_up(allocated_items + 1);
     size_t total_size
         = entries_size + sizeof(struct MVMFixKeyHashTableControl) + metadata_size;
@@ -140,8 +140,8 @@ MVM_STATIC_INLINE MVMString ***hash_insert_internal(MVMThreadContext *tc,
                 MVMuint32 entries_to_move = find_me_a_gap - ls.metadata;
                 size_t size_to_move = ls.entry_size * entries_to_move;
                 /* When we had entries *ascending* this was
-                 * memmove(entry_raw + sizeof(MVMString ***), entry_raw,
-                 *         sizeof(MVMString ***) * entries_to_move);
+                 * memmove(entry_raw + ls.entry_size, entry_raw,
+                 *         ls.entry_size * entries_to_move);
                  * because we point to the *start* of the block of memory we
                  * want to move, and we want to move it one "entry" forwards.
                  * `entry_raw` is still a pointer to where we want to make free
@@ -256,11 +256,12 @@ static struct MVMFixKeyHashTableControl *maybe_grow_hash(MVMThreadContext *tc,
     MVMuint32 entries_in_use = calc_entries_in_use(control);
     MVMuint8 *entry_raw_orig = MVM_fixkey_hash_entries(control);
     MVMuint8 *metadata_orig = MVM_fixkey_hash_metadata(control);
+    MVMuint8 entry_size = control->entry_size;
 
     struct MVMFixKeyHashTableControl *control_orig = control;
 
     control = hash_allocate_common(tc,
-                                   control_orig->entry_size,
+                                   entry_size,
                                    control_orig->key_right_shift - 1,
                                    control_orig->official_size_log2 + 1);
 
@@ -298,7 +299,7 @@ static struct MVMFixKeyHashTableControl *maybe_grow_hash(MVMThreadContext *tc,
         }
         ++bucket;
         ++metadata;
-        entry_raw -= sizeof(MVMString ***);
+        entry_raw -= entry_size;
     }
     hash_demolish_internal(tc, control_orig);
     return control;
@@ -420,7 +421,7 @@ MVMuint64 MVM_fixkey_hash_fsck(MVMThreadContext *tc, MVMFixKeyHashTable *hashtab
         }
         ++bucket;
         ++metadata;
-        entry_raw -= sizeof(MVMString ***);
+        entry_raw -= control->entry_size;
     }
     if (*metadata != 1) {
         ++errors;

diff --git a/src/core/fixkey_hash_table_funcs.h b/src/core/fixkey_hash_table_funcs.h
@@ -25,7 +25,7 @@ MVM_STATIC_INLINE MVMuint8 *MVM_fixkey_hash_metadata(const struct MVMFixKeyHashT
     return (MVMuint8 *) control + sizeof(struct MVMFixKeyHashTableControl);
 }
 MVM_STATIC_INLINE MVMuint8 *MVM_fixkey_hash_entries(const struct MVMFixKeyHashTableControl *control) {
-    return (MVMuint8 *) control - sizeof(MVMString ***);
+    return (MVMuint8 *) control - control->entry_size;
 }
 
 /* Frees the entire contents of the hash, leaving you just the hashtable itself,
@@ -58,7 +58,7 @@ MVM_fixkey_hash_create_loop_state(MVMThreadContext *tc,
                                   MVMString *key) {
     MVMuint64 hash_val = MVM_string_hash_code(tc, key);
     struct MVM_hash_loop_state retval;
-    retval.entry_size = sizeof(MVMString ***);
+    retval.entry_size = control->entry_size;
     retval.metadata_increment = 1 << control->metadata_hash_bits;
     retval.metadata_hash_mask = retval.metadata_increment - 1;
     retval.probe_distance_shift = control->metadata_hash_bits;