inference: add missing limit-cycle poisoning (#30098)

Seems to have been missing since the original implementation of IPO constant-prop.
It's impressive how long we got away with just poisoning the cache with `Any`
if there was a self-recursive call, but it requires a fairly convoluted
setup to trigger this case.

Fix #29923

Author: vtjnash

base/compiler/abstractinterpretation.jl

@@ -310,25 +310,7 @@ function abstract_call_method(method::Method, @nospecialize(sig), sparams::Simpl
                 # (non-typically, this means that we lose the ability to detect a guaranteed StackOverflow in some cases)
                 return Any, false, nothing
             end
-            infstate = sv
-            topmost = topmost::InferenceState
-            while !(infstate === topmost.parent)
-                if call_result_unused(infstate)
-                    # If we won't propagate the result any further (since it's typically unused),
-                    # it's OK that we keep and cache the "limited" result in the parents
-                    # (non-typically, this means that we lose the ability to detect a guaranteed StackOverflow in some cases)
-                    # TODO: we might be able to halt progress much more strongly here,
-                    # since now we know we won't be able to keep anything much that we learned.
-                    # We were mainly only here to compute the calling convention return type,
-                    # but in most situations now, we are unlikely to be able to use that information.
-                    break
-                end
-                infstate.limited = true
-                for infstate_cycle in infstate.callers_in_cycle
-                    infstate_cycle.limited = true
-                end
-                infstate = infstate.parent
-            end
+            poison_callstack(sv, topmost::InferenceState, true)
             sig = newsig
             sparams = svec()
         end

base/compiler/inferencestate.jl

@@ -226,6 +226,28 @@ function add_mt_backedge!(mt::Core.MethodTable, @nospecialize(typ), caller::Infe
     nothing
 end
 
+function poison_callstack(infstate::InferenceState, topmost::InferenceState, poison_topmost::Bool)
+    poison_topmost && (topmost = topmost.parent)
+    while !(infstate === topmost)
+        if call_result_unused(infstate)
+            # If we won't propagate the result any further (since it's typically unused),
+            # it's OK that we keep and cache the "limited" result in the parents
+            # (non-typically, this means that we lose the ability to detect a guaranteed StackOverflow in some cases)
+            # TODO: we might be able to halt progress much more strongly here,
+            # since now we know we won't be able to keep anything much that we learned.
+            # We were mainly only here to compute the calling convention return type,
+            # but in most situations now, we are unlikely to be able to use that information.
+            break
+        end
+        infstate.limited = true
+        for infstate_cycle in infstate.callers_in_cycle
+            infstate_cycle.limited = true
+        end
+        infstate = infstate.parent
+        infstate === nothing && return
+    end
+end
+
 function is_specializable_vararg_slot(@nospecialize(arg), nargs::Int, vargs::Vector{Any})
     return (isa(arg, Slot) && slot_id(arg) == nargs && !isempty(vargs))
 end

base/compiler/typeinfer.jl

@@ -11,7 +11,6 @@ function typeinf(result::InferenceResult, cached::Bool, params::Params)
 end
 
 function typeinf(frame::InferenceState)
-    cached = frame.cached
     typeinf_nocycle(frame) || return false # frame is now part of a higher cycle
     # with no active ip's, frame is done
     frames = frame.callers_in_cycle
@@ -28,6 +27,7 @@ function typeinf(frame::InferenceState)
     # empty!(frames)
     min_valid = frame.min_valid
     max_valid = frame.max_valid
+    cached = frame.cached
     if cached || frame.parent !== nothing
         for caller in results
             opt = caller.src
@@ -442,13 +442,22 @@ function resolve_call_cycle!(linfo::MethodInstance, parent::InferenceState)
         uncached |= !frame.cached # ensure we never add an uncached frame to a cycle
         limited |= frame.limited
         if frame.linfo === linfo
-            uncached && return true
+            if uncached
+                # our attempt to speculate into a constant call lead to an undesired self-cycle
+                # that cannot be converged: poison our call-stack (up to the discovered duplicate frame)
+                # with the limited flag and abort (set return type to Any) now
+                poison_callstack(parent, frame, false)
+                return true
+            end
             merge_call_chain!(parent, frame, frame, limited)
             return frame
         end
         for caller in frame.callers_in_cycle
             if caller.linfo === linfo
-                uncached && return true
+                if uncached
+                    poison_callstack(parent, frame, false)
+                    return true
+                end
                 merge_call_chain!(parent, frame, caller, limited)
                 return caller
             end

test/compiler/compiler.jl

@@ -2134,3 +2134,11 @@ end
 # Test that inference can infer .instance of types
 f_instance(::Type{T}) where {T} = T.instance
 @test @inferred(f_instance(Nothing)) === nothing
+
+# test for some limit-cycle caching poisoning
+_false30098 = false
+f30098() = _false30098 ? g30098() : 3
+g30098() = (h30098(:f30098); 4)
+h30098(f) = getfield(@__MODULE__, f)()
+@test @inferred(g30098()) == 4 # make sure that this
+@test @inferred(f30098()) == 3 # doesn't pollute the inference cache of this