Skip to content

Commit 7157407

Browse files
authored
Make build_id.lo more random (#58258)
This does not fix the underlying issue that can occur here which is a collision of build_ids.lo between modules in IR decompression. Fixing that requires a somewhat significant overhaul to the serialization of IR (probably using the module identity as a key). This does mean we use a lot more of the bits available here so it makes collisions a lot less likely( they were already extremely rare) but hrtime does tend to only use the lower bits of a 64 bit integer and this will hopefully add some more randomness and make this even less likely
1 parent 7b64cec commit 7157407

File tree

3 files changed

+22
-3
lines changed

3 files changed

+22
-3
lines changed

src/module.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -483,7 +483,8 @@ static jl_module_t *jl_new_module__(jl_sym_t *name, jl_module_t *parent)
483483
m->istopmod = 0;
484484
m->uuid = uuid_zero;
485485
static unsigned int mcounter; // simple counter backup, in case hrtime is not incrementing
486-
m->build_id.lo = jl_hrtime() + (++mcounter);
486+
// TODO: this is used for ir decompression and is liable to hash collisions so use more of the bits
487+
m->build_id.lo = bitmix(jl_hrtime() + (++mcounter), jl_rand());
487488
if (!m->build_id.lo)
488489
m->build_id.lo++; // build id 0 is invalid
489490
m->build_id.hi = ~(uint64_t)0;

src/staticdata.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4410,7 +4410,11 @@ static jl_value_t *jl_restore_package_image_from_stream(ios_t *f, jl_image_t *im
44104410
JL_SIGATOMIC_END();
44114411

44124412
// Add roots to methods
4413-
jl_copy_roots(method_roots_list, jl_worklist_key((jl_array_t*)restored));
4413+
int failed = jl_copy_roots(method_roots_list, jl_worklist_key((jl_array_t*)restored));
4414+
if (failed != 0) {
4415+
jl_printf(JL_STDERR, "Error copying roots to methods from Module: %s\n", pkgname);
4416+
abort();
4417+
}
44144418
// Insert method extensions and handle edges
44154419
int new_methods = jl_array_nrows(extext_methods) > 0;
44164420
if (!new_methods) {

src/staticdata_utils.c

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -736,17 +736,31 @@ static void jl_activate_methods(jl_array_t *external, jl_array_t *internal, size
736736
}
737737
}
738738

739-
static void jl_copy_roots(jl_array_t *method_roots_list, uint64_t key)
739+
static int jl_copy_roots(jl_array_t *method_roots_list, uint64_t key)
740740
{
741741
size_t i, l = jl_array_nrows(method_roots_list);
742+
int failed = 0;
742743
for (i = 0; i < l; i+=2) {
743744
jl_method_t *m = (jl_method_t*)jl_array_ptr_ref(method_roots_list, i);
744745
jl_array_t *roots = (jl_array_t*)jl_array_ptr_ref(method_roots_list, i+1);
745746
if (roots) {
746747
assert(jl_is_array(roots));
748+
if (m->root_blocks) {
749+
// check for key collision
750+
uint64_t *blocks = jl_array_data(m->root_blocks, uint64_t);
751+
size_t nx2 = jl_array_nrows(m->root_blocks);
752+
for (size_t i = 0; i < nx2; i+=2) {
753+
if (blocks[i] == key) {
754+
// found duplicate block
755+
failed = -1;
756+
}
757+
}
758+
}
759+
747760
jl_append_method_roots(m, key, roots);
748761
}
749762
}
763+
return failed;
750764
}
751765

752766
static jl_value_t *read_verify_mod_list(ios_t *s, jl_array_t *depmods)

0 commit comments

Comments
 (0)