Skip to content

Commit 87024fb

Browse files
gbaraldiKristofferC
authored andcommitted
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 (cherry picked from commit 7157407)
1 parent 52125d7 commit 87024fb

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
@@ -24,7 +24,8 @@ JL_DLLEXPORT jl_module_t *jl_new_module_(jl_sym_t *name, jl_module_t *parent, ui
2424
m->istopmod = 0;
2525
m->uuid = uuid_zero;
2626
static unsigned int mcounter; // simple counter backup, in case hrtime is not incrementing
27-
m->build_id.lo = jl_hrtime() + (++mcounter);
27+
// TODO: this is used for ir decompression and is liable to hash collisions so use more of the bits
28+
m->build_id.lo = bitmix(jl_hrtime() + (++mcounter), jl_rand());
2829
if (!m->build_id.lo)
2930
m->build_id.lo++; // build id 0 is invalid
3031
m->build_id.hi = ~(uint64_t)0;

src/staticdata.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3475,7 +3475,11 @@ static jl_value_t *jl_restore_package_image_from_stream(void* pkgimage_handle, i
34753475
jl_insert_methods(extext_methods);
34763476
// No special processing of `new_specializations` is required because recaching handled it
34773477
// Add roots to methods
3478-
jl_copy_roots(method_roots_list, jl_worklist_key((jl_array_t*)restored));
3478+
int failed = jl_copy_roots(method_roots_list, jl_worklist_key((jl_array_t*)restored));
3479+
if (failed != 0) {
3480+
jl_printf(JL_STDERR, "Error copying roots to methods from Module: %s\n", pkgname);
3481+
abort();
3482+
}
34793483
// Handle edges
34803484
size_t world = jl_atomic_load_acquire(&jl_world_counter);
34813485
jl_insert_backedges((jl_array_t*)edges, (jl_array_t*)ext_targets, (jl_array_t*)new_specializations, world); // restore external backedges (needs to be last)

src/staticdata_utils.c

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -847,17 +847,31 @@ static void jl_insert_methods(jl_array_t *list)
847847
}
848848
}
849849

850-
static void jl_copy_roots(jl_array_t *method_roots_list, uint64_t key)
850+
static int jl_copy_roots(jl_array_t *method_roots_list, uint64_t key)
851851
{
852852
size_t i, l = jl_array_len(method_roots_list);
853+
int failed = 0;
853854
for (i = 0; i < l; i+=2) {
854855
jl_method_t *m = (jl_method_t*)jl_array_ptr_ref(method_roots_list, i);
855856
jl_array_t *roots = (jl_array_t*)jl_array_ptr_ref(method_roots_list, i+1);
856857
if (roots) {
857858
assert(jl_is_array(roots));
859+
if (m->root_blocks) {
860+
// check for key collision
861+
uint64_t *blocks = (uint64_t*)jl_array_data(m->root_blocks);
862+
size_t nx2 = jl_array_nrows(m->root_blocks);
863+
for (size_t i = 0; i < nx2; i+=2) {
864+
if (blocks[i] == key) {
865+
// found duplicate block
866+
failed = -1;
867+
}
868+
}
869+
}
870+
858871
jl_append_method_roots(m, key, roots);
859872
}
860873
}
874+
return failed;
861875
}
862876

863877

0 commit comments

Comments
 (0)