Skip to content

Commit

Permalink
Root globals in toplevel exprs (#54433)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
gbaraldi authored and giordano committed Sep 25, 2024
1 parent c5ab7c9 commit f21154f
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 2 deletions.
5 changes: 3 additions & 2 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5457,8 +5457,9 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaidx_
jl_value_t *val = expr;
if (jl_is_quotenode(expr))
val = jl_fieldref_noalloc(expr, 0);
if (jl_is_method(ctx.linfo->def.method)) // toplevel exprs are already rooted
val = jl_ensure_rooted(ctx, val);
// Toplevel exprs are rooted but because codegen assumes this is constant, it removes the write barriers for this code.
// This means we have to globally root the value here. (The other option would be to change how we optimize toplevel code)
val = jl_ensure_rooted(ctx, val);
return mark_julia_const(ctx, val);
}

Expand Down
14 changes: 14 additions & 0 deletions test/gc.jl
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,17 @@ end
run_gctest("gc/objarray.jl")
run_gctest("gc/chunks.jl")
end

#testset doesn't work here because this needs to run in top level
#Check that we ensure objects in toplevel exprs are rooted
global dims54422 = [] # allocate the Binding
GC.gc(); GC.gc(); # force the binding to be old
GC.enable(false); # prevent new objects from being old
@eval begin
Base.Experimental.@force_compile # use the compiler
dims54422 = $([])
nothing
end
GC.enable(true); GC.gc(false) # incremental collection
@test typeof(dims54422) == Vector{Any}
@test isempty(dims54422)

0 comments on commit f21154f

Please sign in to comment.