Skip to content

limit recursion depth to prevent stack overflow in 2-arg promote_type #57517

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

Open
wants to merge 53 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
4e935a9
prevent stack overflow in 2-arg `promote_type`
nsajko Feb 24, 2025
4ae0df1
reduce PR scope, use recursion instead of looping for better inference
nsajko Feb 24, 2025
4a64b69
fix
nsajko Feb 24, 2025
b3c5fdb
fix bootstrap
nsajko Feb 24, 2025
9ce7c8a
nicer tuple definition, try if this fixes bootstrap
nsajko Feb 24, 2025
fffa54d
try increasing the limit, hopefully without breaking bootstrap again
nsajko Feb 24, 2025
abd6288
decrease limit, hopefully fixing the bootstrap
nsajko Feb 24, 2025
0b3be6e
make the exception a constant binding
nsajko Jun 3, 2025
062b6d3
tiny style improvement 1
nsajko Jun 3, 2025
31be3d0
style improvement 2
nsajko Jun 3, 2025
414e67a
use a doc string instead of a comment
nsajko Jun 3, 2025
91cb213
merge all `_promote_type_binary` methods into a single one
nsajko Jun 3, 2025
3384486
`_promote_type_binary`: style: use more local variables
nsajko Jun 3, 2025
6cde468
inline `promote_result` into `_promote_type_binary`
nsajko Jun 3, 2025
86da78c
normalize the output of `promote_rule` before using it
nsajko Jun 3, 2025
d4ad868
fix bootstrap: macro `noinline`
nsajko Jun 3, 2025
287f16f
order like before
nsajko Jun 3, 2025
279c7a0
fix
nsajko Jun 3, 2025
66e3eb1
also normalize the output of `typejoin`
nsajko Jun 3, 2025
31d2b46
check recursion depth only after checking subtyping
nsajko Jun 3, 2025
c3e1cdc
factor out subtyping checks into functions
nsajko Jun 3, 2025
b6d19e3
delete `inline` macro invocation and delete outdated comment
nsajko Jun 3, 2025
bf40548
go back to a loop, avoid recursion, see if that's OK now
nsajko Jun 4, 2025
6187344
Revert "go back to a loop, avoid recursion, see if that's OK now"
nsajko Jun 4, 2025
55ee0cd
decrease the recursion depth limit to see what will break
nsajko Jun 4, 2025
aae3d48
stylistic change in preparation for next commit
nsajko Jun 4, 2025
0cb2c56
move the recursion depth check to an even later point
nsajko Jun 4, 2025
294caff
rename `err` to `err_giving_up`
nsajko Jun 4, 2025
782340c
detect the simplest kind of infinite recursion, improving UX
nsajko Jun 4, 2025
de617d0
deduplicate
nsajko Jun 4, 2025
4ad6496
add back simple conflict test, should improve code coverage
nsajko Jun 4, 2025
4e1700a
deduplicate `normalize_type(promote_rule(A, B))`
nsajko Jun 5, 2025
195e808
run `promote_rule(T, S)` before `promote_rule(S, T)`
nsajko Jun 5, 2025
93af1fb
add back the four `promote_type` methods for now to make PkgEval clean
nsajko Jun 5, 2025
103d1ae
add NEWS entry
nsajko Jun 5, 2025
799824b
increase the depth limit to eight
nsajko Jun 6, 2025
b7164cd
do away with `normalize_type`
nsajko Jun 13, 2025
80e03a1
one less method static parameter
nsajko Jun 13, 2025
124acec
even less method static parameters
nsajko Jun 13, 2025
229efbb
slight refactor
nsajko Jun 13, 2025
e762e6e
`type_is_bottom` -> `_type_is_bottom`
nsajko Jun 13, 2025
9e78161
reorder operations so some of them come before the recursive part
nsajko Jun 13, 2025
ea704c1
local functions to global functions
nsajko Jun 13, 2025
d77ce17
replace recursion with metaprogramming, enabling stronger inference
nsajko Jun 13, 2025
715fd7c
expand tests
nsajko Jun 13, 2025
14bc5a1
update NEWS
nsajko Jun 13, 2025
1cb1cb6
introduce custom exception types to improve error messages
nsajko Jun 13, 2025
55d5921
fix thrown exception when reaching depth limit
nsajko Jun 13, 2025
011d7a1
fix effect inference issues: more specialization
nsajko Jun 13, 2025
1d8d891
simplify "type is bottom" check
nsajko Jun 13, 2025
7926227
delete the unused `promote_type` methods again
nsajko Jun 13, 2025
7dbf0a7
Merge branch 'master' into prevent_stack_overflow_in_type_promotion
nsajko Jun 19, 2025
de99bf6
Merge branch 'master' into prevent_stack_overflow_in_type_promotion
nsajko Jun 23, 2025
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
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ is considered a bug fix ([#47102])

- The `hash` algorithm and its values have changed. Most `hash` specializations will remain correct and require no action. Types that reimplement the core hashing logic independently, such as some third-party string packages do, may require a migration to the new algorithm. ([#57509])

- A `promote_type` invocation used to recur without bound in some cases while trying to get `promote_rule` to converge. There's now a fixed recursion depth limit to prevent infinite recursion, after which `promote_type` gives up, throwing `ArgumentError`. A subset of cases that would've caused infinite recursion before is caught even before reaching the depth limit. The depth limit is expected to be high enough for there to be no breakage due to this change. NB: there is in actuality no recursion any more, it is emulated up to a shallow depth via metaprogramming. ([#57517])

Compiler/Runtime improvements
-----------------------------

Expand Down
2 changes: 1 addition & 1 deletion base/operators.jl
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ isunordered(x) = false
isunordered(x::AbstractFloat) = isnan(x)
isunordered(x::Missing) = true

==(T::Type, S::Type) = (@_total_meta; ccall(:jl_types_equal, Cint, (Any, Any), T, S) != 0)
==(T::Type, S::Type) = _types_are_equal(T, S)
!=(T::Type, S::Type) = (@_total_meta; !(T == S))
==(T::TypeVar, S::Type) = false
==(T::Type, S::TypeVar) = false
Expand Down
168 changes: 152 additions & 16 deletions base/promotion.jl
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,17 @@ function tailjoin(A::SimpleVector, i::Int)
return t
end

## type equality

function _types_are_equal(A::Type, B::Type)
@_total_meta
ccall(:jl_types_equal, Cint, (Any, Any), A, B) !== Cint(0)
end

function _type_is_bottom(X::Type)
X === Bottom
end

## promotion mechanism ##

"""
Expand Down Expand Up @@ -307,19 +318,149 @@ promote_type(T) = T
promote_type(T, S, U) = (@inline; promote_type(promote_type(T, S), U))
promote_type(T, S, U, V...) = (@inline; afoldl(promote_type, promote_type(T, S, U), V...))

promote_type(::Type{Bottom}, ::Type{Bottom}) = Bottom
promote_type(::Type{T}, ::Type{T}) where {T} = T
promote_type(::Type{T}, ::Type{Bottom}) where {T} = T
promote_type(::Type{Bottom}, ::Type{T}) where {T} = T
"""
TypePromotionError <: Exception

Exception type thrown by [`promote_type`](@ref) when giving up for either of two reasons:

* Because bugs in the [`promote_rule`](@ref) logic which would have caused infinite recursion were detected.

* Because a threshold on the recursion depth was reached.

Check for bugs in the [`promote_rule`](@ref) logic if this exception gets thrown.
"""
struct TypePromotionError <: Exception
T_initial::Type
S_initial::Type
T::Type
S::Type
ts::Type
st::Type
threshold_reached::Bool
end

function showerror(io::IO, e::TypePromotionError)
print(io, "TypePromotionError: `promote_type(T, S)` failed: ")

desc = if e.threshold_reached
"giving up because the recursion depth limit was reached: check for faulty/conflicting/missing `promote_rule` methods\n"
else
"detected unbounded recursion caused by faulty `promote_rule` logic\n"
end

print(io, desc)

print(io, " initial `T`: ")
show(io, e.T_initial)
print(io, '\n')

print(io, " initial `S`: ")
show(io, e.S_initial)
print(io, '\n')

print(io, " next-to-last `T`: ")
show(io, e.T)
print(io, '\n')

print(io, " next-to-last `S`: ")
show(io, e.S)
print(io, '\n')

print(io, " last `T`: ")
show(io, e.ts)
print(io, '\n')

print(io, " last `S`: ")
show(io, e.st)
print(io, '\n')

nothing
end

function _promote_type_binary_err_giving_up(@nospecialize(T_initial::Type), @nospecialize(S_initial::Type), @nospecialize(T::Type), @nospecialize(S::Type), @nospecialize(ts::Type), @nospecialize(st::Type))
@noinline
@_nospecializeinfer_meta
throw(TypePromotionError(T_initial, S_initial, T, S, ts, st, true))
end
function _promote_type_binary_err_detected_infinite_recursion(@nospecialize(T_initial::Type), @nospecialize(S_initial::Type), @nospecialize(T::Type), @nospecialize(S::Type), @nospecialize(ts::Type), @nospecialize(st::Type))
@noinline
@_nospecializeinfer_meta
throw(TypePromotionError(T_initial, S_initial, T, S, ts, st, false))
end

function _promote_type_binary_detect_loop(T::Type, S::Type, A::Type, B::Type)
onesided(T::Type, S::Type, A::Type, B::Type) = _types_are_equal(T, A) && _types_are_equal(S, B)
onesided(T, S, A, B) || onesided(T, S, B, A)
end

macro _promote_type_binary_step1()
e = quote
# Try promote_rule in both orders.
ts = promote_rule(T, S)
st = promote_rule(S, T)
# If no promote_rule is defined, both directions give Bottom. In that
# case use typejoin on the original types instead.
st_is_bottom = _type_is_bottom(st)
ts_is_bottom = _type_is_bottom(ts)
if st_is_bottom && ts_is_bottom
return typejoin(T, S)
end
if ts_is_bottom
return st
end
if st_is_bottom || _types_are_equal(st, ts)
return ts
end
if _promote_type_binary_detect_loop(T, S, ts, st)
# This is not strictly necessary, as we already limit the recursion depth, but
# makes for nicer UX.
_promote_type_binary_err_detected_infinite_recursion(T_initial, S_initial, T, S, ts, st)
end
end
esc(e)
end

macro _promote_type_binary_step2()
e = quote
T = ts
S = st
end
esc(e)
end

macro _promote_type_binary_step()
e = quote
@_promote_type_binary_step1
@_promote_type_binary_step2
end
esc(e)
end

function _promote_type_binary(::Type{T_initial}, ::Type{S_initial}) where {T_initial, S_initial}
T = T_initial
S = S_initial

@_promote_type_binary_step
@_promote_type_binary_step
@_promote_type_binary_step
@_promote_type_binary_step
@_promote_type_binary_step
@_promote_type_binary_step
@_promote_type_binary_step

@_promote_type_binary_step1

_promote_type_binary_err_giving_up(T_initial, S_initial, T, S, ts, st)
end

function promote_type(::Type{T}, ::Type{S}) where {T,S}
@inline
# Try promote_rule in both orders. Typically only one is defined,
# and there is a fallback returning Bottom below, so the common case is
# promote_type(T, S) =>
# promote_result(T, S, result, Bottom) =>
# typejoin(result, Bottom) => result
promote_result(T, S, promote_rule(T,S), promote_rule(S,T))
if _type_is_bottom(T)
return S
end
if _type_is_bottom(S) || _types_are_equal(S, T)
return T
end
_promote_type_binary(T, S)
end

"""
Expand All @@ -339,11 +480,6 @@ promote_rule(::Type{Bottom}, ::Type{Bottom}, slurp...) = Bottom # not strictly n
promote_rule(::Type{Bottom}, ::Type{T}, slurp...) where {T} = T
promote_rule(::Type{T}, ::Type{Bottom}, slurp...) where {T} = T

promote_result(::Type,::Type,::Type{T},::Type{S}) where {T,S} = (@inline; promote_type(T,S))
# If no promote_rule is defined, both directions give Bottom. In that
# case use typejoin on the original types instead.
promote_result(::Type{T},::Type{S},::Type{Bottom},::Type{Bottom}) where {T,S} = (@inline; typejoin(T, S))

"""
promote(xs...)

Expand Down
23 changes: 23 additions & 0 deletions test/core.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2019,6 +2019,29 @@ g4731() = f4731()
@test f4731() == ""
@test g4731() == ""

@testset "issue #13193" begin
struct Issue13193_SIQuantity{T<:Number} <: Number end
Base.promote_rule(::Type{Issue13193_SIQuantity{T}}, ::Type{Issue13193_SIQuantity{S}}) where {T, S} = Issue13193_SIQuantity{promote_type(T,S)}
Base.promote_rule(::Type{Issue13193_SIQuantity{T}}, ::Type{S}) where {T, S<:Number} = Issue13193_SIQuantity{promote_type(T,S)}
struct Issue13193_Interval{T<:Number} <: Number end
Base.promote_rule(::Type{Issue13193_Interval{T}}, ::Type{Issue13193_Interval{S}}) where {T, S} = Issue13193_Interval{promote_type(T,S)}
Base.promote_rule(::Type{Issue13193_Interval{T}}, ::Type{S}) where {T, S<:Number} = Issue13193_Interval{promote_type(T,S)}
@test_throws Base.TypePromotionError promote_type(Issue13193_Interval{Int}, Issue13193_SIQuantity{Int})
@test_throws Base.TypePromotionError promote_type(Issue13193_SIQuantity{Int}, Issue13193_Interval{Int})
@test Base.Compiler.is_foldable(Base.infer_effects(promote_type, Tuple{Type{Issue13193_Interval{Int}}, Type{Issue13193_SIQuantity{Int}}}))
@test Base.Compiler.is_foldable(Base.infer_effects(promote_type, Tuple{Type{Issue13193_SIQuantity{Int}}, Type{Issue13193_Interval{Int}}}))
end
@testset "straightforward conflict in `promote_rule` definitions" begin
struct ConflictingPromoteRuleDefinitionsA end
struct ConflictingPromoteRuleDefinitionsB end
Base.promote_rule(::Type{ConflictingPromoteRuleDefinitionsA}, ::Type{ConflictingPromoteRuleDefinitionsB}) = ConflictingPromoteRuleDefinitionsA
Base.promote_rule(::Type{ConflictingPromoteRuleDefinitionsB}, ::Type{ConflictingPromoteRuleDefinitionsA}) = ConflictingPromoteRuleDefinitionsB
@test_throws Base.TypePromotionError promote_type(ConflictingPromoteRuleDefinitionsA, ConflictingPromoteRuleDefinitionsB)
@test_throws Base.TypePromotionError promote_type(ConflictingPromoteRuleDefinitionsB, ConflictingPromoteRuleDefinitionsA)
@test Base.Compiler.is_foldable(Base.infer_effects(promote_type, Tuple{Type{ConflictingPromoteRuleDefinitionsA}, Type{ConflictingPromoteRuleDefinitionsB}}))
@test Base.Compiler.is_foldable(Base.infer_effects(promote_type, Tuple{Type{ConflictingPromoteRuleDefinitionsB}, Type{ConflictingPromoteRuleDefinitionsA}}))
end

# issue #4675
f4675(x::StridedArray...) = 1
f4675(x::StridedArray{T}...) where {T} = 2
Expand Down
Loading