Skip to content

Commit 7c2f533

Browse files
gbaraldiKristofferC
authored andcommitted
Root globals in toplevel exprs (#54433)
This fixes #54422, the code here assumes that top level exprs are always rooted, but I don't see that referenced anywhere else, or guaranteed, so conservatively always root objects that show up in code. (cherry picked from commit 6e5e87b)
1 parent 5f46099 commit 7c2f533

File tree

2 files changed

+17
-2
lines changed

2 files changed

+17
-2
lines changed

src/codegen.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6144,8 +6144,9 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaidx_
61446144
jl_value_t *val = expr;
61456145
if (jl_is_quotenode(expr))
61466146
val = jl_fieldref_noalloc(expr, 0);
6147-
if (jl_is_method(ctx.linfo->def.method)) // toplevel exprs are already rooted
6148-
val = jl_ensure_rooted(ctx, val);
6147+
// Toplevel exprs are rooted but because codegen assumes this is constant, it removes the write barriers for this code.
6148+
// This means we have to globally root the value here. (The other option would be to change how we optimize toplevel code)
6149+
val = jl_ensure_rooted(ctx, val);
61496150
return mark_julia_const(ctx, val);
61506151
}
61516152

test/gc.jl

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,17 @@ end
3939
@testset "Base.GC docstrings" begin
4040
@test isempty(Docs.undocumented_names(GC))
4141
end
42+
43+
#testset doesn't work here because this needs to run in top level
44+
#Check that we ensure objects in toplevel exprs are rooted
45+
global dims54422 = [] # allocate the Binding
46+
GC.gc(); GC.gc(); # force the binding to be old
47+
GC.enable(false); # prevent new objects from being old
48+
@eval begin
49+
Base.Experimental.@force_compile # use the compiler
50+
dims54422 = $([])
51+
nothing
52+
end
53+
GC.enable(true); GC.gc(false) # incremental collection
54+
@test typeof(dims54422) == Vector{Any}
55+
@test isempty(dims54422)

0 commit comments

Comments
 (0)