Skip to content

Commit

Permalink
bpart: Give a warning when accessing a backdated const binding
Browse files Browse the repository at this point in the history
This implements the strategy proposed in #57102 (comment).
Example:
```
julia> function foo(i)
           eval(:(const x = $i))
           x
       end
foo (generic function with 1 method)

julia> foo(1)
WARNING: Detected access to binding Main.x in a world prior to its definition world.
  Julia 1.12 has introduced more strict world age semantics for global bindings.
  !!! This code may malfunction under Revise.
  !!! This code will error in future versions of Julia.
Hint: Add an appropriate `invokelatest` around the access to this binding.
1
```

The warning is triggered once per binding to avoid spamming for repeated access.
  • Loading branch information
Keno committed Jan 23, 2025
1 parent 61e8f1d commit ee6a96f
Show file tree
Hide file tree
Showing 19 changed files with 157 additions and 64 deletions.
3 changes: 2 additions & 1 deletion Compiler/src/Compiler.jl
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ using Core: ABIOverride, Builtin, CodeInstance, IntrinsicFunction, MethodInstanc

using Base
using Base: @_foldable_meta, @_gc_preserve_begin, @_gc_preserve_end, @nospecializeinfer,
BINDING_KIND_GLOBAL, BINDING_KIND_UNDEF_CONST, Base, BitVector, Bottom, Callable, DataTypeFieldDesc,
BINDING_KIND_GLOBAL, BINDING_KIND_UNDEF_CONST, BINDING_KIND_BACKDATED_CONST,
Base, BitVector, Bottom, Callable, DataTypeFieldDesc,
EffectsOverride, Filter, Generator, IteratorSize, JLOptions, NUM_EFFECTS_OVERRIDES,
OneTo, Ordering, RefValue, SizeUnknown, _NAMEDTUPLE_NAME,
_array_for, _bits_findnext, _methods_by_ftype, _uniontypes, all, allocatedinline, any,
Expand Down
5 changes: 5 additions & 0 deletions Compiler/src/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -3524,6 +3524,11 @@ function abstract_eval_partition_load(interp::AbstractInterpreter, partition::Co
end

if is_defined_const_binding(kind)
if kind == BINDING_KIND_BACKDATED_CONST
# Infer this as guard. We do not want a later const definition to retroactively improve
# inference results in an earlier world.
return RTEffects(Any, UndefVarError, generic_getglobal_effects)
end
rt = Const(partition_restriction(partition))
return RTEffects(rt, Union{}, Effects(EFFECTS_TOTAL, inaccessiblememonly=is_mutation_free_argtype(rt) ? ALWAYS_TRUE : ALWAYS_FALSE))
end
Expand Down
4 changes: 0 additions & 4 deletions Compiler/src/ssair/slot2ssa.jl
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,6 @@ function fixemup!(@specialize(slot_filter), @specialize(rename_slot), ir::IRCode
return nothing
end
op[] = x
elseif isa(val, GlobalRef) && !(isdefined(val.mod, val.name) && isconst(val.mod, val.name))
typ = typ_for_val(val, ci, ir, idx, Any[])
new_inst = NewInstruction(val, typ)
op[] = NewSSAValue(insert_node!(ir, idx, new_inst).id - length(ir.stmts))
elseif isexpr(val, :static_parameter)
ty = typ_for_val(val, ci, ir, idx, Any[])
if isa(ty, Const)
Expand Down
11 changes: 10 additions & 1 deletion Compiler/src/ssair/verify.jl
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ function verify_ir(ir::IRCode, print::Bool=true,
if mi !== nothing
push!(error_args, "\n", " Method instance: ", mi)
end
error(error_args...)
invokelatest(error, error_args...)
end
# Verify CFG graph. Must be well formed to construct domtree
if !(length(ir.cfg.blocks) - 1 <= length(ir.cfg.index) <= length(ir.cfg.blocks))
Expand Down Expand Up @@ -380,6 +380,15 @@ function verify_ir(ir::IRCode, print::Bool=true,
# undefined GlobalRef is OK in isdefined
continue
end
elseif stmt.head === :throw_undef_if_not
if length(stmt.args) > 3
@verify_error "malformed throw_undef_if_not"
raise_error()
end
if stmt.args[1] isa GlobalRef
# undefined GlobalRef is OK in throw_undef_if_not
continue
end
elseif stmt.head === :gc_preserve_end
# We allow gc_preserve_end tokens to span across try/catch
# blocks, which isn't allowed for regular SSA values, so
Expand Down
2 changes: 1 addition & 1 deletion Compiler/test/invalidation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ let mi = Base.method_instance(basic_caller, (Float64,))
end

# this redefinition below should invalidate the cache
const BASIC_CALLER_WORLD = Base.get_world_counter()
const BASIC_CALLER_WORLD = Base.get_world_counter()+1
basic_callee(x) = x, x
@test !isdefined(Base.method_instance(basic_callee, (Float64,)), :cache)
let mi = Base.method_instance(basic_caller, (Float64,))
Expand Down
4 changes: 4 additions & 0 deletions base/Base_compiler.jl
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,10 @@ using .Order
include("coreir.jl")
include("invalidation.jl")

# Because lowering inserts direct references, it is mandatory for this binding
# to exist before we start inferring code.
function string end

# For OS specific stuff
# We need to strcat things here, before strings are really defined
function strcat(x::String, y::String)
Expand Down
8 changes: 5 additions & 3 deletions base/boot.jl
Original file line number Diff line number Diff line change
Expand Up @@ -383,9 +383,10 @@ struct StackOverflowError <: Exception end
struct UndefRefError <: Exception end
struct UndefVarError <: Exception
var::Symbol
world::UInt64
scope # a Module or Symbol or other object describing the context where this variable was looked for (e.g. Main or :local or :static_parameter)
UndefVarError(var::Symbol) = new(var)
UndefVarError(var::Symbol, @nospecialize scope) = new(var, scope)
UndefVarError(var::Symbol) = new(var, ccall(:jl_get_tls_world_age, UInt, ()))
UndefVarError(var::Symbol, @nospecialize scope) = new(var, ccall(:jl_get_tls_world_age, UInt, ()), scope)
end
struct ConcurrencyViolationError <: Exception
msg::AbstractString
Expand Down Expand Up @@ -717,7 +718,8 @@ macro __doc__(x)
end

isbasicdoc(@nospecialize x) = (isa(x, Expr) && x.head === :.) || isa(x, Union{QuoteNode, Symbol})
iscallexpr(ex::Expr) = (isa(ex, Expr) && ex.head === :where) ? iscallexpr(ex.args[1]) : (isa(ex, Expr) && ex.head === :call)
firstarg(arg1, args...) = arg1
iscallexpr(ex::Expr) = (isa(ex, Expr) && ex.head === :where) ? iscallexpr(firstarg(ex.args...)) : (isa(ex, Expr) && ex.head === :call)
iscallexpr(ex) = false
function ignoredoc(source, mod, str, expr)
(isbasicdoc(expr) || iscallexpr(expr)) && return Expr(:escape, nothing)
Expand Down
4 changes: 2 additions & 2 deletions base/docs/bindings.jl
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ end

bindingexpr(x) = Expr(:call, Binding, splitexpr(x)...)

defined(b::Binding) = isdefined(b.mod, b.var)
resolve(b::Binding) = getfield(b.mod, b.var)
defined(b::Binding) = invokelatest(isdefined, b.mod, b.var)
resolve(b::Binding) = invokelatest(getfield, b.mod, b.var)

function splitexpr(x::Expr)
isexpr(x, :macrocall) ? splitexpr(x.args[1]) :
Expand Down
3 changes: 2 additions & 1 deletion base/runtime_internals.jl
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,9 @@ const BINDING_KIND_FAILED = 0x6
const BINDING_KIND_DECLARED = 0x7
const BINDING_KIND_GUARD = 0x8
const BINDING_KIND_UNDEF_CONST = 0x9
const BINDING_KIND_BACKDATED_CONST = 0xa

is_defined_const_binding(kind::UInt8) = (kind == BINDING_KIND_CONST || kind == BINDING_KIND_CONST_IMPORT)
is_defined_const_binding(kind::UInt8) = (kind == BINDING_KIND_CONST || kind == BINDING_KIND_CONST_IMPORT || kind == BINDING_KIND_BACKDATED_CONST)
is_some_const_binding(kind::UInt8) = (is_defined_const_binding(kind) || kind == BINDING_KIND_UNDEF_CONST)
is_some_imported(kind::UInt8) = (kind == BINDING_KIND_IMPLICIT || kind == BINDING_KIND_EXPLICIT || kind == BINDING_KIND_IMPORTED)
is_some_guard(kind::UInt8) = (kind == BINDING_KIND_GUARD || kind == BINDING_KIND_DECLARED || kind == BINDING_KIND_FAILED || kind == BINDING_KIND_UNDEF_CONST)
Expand Down
17 changes: 10 additions & 7 deletions base/show.jl
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ function _isself(ft::DataType)
isdefined(ftname, :mt) || return false
name = ftname.mt.name
mod = parentmodule(ft) # NOTE: not necessarily the same as ft.name.mt.module
return isdefined(mod, name) && ft == typeof(getfield(mod, name))
return invokelatest(isdefinedglobal, mod, name) && ft == typeof(invokelatest(getglobal, mod, name))
end

function show(io::IO, ::MIME"text/plain", f::Function)
Expand Down Expand Up @@ -542,8 +542,8 @@ function show_function(io::IO, f::Function, compact::Bool, fallback::Function)
fallback(io, f)
elseif compact
print(io, mt.name)
elseif isdefined(mt, :module) && isdefined(mt.module, mt.name) &&
getfield(mt.module, mt.name) === f
elseif isdefined(mt, :module) && isdefinedglobal(mt.module, mt.name) &&
getglobal(mt.module, mt.name) === f
# this used to call the removed internal function `is_exported_from_stdlib`, which effectively
# just checked for exports from Core and Base.
mod = get(io, :module, UsesCoreAndBaseOnly)
Expand Down Expand Up @@ -1025,15 +1025,15 @@ function isvisible(sym::Symbol, parent::Module, from::Module)
from_owner = ccall(:jl_binding_owner, Ptr{Cvoid}, (Any, Any), from, sym)
return owner !== C_NULL && from_owner === owner &&
!isdeprecated(parent, sym) &&
isdefined(from, sym) # if we're going to return true, force binding resolution
isdefinedglobal(from, sym) # if we're going to return true, force binding resolution
end

function is_global_function(tn::Core.TypeName, globname::Union{Symbol,Nothing})
if globname !== nothing
globname_str = string(globname::Symbol)
if ('#' globname_str && '@' globname_str && isdefined(tn, :module) &&
isbindingresolved(tn.module, globname) && isdefined(tn.module, globname) &&
isconcretetype(tn.wrapper) && isa(getfield(tn.module, globname), tn.wrapper))
isbindingresolved(tn.module, globname) && isdefinedglobal(tn.module, globname) &&
isconcretetype(tn.wrapper) && isa(getglobal(tn.module, globname), tn.wrapper))
return true
end
end
Expand Down Expand Up @@ -3364,7 +3364,10 @@ function print_partition(io::IO, partition::Core.BindingPartition)
end
print(io, " - ")
kind = binding_kind(partition)
if is_defined_const_binding(kind)
if kind == BINDING_KIND_BACKDATED_CONST
print(io, "backdated constant binding to ")
print(io, partition_restriction(partition))
elseif is_defined_const_binding(kind)
print(io, "constant binding to ")
print(io, partition_restriction(partition))
elseif kind == BINDING_KIND_UNDEF_CONST
Expand Down
5 changes: 3 additions & 2 deletions doc/src/manual/methods.md
Original file line number Diff line number Diff line change
Expand Up @@ -587,12 +587,13 @@ In the example above, we see that the "current world" (in which the method `newf
is one greater than the task-local "runtime world" that was fixed when the execution of `tryeval` started.

Sometimes it is necessary to get around this (for example, if you are implementing the above REPL).
Fortunately, there is an easy solution: call the function using [`Base.invokelatest`](@ref):
Fortunately, there is an easy solution: call the function using [`Base.invokelatest`](@ref) or
the macro version [`Base.@invokelatest`](@ref):

```jldoctest
julia> function tryeval2()
@eval newfun2() = 2
Base.invokelatest(newfun2)
@invokelatest newfun2()
end
tryeval2 (generic function with 1 method)
Expand Down
3 changes: 2 additions & 1 deletion src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3467,7 +3467,8 @@ static jl_cgval_t emit_globalref(jl_codectx_t &ctx, jl_module_t *mod, jl_sym_t *
break;
pku = jl_atomic_load_acquire(&bpart->restriction);
}
if (bpart && jl_bkind_is_some_constant(decode_restriction_kind(pku))) {
enum jl_partition_kind kind = decode_restriction_kind(pku);
if (bpart && (jl_bkind_is_some_constant(kind) && kind != BINDING_KIND_BACKDATED_CONST)) {
jl_value_t *constval = decode_restriction_value(pku);
if (!constval) {
undef_var_error_ifnot(ctx, ConstantInt::get(getInt1Ty(ctx.builder.getContext()), 0), name, (jl_value_t*)mod);
Expand Down
22 changes: 15 additions & 7 deletions src/julia.h
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,10 @@ enum jl_partition_kind {
// Undef Constant: This binding partition is a constant declared using `const`, but
// without a value.
// ->restriction is NULL
BINDING_KIND_UNDEF_CONST = 0x9
BINDING_KIND_UNDEF_CONST = 0x9,
// Backated constant. A constant that was backdated for compatibility. In all other
// ways equivalent to BINDING_KIND_CONST, but prints a warning on access
BINDING_KIND_BACKDATED_CONST = 0xa,
};

#ifdef _P64
Expand Down Expand Up @@ -693,7 +696,7 @@ typedef struct _jl_binding_t {
jl_globalref_t *globalref; // cached GlobalRef for this binding
_Atomic(jl_value_t*) value;
_Atomic(jl_binding_partition_t*) partitions;
uint8_t declared:1;
uint8_t did_print_backdate_admonition:1;
uint8_t exportp:1; // `public foo` sets `publicp`, `export foo` sets both `publicp` and `exportp`
uint8_t publicp:1; // exportp without publicp is not allowed.
uint8_t deprecated:2; // 0=not deprecated, 1=renamed, 2=moved to another package
Expand Down Expand Up @@ -2025,10 +2028,6 @@ JL_DLLEXPORT void jl_module_public(jl_module_t *from, jl_sym_t *s, int exported)
JL_DLLEXPORT int jl_is_imported(jl_module_t *m, jl_sym_t *s);
JL_DLLEXPORT int jl_module_exports_p(jl_module_t *m, jl_sym_t *var);
JL_DLLEXPORT void jl_add_standard_imports(jl_module_t *m);
STATIC_INLINE jl_function_t *jl_get_function(jl_module_t *m, const char *name)
{
return (jl_function_t*)jl_get_global(m, jl_symbol(name));
}

// eq hash tables
JL_DLLEXPORT jl_genericmemory_t *jl_eqtable_put(jl_genericmemory_t *h JL_ROOTING_ARGUMENT, jl_value_t *key, jl_value_t *val JL_ROOTED_ARGUMENT, int *inserted);
Expand Down Expand Up @@ -2587,9 +2586,18 @@ typedef struct {
} jl_nullable_float32_t;

#define jl_root_task (jl_current_task->ptls->root_task)

JL_DLLEXPORT jl_task_t *jl_get_current_task(void) JL_GLOBALLY_ROOTED JL_NOTSAFEPOINT;

STATIC_INLINE jl_function_t *jl_get_function(jl_module_t *m, const char *name)
{
jl_task_t *ct = jl_get_current_task();
size_t last_world = ct->world_age;
ct->world_age = jl_get_world_counter();
jl_value_t *r = jl_get_global(m, jl_symbol(name));
ct->world_age = last_world;
return (jl_function_t*)r;
}

// TODO: we need to pin the task while using this (set pure bit)
JL_DLLEXPORT jl_jmp_buf *jl_get_safe_restore(void) JL_NOTSAFEPOINT;
JL_DLLEXPORT void jl_set_safe_restore(jl_jmp_buf *) JL_NOTSAFEPOINT;
Expand Down
12 changes: 9 additions & 3 deletions src/julia_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -933,6 +933,10 @@ EXTERN_INLINE_DECLARE enum jl_partition_kind decode_restriction_kind(jl_ptr_kind
if (bits == BINDING_KIND_CONST) {
return BINDING_KIND_UNDEF_CONST;
}
} else {
if (bits == BINDING_KIND_DECLARED) {
return BINDING_KIND_BACKDATED_CONST;
}
}

return (enum jl_partition_kind)bits;
Expand All @@ -956,12 +960,14 @@ STATIC_INLINE jl_ptr_kind_union_t encode_restriction(jl_value_t *val, enum jl_pa
#ifdef _P64
if (kind == BINDING_KIND_GUARD || kind == BINDING_KIND_DECLARED || kind == BINDING_KIND_FAILED || kind == BINDING_KIND_UNDEF_CONST)
assert(val == NULL);
else if (kind == BINDING_KIND_IMPLICIT || kind == BINDING_KIND_CONST)
else if (kind == BINDING_KIND_IMPLICIT || kind == BINDING_KIND_CONST || kind == BINDING_KIND_BACKDATED_CONST)
assert(val != NULL);
if (kind == BINDING_KIND_GUARD)
kind = BINDING_KIND_IMPLICIT;
else if (kind == BINDING_KIND_UNDEF_CONST)
kind = BINDING_KIND_CONST;
else if (kind == BINDING_KIND_BACKDATED_CONST)
kind = BINDING_KIND_DECLARED;
assert((((uintptr_t)val) & 0x7) == 0);
return ((jl_ptr_kind_union_t)val) | kind;
#else
Expand All @@ -975,11 +981,11 @@ STATIC_INLINE int jl_bkind_is_some_import(enum jl_partition_kind kind) JL_NOTSAF
}

STATIC_INLINE int jl_bkind_is_some_constant(enum jl_partition_kind kind) JL_NOTSAFEPOINT {
return kind == BINDING_KIND_CONST || kind == BINDING_KIND_CONST_IMPORT || kind == BINDING_KIND_UNDEF_CONST;
return kind == BINDING_KIND_CONST || kind == BINDING_KIND_CONST_IMPORT || kind == BINDING_KIND_UNDEF_CONST || kind == BINDING_KIND_BACKDATED_CONST;
}

STATIC_INLINE int jl_bkind_is_defined_constant(enum jl_partition_kind kind) JL_NOTSAFEPOINT {
return kind == BINDING_KIND_CONST || kind == BINDING_KIND_CONST_IMPORT;
return kind == BINDING_KIND_CONST || kind == BINDING_KIND_CONST_IMPORT || kind == BINDING_KIND_BACKDATED_CONST;
}

STATIC_INLINE int jl_bkind_is_some_guard(enum jl_partition_kind kind) JL_NOTSAFEPOINT {
Expand Down
Loading

0 comments on commit ee6a96f

Please sign in to comment.