Skip to content

Commit 6cdafa9

Browse files
authored
[release-1.11] Avoid dropping call edges in presence of identical invoke edges (#57077)
The intermediate data structure here (used for edge de-duplication) was accidentally recording `invoke` edges as if they were `call` edges. That bug is _very_ frequently benign, but if there are identical call and invoke edges in the edge list and the invoke edge is scanned first, the call edge will be unsoundly dropped, leading to invalidation (#265) bugs. Already fixed on master (by happy accident) as part of #54894
1 parent 0a8ad1f commit 6cdafa9

File tree

2 files changed

+41
-3
lines changed

2 files changed

+41
-3
lines changed

src/staticdata_utils.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,8 @@ static void jl_collect_edges(jl_array_t *edges, jl_array_t *ext_targets, jl_arra
507507
// (invokeTypes, c) => invoke
508508
// (nullptr, invokeTypes) => missing call
509509
// (invokeTypes, nullptr) => missing invoke (unused--inferred as Any)
510-
void *target = ptrhash_get(&edges_map2, invokeTypes ? (void*)invokeTypes : (void*)callee);
510+
void *key = invokeTypes ? (void*)invokeTypes : (void*)callee;
511+
void *target = ptrhash_get(&edges_map2, key);
511512
if (target == HT_NOTFOUND) {
512513
size_t min_valid = 0;
513514
size_t max_valid = ~(size_t)0;
@@ -551,7 +552,7 @@ static void jl_collect_edges(jl_array_t *edges, jl_array_t *ext_targets, jl_arra
551552
jl_array_ptr_1d_push(ext_targets, callee);
552553
jl_array_ptr_1d_push(ext_targets, matches);
553554
target = (void*)((char*)HT_NOTFOUND + jl_array_nrows(ext_targets) / 3);
554-
ptrhash_put(&edges_map2, (void*)callee, target);
555+
ptrhash_put(&edges_map2, key, target);
555556
}
556557
idxs[++nt] = (char*)target - (char*)HT_NOTFOUND - 1;
557558
}

test/precompile.jl

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1107,6 +1107,17 @@ precompile_test_harness("invoke") do dir
11071107
f44320(::Any) = 2
11081108
g44320() = invoke(f44320, Tuple{Any}, 0)
11091109
g44320()
1110+
# Issue #57115
1111+
f57115(@nospecialize(::Any)) = error("unimplemented")
1112+
function g57115(@nospecialize(x))
1113+
if @noinline rand(Bool)
1114+
# Add an 'invoke' edge from 'foo' to 'bar'
1115+
Core.invoke(f57115, Tuple{Any}, x)
1116+
else
1117+
# ... and also an identical 'call' edge
1118+
@noinline f57115(x)
1119+
end
1120+
end
11101121
11111122
# Adding new specializations should not invalidate `invoke`s
11121123
function getlast(itr)
@@ -1123,6 +1134,8 @@ precompile_test_harness("invoke") do dir
11231134
"""
11241135
module $CallerModule
11251136
using $InvokeModule
1137+
import $InvokeModule: f57115, g57115
1138+
11261139
# involving external modules
11271140
callf(x) = f(x)
11281141
callg(x) = x < 5 ? g(x) : invoke(g, Tuple{Real}, x)
@@ -1143,6 +1156,8 @@ precompile_test_harness("invoke") do dir
11431156
11441157
# Issue #44320
11451158
f44320(::Real) = 3
1159+
# Issue #57115
1160+
f57115(::Int) = 1
11461161
11471162
call_getlast(x) = getlast(x)
11481163
@@ -1163,6 +1178,7 @@ precompile_test_harness("invoke") do dir
11631178
internalnc(3)
11641179
call_getlast([1,2,3])
11651180
end
1181+
precompile(g57115, (Any,))
11661182
11671183
# Now that we've precompiled, invalidate with a new method that overrides the `invoke` dispatch
11681184
$InvokeModule.h(x::Integer) = -1
@@ -1183,7 +1199,7 @@ precompile_test_harness("invoke") do dir
11831199
for m in methods(func)
11841200
m.sig.parameters[end] === T && return m
11851201
end
1186-
error("no ::Real method found for $func")
1202+
error("no ::$T method found for $func")
11871203
end
11881204
function nvalid(mi::Core.MethodInstance)
11891205
isdefined(mi, :cache) || return 0
@@ -1229,6 +1245,27 @@ precompile_test_harness("invoke") do dir
12291245
m = only(methods(M.g44320))
12301246
@test (m.specializations::Core.MethodInstance).cache.max_world == typemax(UInt)
12311247

1248+
m = only(methods(M.g57115))
1249+
mi = m.specializations::Core.MethodInstance
1250+
1251+
f_m = get_method_for_type(M.f57115, Any)
1252+
f_mi = f_m.specializations::Core.MethodInstance
1253+
1254+
# Make sure that f57115(::Any) has a 'call' backedge to 'g57115'
1255+
has_f_call_backedge = false
1256+
it = Core.Compiler.BackedgeIterator(f_mi.backedges)
1257+
# Manually-written iterate(...) protocol, since this is in Core.Compiler
1258+
item = Core.Compiler.iterate(it)
1259+
while item !== nothing
1260+
(; sig, caller) = item[1]
1261+
if sig === nothing && caller === mi
1262+
has_f_call_backedge = true
1263+
break
1264+
end
1265+
item = Core.Compiler.iterate(it, item[2])
1266+
end
1267+
@test has_f_call_backedge
1268+
12321269
m = which(MI.getlast, (Any,))
12331270
@test (m.specializations::Core.MethodInstance).cache.max_world == typemax(UInt)
12341271

0 commit comments

Comments
 (0)