Skip to content

inference: add missing limit-cycle poisoning #30098

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 27, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 1 addition & 19 deletions base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 22 additions & 0 deletions base/compiler/inferencestate.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 12 additions & 3 deletions base/compiler/typeinfer.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions test/compiler/compiler.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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