bpart: Fix a hang in a particular corner case (#58271)

The `jl_get_binding_partition_with_hint` version of the bpart lookup did
not properly set the `max_world` of the gap, leaving it at `~(size_t)0`.
Having a `gap` with that `max_world`, but with `gap.replace` set is
inconsistent and ended up causing an integer overflow downstream in the
computation of `expected_prev_min_world`, which looked like a concurrent
modification causing an infinite retry. Fixes #58257

Author: Keno

Compiler/test/inference.jl

@@ -6428,4 +6428,21 @@ global invalid_setglobal!_exct_modeling::Int
     setglobal!(@__MODULE__, :invalid_setglobal!_exct_modeling, x)
 end == ErrorException
 
+# Issue #58257 - Hang in inference during BindingPartition resolution
+module A58257
+    module B58257
+        using ..A58257
+        # World age here is N
+    end
+    using .B58257
+    # World age here is N+1
+    @eval f() = $(GlobalRef(B58257, :get!))
+end
+
+## The sequence of events is critical here.
+A58257.get!      # Creates binding partition in A, N+1:∞
+A58257.B58257.get!    # Creates binding partition in A.B, N+1:∞
+Base.invoke_in_world(UInt(38678), getglobal, A58257, :get!) # Expands binding partition in A through <N
+@test Base.infer_return_type(A58257.f) == typeof(Base.get!) # Attempt to lookup A.B in world age N hangs
+
 end # module inference

src/module.c

@@ -136,15 +136,18 @@ static jl_binding_partition_t *jl_implicit_import_resolved(jl_binding_t *b, stru
     if (jl_is_binding_partition(gap.parent)) {
         // Check if we can merge this into the previous binding partition
         jl_binding_partition_t *prev = (jl_binding_partition_t *)gap.parent;
+        assert(new_max_world != ~(size_t)0); // It is inconsistent to have a gap with `gap.parent` set, but max_world == ~(size_t)0
         size_t expected_prev_min_world = new_max_world + 1;
         if (prev->restriction == resolution.binding_or_const && prev->kind == new_kind) {
+retry:
             if (!jl_atomic_cmpswap(&prev->min_world, &expected_prev_min_world, new_min_world)) {
                 if (expected_prev_min_world <= new_min_world) {
                     return prev;
                 }
                 else if (expected_prev_min_world <= new_max_world) {
-                    // Concurrent modification by another thread - bail.
-                    return NULL;
+                    // Concurrent modification of the partition. However, our lookup is still valid,
+                    // so we should still be able to extend the partition.
+                    goto retry;
                 }
                 // There remains a gap - proceed
             } else {
@@ -154,7 +157,7 @@ static jl_binding_partition_t *jl_implicit_import_resolved(jl_binding_t *b, stru
                     for (;;) {
                         // We've updated the previous partition - check if we've closed a gap
                         size_t next_max_world = jl_atomic_load_relaxed(&next->max_world);
-                        if (next_max_world == expected_prev_min_world-1 && next->kind == new_kind && next->restriction == resolution.binding_or_const) {
+                        if (next_max_world >= expected_prev_min_world-1 && next->kind == new_kind && next->restriction == resolution.binding_or_const) {
                             if (jl_atomic_cmpswap(&prev->min_world, &expected_prev_min_world, next_min_world)) {
                                 jl_binding_partition_t *nextnext = jl_atomic_load_relaxed(&next->next);
                                 if (!jl_atomic_cmpswap(&prev->next, &next, nextnext)) {
@@ -370,15 +373,15 @@ JL_DLLEXPORT void jl_update_loaded_bpart(jl_binding_t *b, jl_binding_partition_t
     bpart->kind = resolution.ultimate_kind;
 }
 
-STATIC_INLINE jl_binding_partition_t *jl_get_binding_partition_(jl_binding_t *b JL_PROPAGATES_ROOT, jl_value_t *parent, _Atomic(jl_binding_partition_t *)*insert, size_t world, modstack_t *st) JL_GLOBALLY_ROOTED
+STATIC_INLINE jl_binding_partition_t *jl_get_binding_partition_(jl_binding_t *b JL_PROPAGATES_ROOT, jl_value_t *parent, _Atomic(jl_binding_partition_t *)*insert, size_t world, size_t max_world, modstack_t *st) JL_GLOBALLY_ROOTED
 {
     assert(jl_is_binding(b));
     struct implicit_search_gap gap;
     gap.parent = parent;
     gap.insert = insert;
     gap.inherited_flags = 0;
     gap.min_world = 0;
-    gap.max_world = ~(size_t)0;
+    gap.max_world = max_world;
     while (1) {
         gap.replace = jl_atomic_load_relaxed(gap.insert);
         jl_binding_partition_t *bpart = jl_get_binding_partition__(b, world, &gap);
@@ -395,15 +398,14 @@ jl_binding_partition_t *jl_get_binding_partition(jl_binding_t *b, size_t world)
     if (!b)
         return NULL;
     // Duplicate the code for the entry frame for branch prediction
-    return jl_get_binding_partition_(b, (jl_value_t*)b, &b->partitions, world, NULL);
+    return jl_get_binding_partition_(b, (jl_value_t*)b, &b->partitions, world, ~(size_t)0, NULL);
 }
 
 jl_binding_partition_t *jl_get_binding_partition_with_hint(jl_binding_t *b, jl_binding_partition_t *prev, size_t world) JL_GLOBALLY_ROOTED {
     // Helper for getting a binding partition for an older world after we've already looked up the partition for a newer world
     assert(b);
-    // TODO: Is it possible for a concurrent lookup to have expanded this bpart, making this false?
-    assert(jl_atomic_load_relaxed(&prev->min_world) > world);
-    return jl_get_binding_partition_(b, (jl_value_t*)prev, &prev->next, world, NULL);
+    size_t prev_min_world = jl_atomic_load_relaxed(&prev->min_world);
+    return jl_get_binding_partition_(b, (jl_value_t*)prev, &prev->next, world, prev_min_world-1, NULL);
 }
 
 jl_binding_partition_t *jl_get_binding_partition_all(jl_binding_t *b, size_t min_world, size_t max_world) {