From 3cebd25dbb9b3b7a0d3357402950a8a5dcfc696a Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Tue, 7 Oct 2025 03:08:07 +0000 Subject: [PATCH] Align interpreter and codegen error behavior of setglobal! and friends Currently this is an ErrorException in the runtime/interpreter, but a TypeError in codegen. This is not permitted - which error is thrown is semantically observable and codegen is not permitted to change it. Worse, inference is also inconsistent about whether this is TypeError or ErrorException, so this could actually lead to type confusion and crashes. Fix all that by having the runtime also emit a TypeError here. However, in order to not lose the binding name in the error message, adjust the TypeError context field to permit a binding. --- Compiler/src/abstractinterpretation.jl | 11 ++++++----- Compiler/test/inference.jl | 2 +- base/boot.jl | 2 +- base/errorshow.jl | 3 +++ src/codegen.cpp | 3 +-- src/datatype.c | 13 ++++++------- src/genericmemory.c | 2 +- src/julia.h | 6 +++++- src/julia_internal.h | 3 ++- src/module.c | 18 ++++++++---------- src/rtutils.c | 10 ++++++++++ test/core.jl | 5 ++++- 12 files changed, 48 insertions(+), 30 deletions(-) diff --git a/Compiler/src/abstractinterpretation.jl b/Compiler/src/abstractinterpretation.jl index 37d54e66d4ac6..4c20483d0367e 100644 --- a/Compiler/src/abstractinterpretation.jl +++ b/Compiler/src/abstractinterpretation.jl @@ -2485,7 +2485,7 @@ function abstract_eval_setglobal!(interp::AbstractInterpreter, sv::AbsIntState, (rt, exct) = global_assignment_rt_exct(interp, sv, saw_latestworld, gr, v) return CallMeta(rt, exct, Effects(setglobal!_effects, nothrow=exct===Bottom), GlobalAccessInfo(convert(Core.Binding, gr))) end - return CallMeta(Union{}, TypeError, EFFECTS_THROWS, NoCallInfo()) + return CallMeta(Union{}, Union{TypeError, ErrorException}, EFFECTS_THROWS, NoCallInfo()) end ⊑ = partialorder(typeinf_lattice(interp)) if !(hasintersect(widenconst(M), Module) && hasintersect(widenconst(s), Symbol)) @@ -3751,7 +3751,7 @@ end function global_assignment_rt_exct(interp::AbstractInterpreter, sv::AbsIntState, saw_latestworld::Bool, g::GlobalRef, @nospecialize(newty)) if saw_latestworld - return Pair{Any,Any}(newty, ErrorException) + return Pair{Any,Any}(newty, Union{TypeError, ErrorException}) end newty′ = RefValue{Any}(newty) (valid_worlds, ret) = scan_partitions(interp, g, sv.world) do interp::AbstractInterpreter, ::Core.Binding, partition::Core.BindingPartition @@ -3766,15 +3766,16 @@ function global_assignment_binding_rt_exct(interp::AbstractInterpreter, partitio if is_some_guard(kind) return Pair{Any,Any}(newty, ErrorException) elseif is_some_const_binding(kind) || is_some_imported(kind) - return Pair{Any,Any}(Bottom, ErrorException) + # N.B.: Backdating should not improve inference in an earlier world + return Pair{Any,Any}(kind == PARTITION_KIND_BACKDATED_CONST ? newty : Bottom, ErrorException) end ty = kind == PARTITION_KIND_DECLARED ? Any : partition_restriction(partition) wnewty = widenconst(newty) if !hasintersect(wnewty, ty) - return Pair{Any,Any}(Bottom, ErrorException) + return Pair{Any,Any}(Bottom, TypeError) elseif !(wnewty <: ty) retty = tmeet(typeinf_lattice(interp), newty, ty) - return Pair{Any,Any}(retty, ErrorException) + return Pair{Any,Any}(retty, TypeError) end return Pair{Any,Any}(newty, Bottom) end diff --git a/Compiler/test/inference.jl b/Compiler/test/inference.jl index c8df59a60c05e..36b65778b9d90 100644 --- a/Compiler/test/inference.jl +++ b/Compiler/test/inference.jl @@ -6458,7 +6458,7 @@ end global invalid_setglobal!_exct_modeling::Int @test Base.infer_exception_type((Float64,)) do x setglobal!(@__MODULE__, :invalid_setglobal!_exct_modeling, x) -end == ErrorException +end == TypeError # Issue #58257 - Hang in inference during BindingPartition resolution module A58257 diff --git a/base/boot.jl b/base/boot.jl index bbf3fc0faa227..f00e3a34f4cdb 100644 --- a/base/boot.jl +++ b/base/boot.jl @@ -410,7 +410,7 @@ struct TypeError <: Exception # `context` optionally adds extra detail, e.g. the name of the type parameter # that got a bad value. func::Symbol - context::Union{AbstractString,Symbol} + context::Union{AbstractString,GlobalRef,Symbol} expected::Type got TypeError(func, context, @nospecialize(expected::Type), @nospecialize(got)) = diff --git a/base/errorshow.jl b/base/errorshow.jl index 066aff0bf91c7..33edb4cee92a4 100644 --- a/base/errorshow.jl +++ b/base/errorshow.jl @@ -91,6 +91,9 @@ function showerror(io::IO, ex::TypeError) end if ex.context == "" ctx = "in $(ex.func)" + elseif isa(ex.context, Core.GlobalRef) + gr = ex.context + ctx = "in $(ex.func) of global binding `$(gr.mod).$(gr.name)`" elseif ex.func === :var"keyword argument" ctx = "in keyword argument $(ex.context)" else diff --git a/src/codegen.cpp b/src/codegen.cpp index 5a972ef1451df..8cd9ca53b4e5e 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -3231,8 +3231,7 @@ static jl_cgval_t emit_globalop(jl_codectx_t &ctx, jl_module_t *mod, jl_sym_t *s if (ty != nullptr) { const std::string fname = issetglobal ? "setglobal!" : isreplaceglobal ? "replaceglobal!" : isswapglobal ? "swapglobal!" : ismodifyglobal ? "modifyglobal!" : "setglobalonce!"; if (!ismodifyglobal) { - // TODO: use typeassert in jl_check_binding_assign_value too - emit_typecheck(ctx, rval, ty, "typeassert"); + emit_typecheck(ctx, rval, ty, fname.c_str()); rval = update_julia_type(ctx, rval, ty); if (rval.typ == jl_bottom_type) return jl_cgval_t(); diff --git a/src/datatype.c b/src/datatype.c index 6199b061e2430..ee947d2512064 100644 --- a/src/datatype.c +++ b/src/datatype.c @@ -1992,11 +1992,11 @@ jl_value_t *swap_nth_field(jl_datatype_t *st, jl_value_t *v, size_t i, jl_value_ } } -inline jl_value_t *modify_value(jl_value_t *ty, _Atomic(jl_value_t*) *p, jl_value_t *parent, jl_value_t *op, jl_value_t *rhs, int isatomic, jl_module_t *mod, jl_sym_t *name) +inline jl_value_t *modify_value(jl_value_t *ty, _Atomic(jl_value_t*) *p, jl_value_t *parent, jl_value_t *op, jl_value_t *rhs, int isatomic, jl_binding_t *b, jl_module_t *mod, jl_sym_t *name) { jl_value_t *r = isatomic ? jl_atomic_load(p) : jl_atomic_load_relaxed(p); if (__unlikely(r == NULL)) { - if (mod && name) + if (b) jl_undefined_var_error(name, (jl_value_t*)mod); jl_throw(jl_undefref_exception); } @@ -2007,11 +2007,10 @@ inline jl_value_t *modify_value(jl_value_t *ty, _Atomic(jl_value_t*) *p, jl_valu args[1] = rhs; jl_value_t *y = jl_apply_generic(op, args, 2); args[1] = y; - if (!jl_isa(y, ty)) { - if (mod && name) - jl_errorf("cannot assign an incompatible value to the global %s.%s.", jl_symbol_name(mod->name), jl_symbol_name(name)); + if (b) + jl_check_binding_assign_value(b, mod, name, y, "modifyglobal!"); + else if (!jl_isa(y, ty)) jl_type_error(jl_is_genericmemory(parent) ? "memoryrefmodify!" : "modifyfield!", ty, y); - } if (isatomic ? jl_atomic_cmpswap(p, &r, y) : jl_atomic_cmpswap_release(p, &r, y)) { jl_gc_wb(parent, y); break; @@ -2125,7 +2124,7 @@ jl_value_t *modify_nth_field(jl_datatype_t *st, jl_value_t *v, size_t i, jl_valu jl_value_t *ty = jl_field_type_concrete(st, i); char *p = (char*)v + offs; if (jl_field_isptr(st, i)) { - return modify_value(ty, (_Atomic(jl_value_t*)*)p, v, op, rhs, isatomic, NULL, NULL); + return modify_value(ty, (_Atomic(jl_value_t*)*)p, v, op, rhs, isatomic, NULL, NULL, NULL); } else { uint8_t *psel = jl_is_uniontype(ty) ? (uint8_t*)&p[jl_field_size(st, i) - 1] : NULL; diff --git a/src/genericmemory.c b/src/genericmemory.c index 35ef2b545a32d..ae45237433fcc 100644 --- a/src/genericmemory.c +++ b/src/genericmemory.c @@ -506,7 +506,7 @@ JL_DLLEXPORT jl_value_t *jl_memoryrefmodify(jl_genericmemoryref_t m, jl_value_t char *data = (char*)m.ptr_or_offset; if (layout->flags.arrayelem_isboxed) { assert(data - (char*)m.mem->ptr < sizeof(jl_value_t*) * m.mem->length); - return modify_value(eltype, (_Atomic(jl_value_t*)*)data, owner, op, rhs, isatomic, NULL, NULL); + return modify_value(eltype, (_Atomic(jl_value_t*)*)data, owner, op, rhs, isatomic, NULL, NULL, NULL); } size_t fsz = layout->size; uint8_t *psel = NULL; diff --git a/src/julia.h b/src/julia.h index 4e0ed0d549f20..c33827e9c5cbf 100644 --- a/src/julia.h +++ b/src/julia.h @@ -2016,7 +2016,7 @@ JL_DLLEXPORT jl_value_t *jl_get_module_binding_or_nothing(jl_module_t *m, jl_sym // get binding for reading JL_DLLEXPORT jl_binding_t *jl_get_binding(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var); -JL_DLLEXPORT jl_value_t *jl_module_globalref(jl_module_t *m, jl_sym_t *var); +JL_DLLEXPORT jl_value_t *jl_module_globalref(jl_module_t *m, jl_sym_t *var) JL_GLOBALLY_ROOTED; JL_DLLEXPORT jl_value_t *jl_get_binding_type(jl_module_t *m, jl_sym_t *var); // get binding for assignment JL_DLLEXPORT void jl_check_binding_currently_writable(jl_binding_t *b, jl_module_t *m, jl_sym_t *s); @@ -2086,6 +2086,10 @@ JL_DLLEXPORT void JL_NORETURN jl_type_error_rt(const char *fname, const char *context, jl_value_t *ty JL_MAYBE_UNROOTED, jl_value_t *got JL_MAYBE_UNROOTED); +JL_DLLEXPORT void JL_NORETURN jl_type_error_global(const char *fname, + jl_module_t *mod, jl_sym_t *sym, + jl_value_t *ty JL_MAYBE_UNROOTED, + jl_value_t *got JL_MAYBE_UNROOTED); JL_DLLEXPORT void JL_NORETURN jl_undefined_var_error(jl_sym_t *var, jl_value_t *scope JL_MAYBE_UNROOTED); JL_DLLEXPORT void JL_NORETURN jl_has_no_field_error(jl_datatype_t *t, jl_sym_t *var); JL_DLLEXPORT void JL_NORETURN jl_argument_error(char *str); diff --git a/src/julia_internal.h b/src/julia_internal.h index 4488388bb9875..0ccd46774f5ed 100644 --- a/src/julia_internal.h +++ b/src/julia_internal.h @@ -882,7 +882,7 @@ int set_nth_fieldonce(jl_datatype_t *st, jl_value_t *v, size_t i, jl_value_t *rh jl_value_t *swap_bits(jl_value_t *ty, char *v, uint8_t *psel, jl_value_t *parent, jl_value_t *rhs, enum atomic_kind isatomic); jl_value_t *replace_value(jl_value_t *ty, _Atomic(jl_value_t*) *p, jl_value_t *parent, jl_value_t *expected, jl_value_t *rhs, int isatomic, jl_module_t *mod, jl_sym_t *name); jl_value_t *replace_bits(jl_value_t *ty, char *p, uint8_t *psel, jl_value_t *parent, jl_value_t *expected, jl_value_t *rhs, enum atomic_kind isatomic); -jl_value_t *modify_value(jl_value_t *ty, _Atomic(jl_value_t*) *p, jl_value_t *parent, jl_value_t *op, jl_value_t *rhs, int isatomic, jl_module_t *mod, jl_sym_t *name); +jl_value_t *modify_value(jl_value_t *ty, _Atomic(jl_value_t*) *p, jl_value_t *parent, jl_value_t *op, jl_value_t *rhs, int isatomic, jl_binding_t *b, jl_module_t *mod, jl_sym_t *name); jl_value_t *modify_bits(jl_value_t *ty, char *p, uint8_t *psel, jl_value_t *parent, jl_value_t *op, jl_value_t *rhs, enum atomic_kind isatomic); int setonce_bits(jl_datatype_t *rty, char *p, jl_value_t *owner, jl_value_t *rhs, enum atomic_kind isatomic); jl_expr_t *jl_exprn(jl_sym_t *head, size_t n); @@ -895,6 +895,7 @@ jl_array_t *jl_get_loaded_modules(void); JL_DLLEXPORT int jl_datatype_isinlinealloc(jl_datatype_t *ty, int pointerfree); int jl_type_equality_is_identity(jl_value_t *t1, jl_value_t *t2) JL_NOTSAFEPOINT; +jl_value_t *jl_check_binding_assign_value(jl_binding_t *b JL_PROPAGATES_ROOT, jl_module_t *mod, jl_sym_t *var, jl_value_t *rhs JL_MAYBE_UNROOTED, const char *msg); void jl_binding_set_type(jl_binding_t *b, jl_module_t *mod, jl_sym_t *sym, jl_value_t *ty); JL_DLLEXPORT void jl_declare_global(jl_module_t *m, jl_value_t *arg, jl_value_t *set_type, int strong); JL_DLLEXPORT jl_binding_partition_t *jl_declare_constant_val3(jl_binding_t *b JL_ROOTING_ARGUMENT, jl_module_t *mod, jl_sym_t *var, jl_value_t *val JL_ROOTED_ARGUMENT JL_MAYBE_UNROOTED, enum jl_partition_kind, size_t new_world) JL_GLOBALLY_ROOTED; diff --git a/src/module.c b/src/module.c index 24ac81266da47..3ec8f5bd52782 100644 --- a/src/module.c +++ b/src/module.c @@ -1886,7 +1886,7 @@ void jl_binding_deprecation_warning(jl_binding_t *b) // For a generally writable binding (checked using jl_check_binding_currently_writable in this world age), check whether // we can actually write the value `rhs` to it. -jl_value_t *jl_check_binding_assign_value(jl_binding_t *b JL_PROPAGATES_ROOT, jl_module_t *mod, jl_sym_t *var, jl_value_t *rhs JL_MAYBE_UNROOTED) +jl_value_t *jl_check_binding_assign_value(jl_binding_t *b JL_PROPAGATES_ROOT, jl_module_t *mod, jl_sym_t *var, jl_value_t *rhs JL_MAYBE_UNROOTED, const char *msg) { JL_GC_PUSH1(&rhs); // callee-rooted jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age); @@ -1894,10 +1894,8 @@ jl_value_t *jl_check_binding_assign_value(jl_binding_t *b JL_PROPAGATES_ROOT, jl assert(kind == PARTITION_KIND_DECLARED || kind == PARTITION_KIND_GLOBAL); jl_value_t *old_ty = kind == PARTITION_KIND_DECLARED ? (jl_value_t*)jl_any_type : bpart->restriction; JL_GC_PROMISE_ROOTED(old_ty); - if (old_ty != (jl_value_t*)jl_any_type && jl_typeof(rhs) != old_ty) { - if (!jl_isa(rhs, old_ty)) - jl_errorf("cannot assign an incompatible value to the global %s.%s.", - jl_symbol_name(mod->name), jl_symbol_name(var)); + if (old_ty != (jl_value_t*)jl_any_type && jl_typeof(rhs) != old_ty && !jl_isa(rhs, old_ty)) { + jl_type_error_global(msg, mod, var, old_ty, rhs); } JL_GC_POP(); return old_ty; @@ -1905,7 +1903,7 @@ jl_value_t *jl_check_binding_assign_value(jl_binding_t *b JL_PROPAGATES_ROOT, jl JL_DLLEXPORT void jl_checked_assignment(jl_binding_t *b, jl_module_t *mod, jl_sym_t *var, jl_value_t *rhs) { - if (jl_check_binding_assign_value(b, mod, var, rhs) != NULL) { + if (jl_check_binding_assign_value(b, mod, var, rhs, "setglobal!") != NULL) { jl_atomic_store_release(&b->value, rhs); jl_gc_wb(b, rhs); } @@ -1913,7 +1911,7 @@ JL_DLLEXPORT void jl_checked_assignment(jl_binding_t *b, jl_module_t *mod, jl_sy JL_DLLEXPORT jl_value_t *jl_checked_swap(jl_binding_t *b, jl_module_t *mod, jl_sym_t *var, jl_value_t *rhs) { - jl_check_binding_assign_value(b, mod, var, rhs); + jl_check_binding_assign_value(b, mod, var, rhs, "swapglobal!"); jl_value_t *old = jl_atomic_exchange(&b->value, rhs); jl_gc_wb(b, rhs); if (__unlikely(old == NULL)) @@ -1923,7 +1921,7 @@ JL_DLLEXPORT jl_value_t *jl_checked_swap(jl_binding_t *b, jl_module_t *mod, jl_s JL_DLLEXPORT jl_value_t *jl_checked_replace(jl_binding_t *b, jl_module_t *mod, jl_sym_t *var, jl_value_t *expected, jl_value_t *rhs) { - jl_value_t *ty = jl_check_binding_assign_value(b, mod, var, rhs); + jl_value_t *ty = jl_check_binding_assign_value(b, mod, var, rhs, "replaceglobal!"); return replace_value(ty, &b->value, (jl_value_t*)b, expected, rhs, 1, mod, var); } @@ -1937,12 +1935,12 @@ JL_DLLEXPORT jl_value_t *jl_checked_modify(jl_binding_t *b, jl_module_t *mod, jl jl_symbol_name(mod->name), jl_symbol_name(var)); jl_value_t *ty = bpart->restriction; JL_GC_PROMISE_ROOTED(ty); - return modify_value(ty, &b->value, (jl_value_t*)b, op, rhs, 1, mod, var); + return modify_value(ty, &b->value, (jl_value_t*)b, op, rhs, 1, b, mod, var); } JL_DLLEXPORT jl_value_t *jl_checked_assignonce(jl_binding_t *b, jl_module_t *mod, jl_sym_t *var, jl_value_t *rhs ) { - jl_check_binding_assign_value(b, mod, var, rhs); + jl_check_binding_assign_value(b, mod, var, rhs, "setglobalonce!"); jl_value_t *old = NULL; if (jl_atomic_cmpswap(&b->value, &old, rhs)) jl_gc_wb(b, rhs); diff --git a/src/rtutils.c b/src/rtutils.c index 401bce1ce4f15..cb5da453506db 100644 --- a/src/rtutils.c +++ b/src/rtutils.c @@ -121,6 +121,16 @@ JL_DLLEXPORT void JL_NORETURN jl_type_error_rt(const char *fname, const char *co jl_throw(ex); } +JL_DLLEXPORT void JL_NORETURN jl_type_error_global(const char *fname, jl_module_t *mod, jl_sym_t *sym, + jl_value_t *expected JL_MAYBE_UNROOTED, + jl_value_t *got JL_MAYBE_UNROOTED) +{ + jl_value_t *gr = jl_module_globalref(mod, sym); + jl_value_t *ex = jl_new_struct(jl_typeerror_type, jl_symbol(fname), gr, expected, got); + jl_throw(ex); +} + + // with function name or description only JL_DLLEXPORT void JL_NORETURN jl_type_error(const char *fname, jl_value_t *expected JL_MAYBE_UNROOTED, diff --git a/test/core.jl b/test/core.jl index 618189fea6bbe..c8922083cea53 100644 --- a/test/core.jl +++ b/test/core.jl @@ -8153,7 +8153,10 @@ end setglobal!(m, :x, 2, :release) @test m.x === 2 @test_throws ConcurrencyViolationError setglobal!(m, :x, 3, :not_atomic) - @test_throws ErrorException setglobal!(m, :x, 4., :release) + @test_throws TypeError setglobal!(m, :x, 4., :release) + + f_set_bad_type(m) = setglobal!(m, :x, 4., :release) + @test_throws TypeError f_set_bad_type(m) m.x = 1 @test m.x === 1