staticdata: Insert backedges recursively (#57212)

In the new edges scheme, when we perform constant propagation, we create
a dummy CodeInstance that is not inserted into the specializations
cache, but instead simply serves as a container for all edges that were
encountered during the constant propagation. These CodeInstances are not
part of either the internal or external edges list collected during
pkgimage generation. As such, while we were verifying edges recursively
and would thus catch an invalidation prior to package image reload, we
were failing to also insert backedges for these CodeInstance. We were
thus failing to invalidate such methods if the method was redefined
after re-load of the package image. Fix that by moving the storing of
the backedges to the end of the validation code, so that it too happens
recursively.

Author: Keno

base/staticdata.jl

@@ -39,26 +39,16 @@ function _insert_backedges(edges::Vector{Any}, stack::Vector{CodeInstance}, visi
         verify_method_graph(codeinst, stack, visiting)
         minvalid = codeinst.min_world
         maxvalid = codeinst.max_world
-        if maxvalid ≥ minvalid
-            if get_world_counter() == maxvalid
-                # if this callee is still valid, add all the backedges
-                Base.Compiler.store_backedges(codeinst, codeinst.edges)
-            end
-            if get_world_counter() == maxvalid
-                maxvalid = typemax(UInt)
-                @atomic :monotonic codeinst.max_world = maxvalid
-            end
-            if external
-                caller = get_ci_mi(codeinst)
-                @assert isdefined(codeinst, :inferred) # See #53586, #53109
-                inferred = @ccall jl_rettype_inferred(
-                    codeinst.owner::Any, caller::Any, minvalid::UInt, maxvalid::UInt)::Any
-                if inferred !== nothing
-                    # We already got a code instance for this world age range from
-                    # somewhere else - we don't need this one.
-                else
-                    @ccall jl_mi_cache_insert(caller::Any, codeinst::Any)::Cvoid
-                end
+        if maxvalid ≥ minvalid && external
+            caller = get_ci_mi(codeinst)
+            @assert isdefined(codeinst, :inferred) # See #53586, #53109
+            inferred = @ccall jl_rettype_inferred(
+                codeinst.owner::Any, caller::Any, minvalid::UInt, maxvalid::UInt)::Any
+            if inferred !== nothing
+                # We already got a code instance for this world age range from
+                # somewhere else - we don't need this one.
+            else
+                @ccall jl_mi_cache_insert(caller::Any, codeinst::Any)::Cvoid
             end
         end
     end
@@ -196,9 +186,14 @@ function verify_method(codeinst::CodeInstance, stack::Vector{CodeInstance}, visi
     while length(stack) ≥ depth
         child = pop!(stack)
         if maxworld ≠ 0
-            @atomic  :monotonic child.min_world = minworld
+            @atomic :monotonic child.min_world = minworld
+        end
+        if maxworld == current_world
+            Base.Compiler.store_backedges(child, child.edges)
+            @atomic :monotonic child.max_world = typemax(UInt)
+        else
+            @atomic :monotonic child.max_world = maxworld
         end
-        @atomic :monotonic child.max_world = maxworld
         @assert visiting[child] == length(stack) + 1
         delete!(visiting, child)
         invalidations = _jl_debug_method_invalidation[]

test/precompile.jl

@@ -2222,4 +2222,37 @@ precompile_test_harness("No package module") do load_path
         String(take!(io)))
 end
 
+precompile_test_harness("Constprop CodeInstance invalidation") do load_path
+    write(joinpath(load_path, "DefineTheMethod.jl"),
+        """
+        module DefineTheMethod
+            export the_method
+            the_method_val(::Val{x}) where {x} = x
+            the_method_val(::Val{1}) = 0xdeadbeef
+            the_method_val(::Val{2}) = 2
+            the_method_val(::Val{3}) = 3
+            the_method_val(::Val{4}) = 4
+            the_method_val(::Val{5}) = 5
+            Base.@constprop :aggressive the_method(x) = the_method_val(Val{x}())
+            the_method(2)
+        end
+        """)
+    Base.compilecache(Base.PkgId("DefineTheMethod"))
+    write(joinpath(load_path, "CallTheMethod.jl"),
+        """
+        module CallTheMethod
+            using DefineTheMethod
+            call_the_method() = the_method(1)
+            call_the_method()
+        end
+        """)
+    Base.compilecache(Base.PkgId("CallTheMethod"))
+    @eval using DefineTheMethod
+    @eval using CallTheMethod
+    @eval DefineTheMethod.the_method_val(::Val{1}) = Int(0)
+    invokelatest() do
+        @test Int(0) == CallTheMethod.call_the_method()
+    end
+end
+
 finish_precompile_test!()