isdefined can be incorrectly constant propagated

[simonbyrne]

If PkgA conditionally defines a variable during __init__():

module PkgA
function __init__()
    global var
    if haskey(ENV, "varA")
        var = ENV["varA"]
    end
end
end

then a check to isdefined(PkgA, :var) in a the __init__() function of a dependent package may be incorrectly compiled to be constant. This is a problem if you have code such as:

module PkgB
using PkgA

function __init__()
    if isdefined(PkgA, :var)
        @show PkgA.var
    end
end
end

If I precompile once with the environment variable varA set, then run it again without, I get the following errors:

Julia 1.8.5

ERROR: InitError: UndefVarError: var not defined
Stacktrace:
  [1] getproperty
    @ ./Base.jl:31 [inlined]
  [2] macro expansion
    @ ./show.jl:1047 [inlined]
  [3] __init__()
    @ PkgB ~/misc/undef/PkgB/src/PkgB.jl:7
  [4] _include_from_serialized(pkg::Base.PkgId, path::String, depmods::Vector{Any})
    @ Base ./loading.jl:831
  [5] _require_search_from_serialized(pkg::Base.PkgId, sourcepath::String, build_id::UInt64)
    @ Base ./loading.jl:1039
  [6] _require(pkg::Base.PkgId)
    @ Base ./loading.jl:1315
  [7] _require_prelocked(uuidkey::Base.PkgId)
    @ Base ./loading.jl:1200
  [8] macro expansion
    @ ./loading.jl:1180 [inlined]
  [9] macro expansion
    @ ./lock.jl:223 [inlined]
 [10] require(into::Module, mod::Symbol)
    @ Base ./loading.jl:1144
during initialization of module PkgB

Julia 1.9-rc1:

➜  undef git:(main) julia@1.9 --project=PkgB -e 'using PkgB'

[6018] signal (11.2): Segmentation fault: 11
in expression starting at none:1
__init__ at /Users/simon/misc/undef/PkgB/src/PkgB.jl:7
jfptr___init___11 at /Users/simon/.julia/compiled/v1.9/PkgB/DW2fn_NQBT0.dylib (unknown line)
ijl_apply_generic at /Applications/Julia-1.9.app/Contents/Resources/julia/lib/julia/libjulia-internal.1.9.dylib (unknown line)
jl_module_run_initializer at /Applications/Julia-1.9.app/Contents/Resources/julia/lib/julia/libjulia-internal.1.9.dylib (unknown line)
ijl_init_restored_modules at /Applications/Julia-1.9.app/Contents/Resources/julia/lib/julia/libjulia-internal.1.9.dylib (unknown line)
register_restored_modules at ./loading.jl:1074
_include_from_serialized at ./loading.jl:1020
_require_search_from_serialized at ./loading.jl:1465
_require at ./loading.jl:1742
_require_prelocked at ./loading.jl:1619
macro expansion at ./loading.jl:1607 [inlined]
macro expansion at ./lock.jl:267 [inlined]
require at ./loading.jl:1570
jfptr_require_28442 at /Applications/Julia-1.9.app/Contents/Resources/julia/lib/julia/sys.dylib (unknown line)
ijl_apply_generic at /Applications/Julia-1.9.app/Contents/Resources/julia/lib/julia/libjulia-internal.1.9.dylib (unknown line)
eval_import_path at /Applications/Julia-1.9.app/Contents/Resources/julia/lib/julia/libjulia-internal.1.9.dylib (unknown line)
jl_toplevel_eval_flex at /Applications/Julia-1.9.app/Contents/Resources/julia/lib/julia/libjulia-internal.1.9.dylib (unknown line)
jl_toplevel_eval_flex at /Applications/Julia-1.9.app/Contents/Resources/julia/lib/julia/libjulia-internal.1.9.dylib (unknown line)
ijl_toplevel_eval_in at /Applications/Julia-1.9.app/Contents/Resources/julia/lib/julia/libjulia-internal.1.9.dylib (unknown line)
jlplt_ijl_toplevel_eval_in_12236 at /Applications/Julia-1.9.app/Contents/Resources/julia/lib/julia/sys.dylib (unknown line)
eval at ./boot.jl:370 [inlined]
exec_options at ./client.jl:280
_start at ./client.jl:522
jfptr__start_30743 at /Applications/Julia-1.9.app/Contents/Resources/julia/lib/julia/sys.dylib (unknown line)
ijl_apply_generic at /Applications/Julia-1.9.app/Contents/Resources/julia/lib/julia/libjulia-internal.1.9.dylib (unknown line)
true_main at /Applications/Julia-1.9.app/Contents/Resources/julia/lib/julia/libjulia-internal.1.9.dylib (unknown line)
jl_repl_entrypoint at /Applications/Julia-1.9.app/Contents/Resources/julia/lib/julia/libjulia-internal.1.9.dylib (unknown line)
Allocations: 2995 (Pool: 2984; Big: 11); GC: 0

Originally observed in JuliaGPU/CUDA.jl#1798

I have created a simple reproducer here:
https://github.com/simonbyrne/undef

[KristofferC]

@vchuravy, this reproduces on 1.8 (and 1.7) so I am not sure the milestone is warranted. Sure, the error mode is a bit harsher now but the bug is presumably the same.

[vchuravy]

A segmentation fault is not something a user can guard against. I will try to spend some cycles on it today or tomorrow.

[vchuravy]

RR trace available: https://julialang-dumps.s3.amazonaws.com/reports/2023-03-16T15-21-22-vchuravy.tar.zst

[vchuravy]

The seqfault is "just" that we expect the global to exist...

On the other hand, this seems clearly wrong:

julia/base/compiler/abstractinterpretation.jl

Lines 2412 to 2423 in 551f303

	elseif isa(sym, Symbol)
	if isdefined(sv.mod, sym)
	t = Const(true)
	elseif InferenceParams(interp).assume_bindings_static
	t = Const(false)
	end
	elseif isa(sym, GlobalRef)
	if isdefined(sym.mod, sym.name)
	t = Const(true)
	elseif InferenceParams(interp).assume_bindings_static
	t = Const(false)
	end

[simeonschaub]

I'm not sure there is an easy fix for this without breaking a bunch of crucial optimizations. The assumptions that bindings can never be undefined once they have been accessed is pretty essential to most of our handling of globals. Could you instead initialize the global as nothing first?

[vchuravy]

So from my brief investigation we have made the assumption throughout inference and codegen that:

A GlobalRef seen valid once will always be defined. This assumption holds in the context of JIT since we don't have a undefine this GlobalRef operation in the language. In the context of caching this assumption fails since we are not gurantueed to re-execute the codepath that defined the global.

One solution I thought about was tracking the context in which the global is defined? I.e. if it is a global in a top-level constant the assumption is fine, if it is a global defined from a "function execution/compilation" the assumption is not okay to make.

A long-term solution to this is @vtjnash proposal in #8870 (comment) i.e. extend the world-age semantics to global-ref to track the range of validity.

Either way this will require rather invasive changes through-out inference and the compiler :/