From 9e8fe688c5e32bde3ab48bb71f9d4ab45ef272ee Mon Sep 17 00:00:00 2001 From: Valentin Churavy Date: Fri, 24 Nov 2023 17:35:51 -0500 Subject: [PATCH] Turn Method Overwritten Error into a PrecompileError -- turning off caching (#52214) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #52213 Overwritting methods during cache creation is currently not something that the system can support and can lead to surprising, counter-intuitive and fatal errors. In 1.10 we turned it from a warning to a strong error, with this PR it remains a strong error, but the precompilation system recognizes it and essentially sets `__precompile__(false)` for this module and all modules that depend on it. Before: ``` julia> using OverwriteMethodError [ Info: Precompiling OverwriteMethodError [top-level] WARNING: Method definition +(Bool, Bool) in module Base at bool.jl:166 overwritten in module OverwriteMethodError at /home/vchuravy/src/julia2/OverwriteMethodError.jl:2. ERROR: LoadError: Method overwriting is not permitted during Module precompile. Stacktrace: [1] top-level scope @ ~/src/julia2/OverwriteMethodError.jl:2 [2] include @ Base ./Base.jl:489 [inlined] [3] include_package_for_output(pkg::Base.PkgId, input::String, depot_path::Vector{String}, dl_load_path::Vector{String}, load_path::Vector{String}, concrete_deps::Vector{Pair{Base.PkgId, UInt128}}, source::Nothing) @ Base ./loading.jl:2216 [4] top-level scope @ stdin:3 in expression starting at /home/vchuravy/src/julia2/OverwriteMethodError.jl:1 in expression starting at stdin:3 ERROR: Failed to precompile OverwriteMethodError [top-level] to "/home/vchuravy/.julia/compiled/v1.10/jl_guiuCQ". Stacktrace: [1] error(s::String) @ Base ./error.jl:35 [2] compilecache(pkg::Base.PkgId, path::String, internal_stderr::IO, internal_stdout::IO, keep_loaded_modules::Bool) @ Base ./loading.jl:2462 [3] compilecache @ Base ./loading.jl:2334 [inlined] [4] (::Base.var"#968#969"{Base.PkgId})() @ Base ./loading.jl:1968 [5] mkpidlock(f::Base.var"#968#969"{Base.PkgId}, at::String, pid::Int32; kwopts::@Kwargs{stale_age::Int64, wait::Bool}) @ FileWatching.Pidfile ~/.julia/juliaup/julia-1.10.0-rc1+0.x64.linux.gnu/share/julia/stdlib/v1.10/FileWatching/src/pidfile.jl:93 [6] #mkpidlock#6 @ FileWatching.Pidfile ~/.julia/juliaup/julia-1.10.0-rc1+0.x64.linux.gnu/share/julia/stdlib/v1.10/FileWatching/src/pidfile.jl:88 [inlined] [7] trymkpidlock(::Function, ::Vararg{Any}; kwargs::@Kwargs{stale_age::Int64}) @ FileWatching.Pidfile ~/.julia/juliaup/julia-1.10.0-rc1+0.x64.linux.gnu/share/julia/stdlib/v1.10/FileWatching/src/pidfile.jl:111 [8] #invokelatest#2 @ Base ./essentials.jl:889 [inlined] [9] invokelatest @ Base ./essentials.jl:884 [inlined] [10] maybe_cachefile_lock(f::Base.var"#968#969"{Base.PkgId}, pkg::Base.PkgId, srcpath::String; stale_age::Int64) @ Base ./loading.jl:2977 [11] maybe_cachefile_lock @ Base ./loading.jl:2974 [inlined] [12] _require(pkg::Base.PkgId, env::String) @ Base ./loading.jl:1964 [13] __require_prelocked(uuidkey::Base.PkgId, env::String) @ Base ./loading.jl:1806 [14] #invoke_in_world#3 @ Base ./essentials.jl:921 [inlined] [15] invoke_in_world @ Base ./essentials.jl:918 [inlined] [16] _require_prelocked(uuidkey::Base.PkgId, env::String) @ Base ./loading.jl:1797 [17] macro expansion @ Base ./loading.jl:1784 [inlined] [18] macro expansion @ Base ./lock.jl:267 [inlined] [19] __require(into::Module, mod::Symbol) @ Base ./loading.jl:1747 [20] #invoke_in_world#3 @ Base ./essentials.jl:921 [inlined] [21] invoke_in_world @ Base ./essentials.jl:918 [inlined] [22] require(into::Module, mod::Symbol) @ Base ./loading.jl:1740 ``` After: ``` julia> using OverwriteMethodError ┌ Info: Precompiling OverwriteMethodError [top-level] └ @ Base loading.jl:2486 WARNING: Method definition +(Bool, Bool) in module Base at bool.jl:166 overwritten in module OverwriteMethodError at /home/vchuravy/src/julia2/OverwriteMethodError.jl:2. ERROR: Method overwriting is not permitted during Module precompile. ┌ Info: Skipping precompilation since __precompile__(false). Importing OverwriteMethodError [top-level]. └ @ Base loading.jl:2084 ``` --------- Co-authored-by: Kristoffer Carlsson --- base/boot.jl | 2 ++ base/loading.jl | 2 +- src/gf.c | 6 ++++-- src/jl_exported_data.inc | 1 + src/jltypes.c | 1 + src/julia.h | 1 + src/staticdata.c | 3 ++- test/precompile.jl | 10 ++++++++++ 8 files changed, 22 insertions(+), 4 deletions(-) diff --git a/base/boot.jl b/base/boot.jl index 056b14fa5d0ac..2dc19528e178e 100644 --- a/base/boot.jl +++ b/base/boot.jl @@ -413,6 +413,8 @@ struct InitError <: WrappedException error end +struct PrecompilableError <: Exception end + String(s::String) = s # no constructor yet const Cvoid = Nothing diff --git a/base/loading.jl b/base/loading.jl index 9a3a922298f1e..95b83e84aad15 100644 --- a/base/loading.jl +++ b/base/loading.jl @@ -1745,7 +1745,7 @@ function include_dependency(path::AbstractString) end # we throw PrecompilableError when a module doesn't want to be precompiled -struct PrecompilableError <: Exception end +import Core: PrecompilableError function show(io::IO, ex::PrecompilableError) print(io, "Error when precompiling module, potentially caused by a __precompile__(false) declaration in the module.") end diff --git a/src/gf.c b/src/gf.c index fffeb928a1889..5013706030777 100644 --- a/src/gf.c +++ b/src/gf.c @@ -1572,8 +1572,10 @@ static void method_overwrite(jl_typemap_entry_t *newentry, jl_method_t *oldvalue jl_printf(s, ".\n"); jl_uv_flush(s); } - if (jl_generating_output()) - jl_error("Method overwriting is not permitted during Module precompile."); + if (jl_generating_output()) { + jl_printf(JL_STDERR, "ERROR: Method overwriting is not permitted during Module precompilation. Use `__precompile__(false)` to opt-out of precompilation.\n"); + jl_throw(jl_precompilable_error); + } } static void update_max_args(jl_methtable_t *mt, jl_value_t *type) diff --git a/src/jl_exported_data.inc b/src/jl_exported_data.inc index b6b9774c1538e..1a843c71f0e1e 100644 --- a/src/jl_exported_data.inc +++ b/src/jl_exported_data.inc @@ -145,6 +145,7 @@ XX(jl_weakref_type) \ XX(jl_libdl_module) \ XX(jl_libdl_dlopen_func) \ + XX(jl_precompilable_error) \ // Data symbols that are defined inside the public libjulia #define JL_EXPORTED_DATA_SYMBOLS(XX) \ diff --git a/src/jltypes.c b/src/jltypes.c index c7c7d213c5317..a30d746bbb320 100644 --- a/src/jltypes.c +++ b/src/jltypes.c @@ -3510,6 +3510,7 @@ void post_boot_hooks(void) jl_loaderror_type = (jl_datatype_t*)core("LoadError"); jl_initerror_type = (jl_datatype_t*)core("InitError"); jl_missingcodeerror_type = (jl_datatype_t*)core("MissingCodeError"); + jl_precompilable_error = jl_new_struct_uninit((jl_datatype_t*)core("PrecompilableError")); jl_pair_type = core("Pair"); jl_kwcall_func = core("kwcall"); jl_kwcall_mt = ((jl_datatype_t*)jl_typeof(jl_kwcall_func))->name->mt; diff --git a/src/julia.h b/src/julia.h index 5198bcab5a2bd..d728458f50180 100644 --- a/src/julia.h +++ b/src/julia.h @@ -837,6 +837,7 @@ extern JL_DLLIMPORT jl_value_t *jl_readonlymemory_exception JL_GLOBALLY_ROOTED; extern JL_DLLIMPORT jl_value_t *jl_diverror_exception JL_GLOBALLY_ROOTED; extern JL_DLLIMPORT jl_value_t *jl_undefref_exception JL_GLOBALLY_ROOTED; extern JL_DLLIMPORT jl_value_t *jl_interrupt_exception JL_GLOBALLY_ROOTED; +extern JL_DLLIMPORT jl_value_t *jl_precompilable_error JL_GLOBALLY_ROOTED; extern JL_DLLIMPORT jl_datatype_t *jl_boundserror_type JL_GLOBALLY_ROOTED; extern JL_DLLIMPORT jl_value_t *jl_an_empty_vec_any JL_GLOBALLY_ROOTED; extern JL_DLLIMPORT jl_value_t *jl_an_empty_memory_any JL_GLOBALLY_ROOTED; diff --git a/src/staticdata.c b/src/staticdata.c index 42a5fdb63b63e..52a354b4933d7 100644 --- a/src/staticdata.c +++ b/src/staticdata.c @@ -98,7 +98,7 @@ extern "C" { // TODO: put WeakRefs on the weak_refs list during deserialization // TODO: handle finalizers -#define NUM_TAGS 174 +#define NUM_TAGS 175 // An array of references that need to be restored from the sysimg // This is a manually constructed dual of the gvars array, which would be produced by codegen for Julia code, for C. @@ -237,6 +237,7 @@ jl_value_t **const*const get_tags(void) { INSERT_TAG(jl_readonlymemory_exception); INSERT_TAG(jl_atomicerror_type); INSERT_TAG(jl_missingcodeerror_type); + INSERT_TAG(jl_precompilable_error); // other special values INSERT_TAG(jl_emptysvec); diff --git a/test/precompile.jl b/test/precompile.jl index 051b750fa7fdf..1ac3999947736 100644 --- a/test/precompile.jl +++ b/test/precompile.jl @@ -571,6 +571,16 @@ precompile_test_harness(false) do dir @test Base.compilecache(Base.PkgId("Baz")) == Base.PrecompilableError() # due to __precompile__(false) + OverwriteMethodError_file = joinpath(dir, "OverwriteMethodError.jl") + write(OverwriteMethodError_file, + """ + module OverwriteMethodError + Base.:(+)(x::Bool, y::Bool) = false + end + """) + + @test Base.compilecache(Base.PkgId("OverwriteMethodError")) == Base.PrecompilableError() # due to piracy + UseBaz_file = joinpath(dir, "UseBaz.jl") write(UseBaz_file, """