From a69aaab6c4aeb6eebf3ebf06ea5886b2f7c1fb24 Mon Sep 17 00:00:00 2001 From: Gabriel Baraldi Date: Sat, 3 May 2025 12:33:46 -0300 Subject: [PATCH] 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 --- src/module.c | 3 ++- src/staticdata.c | 6 +++++- src/staticdata_utils.c | 16 +++++++++++++++- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/module.c b/src/module.c index d39b7e377b2df..173f4dacf2769 100644 --- a/src/module.c +++ b/src/module.c @@ -24,7 +24,8 @@ JL_DLLEXPORT jl_module_t *jl_new_module_(jl_sym_t *name, jl_module_t *parent, ui m->istopmod = 0; m->uuid = uuid_zero; static unsigned int mcounter; // simple counter backup, in case hrtime is not incrementing - m->build_id.lo = jl_hrtime() + (++mcounter); + // TODO: this is used for ir decompression and is liable to hash collisions so use more of the bits + m->build_id.lo = bitmix(jl_hrtime() + (++mcounter), jl_rand()); if (!m->build_id.lo) m->build_id.lo++; // build id 0 is invalid m->build_id.hi = ~(uint64_t)0; diff --git a/src/staticdata.c b/src/staticdata.c index 13284384e0485..ad62adb4ba178 100644 --- a/src/staticdata.c +++ b/src/staticdata.c @@ -3449,7 +3449,11 @@ static jl_value_t *jl_restore_package_image_from_stream(void* pkgimage_handle, i // No special processing of `new_specializations` is required because recaching handled it // Add roots to methods - jl_copy_roots(method_roots_list, jl_worklist_key((jl_array_t*)restored)); + int failed = jl_copy_roots(method_roots_list, jl_worklist_key((jl_array_t*)restored)); + if (failed != 0) { + jl_printf(JL_STDERR, "Error copying roots to methods from Module: %s\n", pkgname); + abort(); + } // Insert method extensions jl_insert_methods(extext_methods); // Handle edges diff --git a/src/staticdata_utils.c b/src/staticdata_utils.c index 807fa2196ddf7..f08ca21017700 100644 --- a/src/staticdata_utils.c +++ b/src/staticdata_utils.c @@ -817,17 +817,31 @@ static void jl_insert_methods(jl_array_t *list) } } -static void jl_copy_roots(jl_array_t *method_roots_list, uint64_t key) +static int jl_copy_roots(jl_array_t *method_roots_list, uint64_t key) { size_t i, l = jl_array_len(method_roots_list); + int failed = 0; for (i = 0; i < l; i+=2) { jl_method_t *m = (jl_method_t*)jl_array_ptr_ref(method_roots_list, i); jl_array_t *roots = (jl_array_t*)jl_array_ptr_ref(method_roots_list, i+1); if (roots) { assert(jl_is_array(roots)); + if (m->root_blocks) { + // check for key collision + uint64_t *blocks = (uint64_t*)jl_array_data(m->root_blocks); + size_t nx2 = jl_array_nrows(m->root_blocks); + for (size_t i = 0; i < nx2; i+=2) { + if (blocks[i] == key) { + // found duplicate block + failed = -1; + } + } + } + jl_append_method_roots(m, key, roots); } } + return failed; }