set slot_syms for methods of OCs constructed via `Core.OpaqueClosur…

…e` (#53650) Previously `oc` constructed via `Core.OpaqueClosure` does not set `oc.source.slot_syms` set, which caused segfaults either when trying to `show` `oc.source` or if invocation of `oc(...)` threw an error. This commit fixes that by making sure `oc.source.slot_syms` is set for `oc` created with `jl_new_opaque_closure_from_code_info`.
JuliaLang · Mar 12, 2024 · 7eb5cb8 · 7eb5cb8
1 parent 22602a2
commit 7eb5cb8
Show file tree

Hide file tree

Showing 8 changed files with 59 additions and 21 deletions.
diff --git a/base/compiler/ssair/legacy.jl b/base/compiler/ssair/legacy.jl
@@ -1,16 +1,16 @@
 # This file is a part of Julia. License is MIT: https://julialang.org/license
 
 """
-    inflate_ir!(ci::CodeInfo, linfo::MethodInstance) -> ir::IRCode
+    inflate_ir!(ci::CodeInfo, mi::MethodInstance) -> ir::IRCode
     inflate_ir!(ci::CodeInfo, sptypes::Vector{VarState}, argtypes::Vector{Any}) -> ir::IRCode
 
 Inflates `ci::CodeInfo`-IR to `ir::IRCode`-format.
 This should be used with caution as it is a in-place transformation where the fields of
 the original `ci::CodeInfo` are modified.
 """
-function inflate_ir!(ci::CodeInfo, linfo::MethodInstance)
-    sptypes = sptypes_from_meth_instance(linfo)
-    argtypes, _ = matching_cache_argtypes(fallback_lattice, linfo)
+function inflate_ir!(ci::CodeInfo, mi::MethodInstance)
+    sptypes = sptypes_from_meth_instance(mi)
+    argtypes, _ = matching_cache_argtypes(fallback_lattice, mi)
     return inflate_ir!(ci, sptypes, argtypes)
 end
 function inflate_ir!(ci::CodeInfo, sptypes::Vector{VarState}, argtypes::Vector{Any})
@@ -42,14 +42,16 @@ function inflate_ir!(ci::CodeInfo, sptypes::Vector{VarState}, argtypes::Vector{A
 end
 
 """
-    inflate_ir(ci::CodeInfo, linfo::MethodInstance) -> ir::IRCode
-    inflate_ir(ci::CodeInfo, sptypes::Vector{VarState}, argtypes::Vector{Any}) -> ir::IRCode
     inflate_ir(ci::CodeInfo) -> ir::IRCode
+    inflate_ir(ci::CodeInfo, mi::MethodInstance) -> ir::IRCode
+    inflate_ir(ci::CodeInfo, argtypes::Vector{Any}) -> ir::IRCode
+    inflate_ir(ci::CodeInfo, sptypes::Vector{VarState}, argtypes::Vector{Any}) -> ir::IRCode
 
 Non-destructive version of `inflate_ir!`.
 Mainly used for testing or interactive use.
 """
-inflate_ir(ci::CodeInfo, linfo::MethodInstance) = inflate_ir!(copy(ci), linfo)
+inflate_ir(ci::CodeInfo, mi::MethodInstance) = inflate_ir!(copy(ci), mi)
+inflate_ir(ci::CodeInfo, argtypes::Vector{Any}) = inflate_ir(ci, VarState[], argtypes)
 inflate_ir(ci::CodeInfo, sptypes::Vector{VarState}, argtypes::Vector{Any}) = inflate_ir!(copy(ci), sptypes, argtypes)
 function inflate_ir(ci::CodeInfo)
     parent = ci.parent

diff --git a/base/opaque_closure.jl b/base/opaque_closure.jl
@@ -58,17 +58,24 @@ end
 
 function Core.OpaqueClosure(ir::IRCode, @nospecialize env...;
                             isva::Bool = false,
+                            slotnames::Union{Nothing,Vector{Symbol}}=nothing,
                             do_compile::Bool = true)
     # NOTE: we need ir.argtypes[1] == typeof(env)
     ir = Core.Compiler.copy(ir)
     # if the user didn't specify a definition MethodInstance or filename Symbol to use for the debuginfo, set a filename now
     ir.debuginfo.def === nothing && (ir.debuginfo.def = :var"generated IR for OpaqueClosure")
-    nargs = length(ir.argtypes)-1
+    nargtypes = length(ir.argtypes)
+    nargs = nargtypes-1
     sig = compute_oc_signature(ir, nargs, isva)
     rt = compute_ir_rettype(ir)
     src = ccall(:jl_new_code_info_uninit, Ref{CodeInfo}, ())
-    src.slotnames = fill(:none, nargs+1)
-    src.slotflags = fill(zero(UInt8), length(ir.argtypes))
+    if slotnames === nothing
+        src.slotnames = fill(:none, nargtypes)
+    else
+        length(slotnames) == nargtypes || error("mismatched `argtypes` and `slotnames`")
+        src.slotnames = slotnames
+    end
+    src.slotflags = fill(zero(UInt8), nargtypes)
     src.slottypes = copy(ir.argtypes)
     src = Core.Compiler.ir_to_codeinf!(src, ir)
     return generate_opaque_closure(sig, Union{}, rt, src, nargs, isva, env...; do_compile)

diff --git a/src/julia_internal.h b/src/julia_internal.h
@@ -839,7 +839,7 @@ JL_DLLEXPORT jl_value_t *jl_as_global_root(jl_value_t *val, int insert) JL_GLOBA
 jl_opaque_closure_t *jl_new_opaque_closure(jl_tupletype_t *argt, jl_value_t *rt_lb, jl_value_t *rt_ub,
     jl_value_t *source,  jl_value_t **env, size_t nenv, int do_compile);
 jl_method_t *jl_make_opaque_closure_method(jl_module_t *module, jl_value_t *name,
-    int nargs, jl_value_t *functionloc, jl_value_t *uninferred_source, int isva);
+    int nargs, jl_value_t *functionloc, jl_code_info_t *ci, int isva, int isinferred);
 JL_DLLEXPORT int jl_is_valid_oc_argtype(jl_tupletype_t *argt, jl_method_t *source);
 
 // Each tuple can exist in one of 4 Vararg states:

diff --git a/src/method.c b/src/method.c
@@ -121,7 +121,8 @@ static jl_value_t *resolve_globals(jl_value_t *expr, jl_module_t *module, jl_sve
                 } else if (!jl_is_long(oc_nargs)) {
                     jl_type_error("opaque_closure_method", (jl_value_t*)jl_long_type, oc_nargs);
                 }
-                jl_method_t *m = jl_make_opaque_closure_method(module, name, jl_unbox_long(oc_nargs), functionloc, ci, isva);
+                jl_method_t *m = jl_make_opaque_closure_method(module, name,
+                    jl_unbox_long(oc_nargs), functionloc, (jl_code_info_t*)ci, isva, /*isinferred*/0);
                 return (jl_value_t*)m;
             }
             if (e->head == jl_cfunction_sym) {
@@ -997,7 +998,7 @@ void push_edge(jl_array_t *list, jl_value_t *invokesig, jl_method_instance_t *ca
 // method definition ----------------------------------------------------------
 
 jl_method_t *jl_make_opaque_closure_method(jl_module_t *module, jl_value_t *name,
-    int nargs, jl_value_t *functionloc, jl_value_t *uninferred_source, int isva)
+    int nargs, jl_value_t *functionloc, jl_code_info_t *ci, int isva, int isinferred)
 {
     jl_method_t *m = jl_new_method_uninit(module);
     JL_GC_PUSH1(&m);
@@ -1016,8 +1017,12 @@ jl_method_t *jl_make_opaque_closure_method(jl_module_t *module, jl_value_t *name
     jl_value_t *file = jl_linenode_file(functionloc);
     m->file = jl_is_symbol(file) ? (jl_sym_t*)file : jl_empty_sym;
     m->line = jl_linenode_line(functionloc);
-    if (jl_is_code_info(uninferred_source))
-        jl_method_set_source(m, (jl_code_info_t*)uninferred_source);
+    if (isinferred) {
+        m->slot_syms = jl_compress_argnames(ci->slotnames);
+        jl_gc_wb(m, m->slot_syms);
+    } else {
+        jl_method_set_source(m, ci);
+    }
     JL_GC_POP();
     return m;
 }

diff --git a/src/opaque_closure.c b/src/opaque_closure.c
@@ -139,7 +139,7 @@ JL_DLLEXPORT jl_opaque_closure_t *jl_new_opaque_closure_from_code_info(jl_tuplet
     JL_GC_PUSH3(&root, &sigtype, &inst);
     root = jl_box_long(lineno);
     root = jl_new_struct(jl_linenumbernode_type, root, file);
-    jl_method_t *meth = jl_make_opaque_closure_method(mod, jl_nothing, nargs, root, isinferred ? jl_nothing : (jl_value_t*)ci, isva);
+    jl_method_t *meth = jl_make_opaque_closure_method(mod, jl_nothing, nargs, root, ci, isva, isinferred);
     root = (jl_value_t*)meth;
     size_t world = jl_current_task->world_age;
     // these are only legal in the current world since they are not in any tables

diff --git a/test/compiler/irutils.jl b/test/compiler/irutils.jl
@@ -62,7 +62,8 @@ let m = Meta.@lower 1 + 1
     orig_src = m.args[1]::CodeInfo
     global function make_codeinfo(code::Vector{Any};
                                   ssavaluetypes::Union{Nothing,Vector{Any}}=nothing,
-                                  slottypes::Union{Nothing,Vector{Any}}=nothing)
+                                  slottypes::Union{Nothing,Vector{Any}}=nothing,
+                                  slotnames::Union{Nothing,Vector{Symbol}}=nothing)
         src = copy(orig_src)
         src.code = code
         nstmts = length(src.code)
@@ -77,14 +78,21 @@ let m = Meta.@lower 1 + 1
             src.slottypes = slottypes
             src.slotflags = fill(zero(UInt8), length(slottypes))
         end
+        if slotnames !== nothing
+            src.slotnames = slotnames
+        end
         return src
     end
     global function make_ircode(code::Vector{Any};
-                                ssavaluetypes::Union{Nothing,Vector{Any}}=nothing,
                                 slottypes::Union{Nothing,Vector{Any}}=nothing,
-                                verify::Bool=true)
-        src = make_codeinfo(code; ssavaluetypes, slottypes)
-        ir = Core.Compiler.inflate_ir(src)
+                                verify::Bool=true,
+                                kwargs...)
+        src = make_codeinfo(code; slottypes, kwargs...)
+        if slottypes !== nothing
+            ir = Core.Compiler.inflate_ir(src, slottypes)
+        else
+            ir = Core.Compiler.inflate_ir(src)
+        end
         verify && Core.Compiler.verify_ir(ir)
         return ir
     end

diff --git a/test/compiler/ssair.jl b/test/compiler/ssair.jl
@@ -265,6 +265,17 @@ let code = Any[
     oc = Core.OpaqueClosure(ir)
     @test oc(false, 1, 1) == 2
     @test_throws "potential throw" oc(true, 1, 1)
+
+    let buf = IOBuffer()
+        oc = Core.OpaqueClosure(ir; slotnames=Symbol[:ocfunc, :x, :y, :z])
+        try
+            oc(true, 1, 1)
+        catch
+            Base.show_backtrace(buf, catch_backtrace())
+        end
+        s = String(take!(buf))
+        @test occursin("(x::Bool, y::$Int, z::$Int)", s)
+    end
 end
 
 # Test dynamic update of domtree with edge insertions and deletions in the

diff --git a/test/opaque_closure.jl b/test/opaque_closure.jl
@@ -353,3 +353,8 @@ end
 foopaque() = Base.Experimental.@opaque(@noinline x::Int->println(x))(1)
 
 code_llvm(devnull,foopaque,()) #shouldn't crash
+
+let ir = first(only(Base.code_ircode(sin, (Int,))))
+    oc = Core.OpaqueClosure(ir)
+    @test (Base.show_method(IOBuffer(), oc.source::Method); true)
+end