From a1854224d5daaccad8036888edb3cbb9f382b9e6 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Mon, 19 Apr 2021 05:40:59 -0400 Subject: [PATCH] safepoints are required in any lock than may be used with allocations (#40487) (which is pretty much all locks) (cherry picked from commit 399ec046c4e84c3e5dcd91ba8203f9f3c8d3d054) --- src/codegen.cpp | 5 ++++- src/dlload.c | 14 +------------- src/gc.c | 9 ++++----- src/gf.c | 8 ++++---- src/julia.h | 14 +++++++------- src/julia_internal.h | 7 +++---- src/module.c | 36 ++++++++++++++++++------------------ src/runtime_ccall.cpp | 38 ++++++++++++++++++-------------------- 8 files changed, 59 insertions(+), 72 deletions(-) diff --git a/src/codegen.cpp b/src/codegen.cpp index f3abdf10ae897a..ddad461154b38a 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -607,7 +607,10 @@ static const auto jlegal_func = new JuliaFunction{ [](LLVMContext &C) { Type *T = PointerType::get(T_jlvalue, AddressSpace::CalleeRooted); return FunctionType::get(T_int32, {T, T}, false); }, - nullptr, + [](LLVMContext &C) { return AttributeList::get(C, + Attributes(C, {Attribute::ReadOnly, Attribute::NoUnwind, Attribute::ArgMemOnly}), + AttributeSet(), + None); }, }; static const auto jl_alloc_obj_func = new JuliaFunction{ "julia.gc_alloc_obj", diff --git a/src/dlload.c b/src/dlload.c index ecb34d2be57e33..71af6238b4cf61 100644 --- a/src/dlload.c +++ b/src/dlload.c @@ -153,7 +153,7 @@ JL_DLLEXPORT int jl_dlclose(void *handle) JL_NOTSAFEPOINT #endif } -JL_DLLEXPORT void *jl_load_dynamic_library(const char *modname, unsigned flags, int throw_err) JL_NOTSAFEPOINT // (or throw) +JL_DLLEXPORT void *jl_load_dynamic_library(const char *modname, unsigned flags, int throw_err) { char path[PATHBUF], relocated[PATHBUF]; int i; @@ -175,20 +175,12 @@ JL_DLLEXPORT void *jl_load_dynamic_library(const char *modname, unsigned flags, if (!GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, (LPCWSTR)(uintptr_t)(&jl_load_dynamic_library), (HMODULE*)&handle)) { -#ifndef __clang_analyzer__ - // Hide the error throwing from the analyser since there isn't a way to express - // "safepoint only when throwing error" currently. jl_error("could not load base module"); -#endif } #else Dl_info info; if (!dladdr((void*)(uintptr_t)&jl_load_dynamic_library, &info) || !info.dli_fname) { -#ifndef __clang_analyzer__ - // Hide the error throwing from the analyser since there isn't a way to express - // "safepoint only when throwing error" currently. jl_error("could not load base module"); -#endif } handle = dlopen(info.dli_fname, RTLD_NOW); #endif @@ -271,11 +263,7 @@ JL_DLLEXPORT void *jl_load_dynamic_library(const char *modname, unsigned flags, #else const char *reason = dlerror(); #endif -#ifndef __clang_analyzer__ - // Hide the error throwing from the analyser since there isn't a way to express - // "safepoint only when throwing error" currently. jl_errorf("could not load library \"%s\"\n%s", modname, reason); -#endif } handle = NULL; diff --git a/src/gc.c b/src/gc.c index 94f5a80fd0cbea..df06d7e5a6e7f6 100644 --- a/src/gc.c +++ b/src/gc.c @@ -480,9 +480,9 @@ void jl_gc_run_all_finalizers(jl_ptls_t ptls) run_finalizers(ptls); } -static void gc_add_finalizer_(jl_ptls_t ptls, void *v, void *f) +static void gc_add_finalizer_(jl_ptls_t ptls, void *v, void *f) JL_NOTSAFEPOINT { - int8_t gc_state = jl_gc_unsafe_enter(ptls); + assert(ptls->gc_state == 0); arraylist_t *a = &ptls->finalizers; // This acquire load and the release store at the end are used to // synchronize with `finalize_object` on another thread. Apart from the GC, @@ -506,15 +506,14 @@ static void gc_add_finalizer_(jl_ptls_t ptls, void *v, void *f) items[oldlen] = v; items[oldlen + 1] = f; jl_atomic_store_release(&a->len, oldlen + 2); - jl_gc_unsafe_leave(ptls, gc_state); } -JL_DLLEXPORT void jl_gc_add_ptr_finalizer(jl_ptls_t ptls, jl_value_t *v, void *f) +JL_DLLEXPORT void jl_gc_add_ptr_finalizer(jl_ptls_t ptls, jl_value_t *v, void *f) JL_NOTSAFEPOINT { gc_add_finalizer_(ptls, (void*)(((uintptr_t)v) | 1), f); } -JL_DLLEXPORT void jl_gc_add_finalizer_th(jl_ptls_t ptls, jl_value_t *v, jl_function_t *f) +JL_DLLEXPORT void jl_gc_add_finalizer_th(jl_ptls_t ptls, jl_value_t *v, jl_function_t *f) JL_NOTSAFEPOINT { if (__unlikely(jl_typeis(f, jl_voidpointer_type))) { jl_gc_add_ptr_finalizer(ptls, v, jl_unbox_voidpointer(f)); diff --git a/src/gf.c b/src/gf.c index cf22c9b0095a3f..33ec6003c95f94 100644 --- a/src/gf.c +++ b/src/gf.c @@ -1363,7 +1363,7 @@ static void invalidate_method_instance(jl_method_instance_t *replaced, size_t ma } if (!jl_is_method(replaced->def.method)) return; // shouldn't happen, but better to be safe - JL_LOCK_NOGC(&replaced->def.method->writelock); + JL_LOCK(&replaced->def.method->writelock); jl_code_instance_t *codeinst = replaced->cache; while (codeinst) { if (codeinst->max_world == ~(size_t)0) { @@ -1383,13 +1383,13 @@ static void invalidate_method_instance(jl_method_instance_t *replaced, size_t ma invalidate_method_instance(replaced, max_world, depth + 1); } } - JL_UNLOCK_NOGC(&replaced->def.method->writelock); + JL_UNLOCK(&replaced->def.method->writelock); } // invalidate cached methods that overlap this definition static void invalidate_backedges(jl_method_instance_t *replaced_mi, size_t max_world, const char *why) { - JL_LOCK_NOGC(&replaced_mi->def.method->writelock); + JL_LOCK(&replaced_mi->def.method->writelock); jl_array_t *backedges = replaced_mi->backedges; if (backedges) { // invalidate callers (if any) @@ -1400,7 +1400,7 @@ static void invalidate_backedges(jl_method_instance_t *replaced_mi, size_t max_w invalidate_method_instance(replaced[i], max_world, 1); } } - JL_UNLOCK_NOGC(&replaced_mi->def.method->writelock); + JL_UNLOCK(&replaced_mi->def.method->writelock); if (why && _jl_debug_method_invalidation) { jl_array_ptr_1d_push(_jl_debug_method_invalidation, (jl_value_t*)replaced_mi); jl_value_t *loctag = jl_cstr_to_string(why); diff --git a/src/julia.h b/src/julia.h index b445404e76046c..9bd41f639a20c4 100644 --- a/src/julia.h +++ b/src/julia.h @@ -808,7 +808,8 @@ typedef enum { JL_DLLEXPORT void jl_gc_collect(jl_gc_collection_t); -JL_DLLEXPORT void jl_gc_add_finalizer(jl_value_t *v, jl_function_t *f); +JL_DLLEXPORT void jl_gc_add_finalizer(jl_value_t *v, jl_function_t *f) JL_NOTSAFEPOINT; +JL_DLLEXPORT void jl_gc_add_ptr_finalizer(jl_ptls_t ptls, jl_value_t *v, void *f) JL_NOTSAFEPOINT; JL_DLLEXPORT void jl_finalize(jl_value_t *o); JL_DLLEXPORT jl_weakref_t *jl_gc_new_weakref(jl_value_t *value); JL_DLLEXPORT jl_value_t *jl_gc_alloc_0w(void); @@ -1470,11 +1471,10 @@ JL_DLLEXPORT jl_binding_t *jl_get_binding_or_error(jl_module_t *m, jl_sym_t *var JL_DLLEXPORT jl_value_t *jl_module_globalref(jl_module_t *m, jl_sym_t *var); // get binding for assignment JL_DLLEXPORT jl_binding_t *jl_get_binding_wr(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var, int error); -JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m JL_PROPAGATES_ROOT, - jl_sym_t *var); +JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var); JL_DLLEXPORT int jl_boundp(jl_module_t *m, jl_sym_t *var); -JL_DLLEXPORT int jl_defines_or_exports_p(jl_module_t *m, jl_sym_t *var) JL_NOTSAFEPOINT; -JL_DLLEXPORT int jl_binding_resolved_p(jl_module_t *m, jl_sym_t *var) JL_NOTSAFEPOINT; +JL_DLLEXPORT int jl_defines_or_exports_p(jl_module_t *m, jl_sym_t *var); +JL_DLLEXPORT int jl_binding_resolved_p(jl_module_t *m, jl_sym_t *var); JL_DLLEXPORT int jl_is_const(jl_module_t *m, jl_sym_t *var); JL_DLLEXPORT jl_value_t *jl_get_global(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var); JL_DLLEXPORT void jl_set_global(jl_module_t *m JL_ROOTING_ARGUMENT, jl_sym_t *var, jl_value_t *val JL_ROOTED_ARGUMENT); @@ -1488,7 +1488,7 @@ JL_DLLEXPORT void jl_module_import(jl_module_t *to, jl_module_t *from, jl_sym_t JL_DLLEXPORT void jl_module_import_as(jl_module_t *to, jl_module_t *from, jl_sym_t *s, jl_sym_t *asname); JL_DLLEXPORT void jl_module_export(jl_module_t *from, jl_sym_t *s); 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_NOTSAFEPOINT; +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) { @@ -1642,7 +1642,7 @@ enum JL_RTLD_CONSTANT { #define JL_RTLD_DEFAULT (JL_RTLD_LAZY | JL_RTLD_DEEPBIND) typedef void *jl_uv_libhandle; // compatible with dlopen (void*) / LoadLibrary (HMODULE) -JL_DLLEXPORT jl_uv_libhandle jl_load_dynamic_library(const char *fname, unsigned flags, int throw_err) JL_NOTSAFEPOINT; +JL_DLLEXPORT jl_uv_libhandle jl_load_dynamic_library(const char *fname, unsigned flags, int throw_err); JL_DLLEXPORT jl_uv_libhandle jl_dlopen(const char *filename, unsigned flags) JL_NOTSAFEPOINT; JL_DLLEXPORT int jl_dlclose(jl_uv_libhandle handle) JL_NOTSAFEPOINT; JL_DLLEXPORT int jl_dlsym(jl_uv_libhandle handle, const char *symbol, void ** value, int throw_err) JL_NOTSAFEPOINT; diff --git a/src/julia_internal.h b/src/julia_internal.h index 172492f540e0ac..d1c040d8ee71ac 100644 --- a/src/julia_internal.h +++ b/src/julia_internal.h @@ -527,7 +527,7 @@ void jl_compute_field_offsets(jl_datatype_t *st); jl_array_t *jl_new_array_for_deserialization(jl_value_t *atype, uint32_t ndims, size_t *dims, int isunboxed, int hasptr, int isunion, int elsz); void jl_module_run_initializer(jl_module_t *m); -jl_binding_t *jl_get_module_binding(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var) JL_NOTSAFEPOINT; +jl_binding_t *jl_get_module_binding(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var); void jl_binding_deprecation_warning(jl_module_t *m, jl_binding_t *b); extern jl_array_t *jl_module_init_order JL_GLOBALLY_ROOTED; extern htable_t jl_current_modules JL_GLOBALLY_ROOTED; @@ -1004,10 +1004,9 @@ extern void *jl_crtdll_handle; extern void *jl_winsock_handle; #endif -void *jl_get_library_(const char *f_lib, int throw_err) JL_NOTSAFEPOINT; +void *jl_get_library_(const char *f_lib, int throw_err); #define jl_get_library(f_lib) jl_get_library_(f_lib, 1) -JL_DLLEXPORT void *jl_load_and_lookup(const char *f_lib, const char *f_name, - void **hnd) JL_NOTSAFEPOINT; +JL_DLLEXPORT void *jl_load_and_lookup(const char *f_lib, const char *f_name, void **hnd); JL_DLLEXPORT void *jl_lazy_load_and_lookup(jl_value_t *lib_val, const char *f_name); JL_DLLEXPORT jl_value_t *jl_get_cfunction_trampoline( jl_value_t *fobj, jl_datatype_t *result, htable_t *cache, jl_svec_t *fill, diff --git a/src/module.c b/src/module.c index 20c119bedc27c3..231efbb357653d 100644 --- a/src/module.c +++ b/src/module.c @@ -150,7 +150,7 @@ static jl_binding_t *new_binding(jl_sym_t *name) // get binding for assignment JL_DLLEXPORT jl_binding_t *jl_get_binding_wr(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var, int error) { - JL_LOCK_NOGC(&m->lock); + JL_LOCK(&m->lock); jl_binding_t **bp = (jl_binding_t**)ptrhash_bp(&m->bindings, var); jl_binding_t *b = *bp; @@ -160,7 +160,7 @@ JL_DLLEXPORT jl_binding_t *jl_get_binding_wr(jl_module_t *m JL_PROPAGATES_ROOT, b->owner = m; } else if (error) { - JL_UNLOCK_NOGC(&m->lock); + JL_UNLOCK(&m->lock); jl_errorf("cannot assign a value to variable %s.%s from module %s", jl_symbol_name(b->owner->name), jl_symbol_name(var), jl_symbol_name(m->name)); } @@ -170,10 +170,11 @@ JL_DLLEXPORT jl_binding_t *jl_get_binding_wr(jl_module_t *m JL_PROPAGATES_ROOT, b = new_binding(var); b->owner = m; *bp = b; + JL_GC_PROMISE_ROOTED(b); jl_gc_wb_buf(m, b, sizeof(jl_binding_t)); } - JL_UNLOCK_NOGC(&m->lock); + JL_UNLOCK(&m->lock); return b; } @@ -208,7 +209,7 @@ JL_DLLEXPORT jl_module_t *jl_get_module_of_binding(jl_module_t *m, jl_sym_t *var // like jl_get_binding_wr, but has different error paths JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m, jl_sym_t *var) { - JL_LOCK_NOGC(&m->lock); + JL_LOCK(&m->lock); jl_binding_t **bp = _jl_get_module_binding_bp(m, var); jl_binding_t *b = *bp; @@ -218,7 +219,7 @@ JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m, jl_sym_ b->owner = m; } else { - JL_UNLOCK_NOGC(&m->lock); + JL_UNLOCK(&m->lock); jl_binding_t *b2 = jl_get_binding(b->owner, b->name); if (b2 == NULL || b2->value == NULL) jl_errorf("invalid method definition: imported function %s.%s does not exist", @@ -239,7 +240,7 @@ JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m, jl_sym_ jl_gc_wb_buf(m, b, sizeof(jl_binding_t)); } - JL_UNLOCK_NOGC(&m->lock); + JL_UNLOCK(&m->lock); return b; } @@ -583,33 +584,33 @@ JL_DLLEXPORT int jl_boundp(jl_module_t *m, jl_sym_t *var) JL_DLLEXPORT int jl_defines_or_exports_p(jl_module_t *m, jl_sym_t *var) { - JL_LOCK_NOGC(&m->lock); + JL_LOCK(&m->lock); jl_binding_t *b = (jl_binding_t*)ptrhash_get(&m->bindings, var); - JL_UNLOCK_NOGC(&m->lock); + JL_UNLOCK(&m->lock); return b != HT_NOTFOUND && (b->exportp || b->owner==m); } -JL_DLLEXPORT int jl_module_exports_p(jl_module_t *m, jl_sym_t *var) JL_NOTSAFEPOINT +JL_DLLEXPORT int jl_module_exports_p(jl_module_t *m, jl_sym_t *var) { - JL_LOCK_NOGC(&m->lock); + JL_LOCK(&m->lock); jl_binding_t *b = _jl_get_module_binding(m, var); - JL_UNLOCK_NOGC(&m->lock); + JL_UNLOCK(&m->lock); return b != HT_NOTFOUND && b->exportp; } -JL_DLLEXPORT int jl_binding_resolved_p(jl_module_t *m, jl_sym_t *var) JL_NOTSAFEPOINT +JL_DLLEXPORT int jl_binding_resolved_p(jl_module_t *m, jl_sym_t *var) { - JL_LOCK_NOGC(&m->lock); + JL_LOCK(&m->lock); jl_binding_t *b = _jl_get_module_binding(m, var); - JL_UNLOCK_NOGC(&m->lock); + JL_UNLOCK(&m->lock); return b != HT_NOTFOUND && b->owner != NULL; } -JL_DLLEXPORT jl_binding_t *jl_get_module_binding(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var) JL_NOTSAFEPOINT +JL_DLLEXPORT jl_binding_t *jl_get_module_binding(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var) { - JL_LOCK_NOGC(&m->lock); + JL_LOCK(&m->lock); jl_binding_t *b = _jl_get_module_binding(m, var); - JL_UNLOCK_NOGC(&m->lock); + JL_UNLOCK(&m->lock); return b == HT_NOTFOUND ? NULL : b; } @@ -626,7 +627,6 @@ JL_DLLEXPORT void jl_set_global(jl_module_t *m JL_ROOTING_ARGUMENT, jl_sym_t *va JL_TYPECHK(jl_set_global, module, (jl_value_t*)m); JL_TYPECHK(jl_set_global, symbol, (jl_value_t*)var); jl_binding_t *bp = jl_get_binding_wr(m, var, 1); - JL_GC_PROMISE_ROOTED(bp); jl_checked_assignment(bp, val); } diff --git a/src/runtime_ccall.cpp b/src/runtime_ccall.cpp index 307acc9c28cd16..050347513aa45d 100644 --- a/src/runtime_ccall.cpp +++ b/src/runtime_ccall.cpp @@ -27,9 +27,8 @@ using namespace llvm; static std::map libMap; static jl_mutex_t libmap_lock; extern "C" -void *jl_get_library_(const char *f_lib, int throw_err) JL_NOTSAFEPOINT +void *jl_get_library_(const char *f_lib, int throw_err) { - void *hnd; if (f_lib == NULL) return jl_RTLD_DEFAULT_handle; #ifdef _OS_WINDOWS_ @@ -40,23 +39,22 @@ void *jl_get_library_(const char *f_lib, int throw_err) JL_NOTSAFEPOINT if (strcmp(f_lib, JL_LIBJULIA_DL_LIBNAME) == 0) return jl_libjulia_handle; #endif - JL_LOCK_NOGC(&libmap_lock); + JL_LOCK(&libmap_lock); // This is the only operation we do on the map, which doesn't invalidate // any references or iterators. void **map_slot = &libMap[f_lib]; - JL_UNLOCK_NOGC(&libmap_lock); - hnd = jl_atomic_load_acquire(map_slot); - if (hnd != NULL) - return hnd; - // We might run this concurrently on two threads but it doesn't matter. - hnd = jl_load_dynamic_library(f_lib, JL_RTLD_DEFAULT, throw_err); - if (hnd != NULL) - jl_atomic_store_release(map_slot, hnd); + void *hnd = *map_slot; + if (hnd == NULL) { + hnd = jl_load_dynamic_library(f_lib, JL_RTLD_DEFAULT, throw_err); + if (hnd != NULL) + *map_slot = hnd; + } + JL_UNLOCK(&libmap_lock); return hnd; } extern "C" JL_DLLEXPORT -void *jl_load_and_lookup(const char *f_lib, const char *f_name, void **hnd) JL_NOTSAFEPOINT +void *jl_load_and_lookup(const char *f_lib, const char *f_name, void **hnd) { void *handle = jl_atomic_load_acquire(hnd); if (!handle) @@ -210,11 +208,11 @@ extern "C" JL_DLLEXPORT char *jl_format_filename(const char *output_pattern) } -static jl_mutex_t trampoline_lock; // for accesses to the cache and freelist +static jl_mutex_t trampoline_lock; // for accesses to the cache and freelist static void *trampoline_freelist; -static void *trampoline_alloc() // lock taken by caller +static void *trampoline_alloc() JL_NOTSAFEPOINT // lock taken by caller { const int sz = 64; // oversized for most platforms. todo: use precise value? if (!trampoline_freelist) { @@ -235,6 +233,7 @@ static void *trampoline_alloc() // lock taken by caller #endif errno = last_errno; void *next = NULL; + assert(sz < jl_page_size); for (size_t i = 0; i + sz <= jl_page_size; i += sz) { void **curr = (void**)((char*)mem + i); *curr = next; @@ -272,6 +271,8 @@ static void trampoline_deleter(void **f) JL_UNLOCK_NOGC(&trampoline_lock); } +typedef void *(*init_trampoline_t)(void *tramp, void **nval) JL_NOTSAFEPOINT; + // Use of `cache` is not clobbered in JL_TRY JL_GCC_IGNORE_START("-Wclobbered") extern "C" JL_DLLEXPORT @@ -282,7 +283,7 @@ jl_value_t *jl_get_cfunction_trampoline( // call-site constants: htable_t *cache, // weakref htable indexed by (fobj, vals) jl_svec_t *fill, - void *(*init_trampoline)(void *tramp, void **nval), + init_trampoline_t init_trampoline, jl_unionall_t *env, jl_value_t **vals) { @@ -339,11 +340,8 @@ jl_value_t *jl_get_cfunction_trampoline( ((void**)result)[1] = (void*)fobj; } if (!permanent) { - void *ptr_finalizer[2] = { - (void*)jl_voidpointer_type, - (void*)&trampoline_deleter - }; - jl_gc_add_finalizer(result, (jl_value_t*)&ptr_finalizer[1]); + jl_ptls_t ptls = jl_get_ptls_states(); + jl_gc_add_ptr_finalizer(ptls, result, (void*)(uintptr_t)&trampoline_deleter); ((void**)result)[2] = (void*)cache; ((void**)result)[3] = (void*)nval; }