inference: always bail out const-prop' with non-const result limited

Investigating into #52763, I've found that `AbstractInterpreters` which
enables the `aggressive_constprop` option, such as `REPLInterpreter`,
might perform const-prop' even if the result of a non-const call is
`LimitedAccuracy'. This can lead to an unintended infinite loop with
a custom aggressive const-prop' implementation.

This commit restricts const-prop' for such cases where the non-const
call result is limited to avoid the issue. This fix is conservative, but
given that accurate inference is mostly impossible when there are
unresolvable cycles (which is indicated by limited result),
aggressive const-prop' isn't necessary for such cases, and I don't
anticipate this leading to any obvious regression.

fix #52763

base/compiler/abstractinterpretation.jl

@@ -814,12 +814,7 @@ end
 function abstract_call_method_with_const_args(interp::AbstractInterpreter,
     result::MethodCallResult, @nospecialize(f), arginfo::ArgInfo, si::StmtInfo,
     match::MethodMatch, sv::AbsIntState, invokecall::Union{Nothing,InvokeCall}=nothing)
-
-    if !const_prop_enabled(interp, sv, match)
-        return nothing
-    end
-    if bail_out_const_call(interp, result, si)
-        add_remark!(interp, sv, "[constprop] No more information to be gained")
+    if !const_prop_enabled(interp, match, sv) || bail_out_const_call(interp, result, si, sv)
         return nothing
     end
     eligibility = concrete_eval_eligible(interp, f, result, arginfo, sv)
@@ -852,7 +847,7 @@ function abstract_call_method_with_const_args(interp::AbstractInterpreter,
     return const_prop_call(interp, mi, result, arginfo, sv, concrete_eval_result)
 end
 
-function const_prop_enabled(interp::AbstractInterpreter, sv::AbsIntState, match::MethodMatch)
+function const_prop_enabled(interp::AbstractInterpreter, match::MethodMatch, sv::AbsIntState)
     if !InferenceParams(interp).ipo_constant_propagation
         add_remark!(interp, sv, "[constprop] Disabled by parameter")
         return false
@@ -864,9 +859,11 @@ function const_prop_enabled(interp::AbstractInterpreter, sv::AbsIntState, match:
     return true
 end
 
-function bail_out_const_call(interp::AbstractInterpreter, result::MethodCallResult, si::StmtInfo)
+function bail_out_const_call(interp::AbstractInterpreter, result::MethodCallResult,
+                             si::StmtInfo, sv::AbsIntState)
     if is_removable_if_unused(result.effects)
         if isa(result.rt, Const) || call_result_unused(si)
+            add_remark!(interp, sv, "[constprop] No more information to be gained")
             return true
         end
     elseif result.rt === Bottom
@@ -876,8 +873,14 @@ function bail_out_const_call(interp::AbstractInterpreter, result::MethodCallResu
             # precise enough to let us determine :consistency of `exct`, so we
             # would have to force constprop just to determine this, which is too
             # expensive.
+            add_remark!(interp, sv, "[constprop] No more information to be gained")
             return true
         end
+    elseif result.rt isa LimitedAccuracy
+        # optimizations like inlining are disabled for limited frames,
+        # thus there won't be much benefit in constant-prop' here
+        add_remark!(interp, sv, "[constprop] Disabled by entry heuristic (limited accuracy)")
+        return true
     end
     return false
 end
@@ -974,7 +977,9 @@ function maybe_get_const_prop_profitable(interp::AbstractInterpreter,
     match::MethodMatch, sv::AbsIntState)
     method = match.method
     force = force_const_prop(interp, f, method)
-    force || const_prop_entry_heuristic(interp, result, si, sv) || return nothing
+    if !force && !const_prop_entry_heuristic(interp, result, si, sv)
+        return nothing
+    end
     nargs::Int = method.nargs
     method.isva && (nargs -= 1)
     length(arginfo.argtypes) < nargs && return nothing
@@ -1021,21 +1026,14 @@ function const_prop_entry_heuristic(interp::AbstractInterpreter, result::MethodC
     elseif isa(rt, PartialStruct) || isa(rt, InterConditional) || isa(rt, InterMustAlias)
         # could be improved to `Const` or a more precise wrapper
         return true
-    elseif isa(rt, LimitedAccuracy)
-        # optimizations like inlining are disabled for limited frames,
-        # thus there won't be much benefit in constant-prop' here
-        add_remark!(interp, sv, "[constprop] Disabled by entry heuristic (limited accuracy)")
-        return false
-    else
-        if isa(rt, Const)
-            if !is_nothrow(result.effects)
-                # Could still be improved to Bottom (or at least could see the effects improved)
-                return true
-            end
+    elseif isa(rt, Const)
+        if !is_nothrow(result.effects)
+            # Could still be improved to Bottom (or at least could see the effects improved)
+            return true
         end
-        add_remark!(interp, sv, "[constprop] Disabled by entry heuristic (unimprovable result)")
-        return false
     end
+    add_remark!(interp, sv, "[constprop] Disabled by entry heuristic (unimprovable result)")
+    return false
 end
 
 # determines heuristically whether if constant propagation can be worthwhile

test/compiler/inference.jl

@@ -4937,10 +4937,19 @@ g() = empty_nt_values(Base.inferencebarrier(Tuple{}))
 # This is somewhat sensitive to the exact recursion level that inference is willing to do, but the intention
 # is to test the case where inference limited a recursion, but then a forced constprop nevertheless managed
 # to terminate the call.
+@newinterp RecurseInterpreter
+function CC.bail_out_const_call(interp::RecurseInterpreter, result::CC.MethodCallResult,
+                                si::CC.StmtInfo, sv::CC.AbsIntState)
+    if result.rt isa CC.LimitedAccuracy
+        return false # allow constprop to recurse into unresolved cycles
+    end
+    return @invoke CC.bail_out_const_call(interp::CC.AbstractInterpreter, result::CC.MethodCallResult,
+                                          si::CC.StmtInfo, sv::CC.AbsIntState)
+end
 Base.@constprop :aggressive type_level_recurse1(x...) = x[1] == 2 ? 1 : (length(x) > 100 ? x : type_level_recurse2(x[1] + 1, x..., x...))
 Base.@constprop :aggressive type_level_recurse2(x...) = type_level_recurse1(x...)
 type_level_recurse_entry() = Val{type_level_recurse1(1)}()
-@test Base.return_types(type_level_recurse_entry, ()) |> only == Val{1}
+@test Base.infer_return_type(type_level_recurse_entry, (); interp=RecurseInterpreter()) == Val{1}
 
 # Test that inference doesn't give up if it can potentially refine effects,
 # even if the return type is Any.