manage stackwalk/profile lock correctly (#45549)

Previously we had the wrong number of them (it was not global) and it
was in the wrong shared library (it was part of codegen instead of
profiling).

The profile_round_robin_thread_order buffer was not allocated on the
thread that exclusively used it, so it sometimes could end up somewhat
awkwardly (in incorrectly) shared between 2 threads.

Author: vtjnash

src/codegen-stubs.c

@@ -66,14 +66,6 @@ JL_DLLEXPORT size_t jl_jit_total_bytes_fallback(void)
     return 0;
 }
 
-JL_DLLEXPORT void jl_lock_profile_fallback(void)
-{
-}
-
-JL_DLLEXPORT void jl_unlock_profile_fallback(void)
-{
-}
-
 JL_DLLEXPORT void *jl_create_native_fallback(jl_array_t *methods, LLVMOrcThreadSafeModuleRef llvmctxt, const jl_cgparams_t *cgparams, int _policy) UNAVAILABLE
 
 JL_DLLEXPORT void jl_dump_compiles_fallback(void *s)

src/debug-registry.h

@@ -15,7 +15,6 @@ typedef struct {
     int64_t slide;
 } objfileentry_t;
 
-
 // Central registry for resolving function addresses to `jl_method_instance_t`s and
 // originating `ObjectFile`s (for the DWARF debug info).
 //
@@ -81,32 +80,6 @@ class JITDebugInfoRegistry
         ~Locked() JL_NOTSAFEPOINT = default;
     };
 
-    template<typename datatype>
-    struct jl_pthread_key_t {
-        static_assert(std::is_trivially_default_constructible<datatype>::value, "Invalid datatype for pthread key!");
-        static_assert(std::is_trivially_destructible<datatype>::value, "Expected datatype to be trivially destructible!");
-        static_assert(sizeof(datatype) == sizeof(void*), "Expected datatype to be like a void*!");
-        pthread_key_t key;
-
-        void init() JL_NOTSAFEPOINT {
-            if (pthread_key_create(&key, NULL))
-                jl_error("fatal: pthread_key_create failed");
-        }
-
-        operator datatype() JL_NOTSAFEPOINT {
-            return reinterpret_cast<datatype>(pthread_getspecific(key));
-        }
-
-        jl_pthread_key_t &operator=(datatype val) JL_NOTSAFEPOINT {
-            pthread_setspecific(key, reinterpret_cast<void*>(val));
-            return *this;
-        }
-
-        void destroy() JL_NOTSAFEPOINT {
-            pthread_key_delete(key);
-        }
-    };
-
     struct sysimg_info_t {
         uint64_t jl_sysimage_base;
         jl_sysimg_fptrs_t sysimg_fptrs;
@@ -159,16 +132,6 @@ class JITDebugInfoRegistry
     JITDebugInfoRegistry() JL_NOTSAFEPOINT;
     ~JITDebugInfoRegistry() JL_NOTSAFEPOINT = default;
 
-    // Any function that acquires this lock must be either a unmanaged thread
-    // or in the GC safe region and must NOT allocate anything through the GC
-    // while holding this lock.
-    // Certain functions in this file might be called from an unmanaged thread
-    // and cannot have any interaction with the julia runtime
-    // They also may be re-entrant, and operating while threads are paused, so we
-    // separately manage the re-entrant count behavior for safety across platforms
-    // Note that we cannot safely upgrade read->write
-    uv_rwlock_t debuginfo_asyncsafe{};
-    jl_pthread_key_t<uintptr_t> debuginfo_asyncsafe_held{};
     libc_frames_t libc_frames{};
 
     void add_code_in_flight(llvm::StringRef name, jl_code_instance_t *codeinst, const llvm::DataLayout &DL) JL_NOTSAFEPOINT;

src/debuginfo.cpp

@@ -52,12 +52,6 @@ struct debug_link_info {
     uint32_t crc32;
 };
 
-extern "C" JL_DLLEXPORT void jl_lock_profile_impl(void) JL_NOTSAFEPOINT;
-extern "C" JL_DLLEXPORT void jl_unlock_profile_impl(void) JL_NOTSAFEPOINT;
-
-template <typename T>
-static void jl_profile_atomic(T f);
-
 #if (defined(_OS_LINUX_) || defined(_OS_FREEBSD_) || (defined(_OS_DARWIN_) && defined(LLVM_SHLIB)))
 extern "C" void __register_frame(void*);
 extern "C" void __deregister_frame(void*);
@@ -101,16 +95,16 @@ void JITDebugInfoRegistry::add_code_in_flight(StringRef name, jl_code_instance_t
 
 jl_method_instance_t *JITDebugInfoRegistry::lookupLinfo(size_t pointer) JL_NOTSAFEPOINT
 {
-    jl_lock_profile_impl();
+    jl_lock_profile();
     auto region = linfomap.lower_bound(pointer);
     jl_method_instance_t *linfo = NULL;
     if (region != linfomap.end() && pointer < region->first + region->second.first)
         linfo = region->second.second;
-    jl_unlock_profile_impl();
+    jl_unlock_profile();
     return linfo;
 }
 
-//Protected by debuginfo_asyncsafe
+//Protected by debuginfo_asyncsafe (profile) lock
 JITDebugInfoRegistry::objectmap_t &
 JITDebugInfoRegistry::getObjectMap() JL_NOTSAFEPOINT
 {
@@ -131,41 +125,21 @@ JITDebugInfoRegistry::get_objfile_map() JL_NOTSAFEPOINT {
     return *this->objfilemap;
 }
 
-JITDebugInfoRegistry::JITDebugInfoRegistry() JL_NOTSAFEPOINT {
-    uv_rwlock_init(&debuginfo_asyncsafe);
-    debuginfo_asyncsafe_held.init();
-}
+JITDebugInfoRegistry::JITDebugInfoRegistry() JL_NOTSAFEPOINT { }
 
 struct unw_table_entry
 {
     int32_t start_ip_offset;
     int32_t fde_offset;
 };
 
-extern "C" JL_DLLEXPORT void jl_lock_profile_impl(void) JL_NOTSAFEPOINT
-{
-    uintptr_t held = getJITDebugRegistry().debuginfo_asyncsafe_held;
-    if (held++ == 0)
-        uv_rwlock_rdlock(&getJITDebugRegistry().debuginfo_asyncsafe);
-    getJITDebugRegistry().debuginfo_asyncsafe_held = held;
-}
-
-extern "C" JL_DLLEXPORT void jl_unlock_profile_impl(void) JL_NOTSAFEPOINT
-{
-    uintptr_t held = getJITDebugRegistry().debuginfo_asyncsafe_held;
-    assert(held);
-    if (--held == 0)
-        uv_rwlock_rdunlock(&getJITDebugRegistry().debuginfo_asyncsafe);
-    getJITDebugRegistry().debuginfo_asyncsafe_held = held;
-}
-
 // some actions aren't signal (especially profiler) safe so we acquire a lock
 // around them to establish a mutual exclusion with unwinding from a signal
 template <typename T>
 static void jl_profile_atomic(T f)
 {
-    assert(0 == getJITDebugRegistry().debuginfo_asyncsafe_held);
-    uv_rwlock_wrlock(&getJITDebugRegistry().debuginfo_asyncsafe);
+    assert(0 == jl_lock_profile_rd_held());
+    jl_lock_profile_wr();
 #ifndef _OS_WINDOWS_
     sigset_t sset;
     sigset_t oset;
@@ -176,7 +150,7 @@ static void jl_profile_atomic(T f)
 #ifndef _OS_WINDOWS_
     pthread_sigmask(SIG_SETMASK, &oset, NULL);
 #endif
-    uv_rwlock_wrunlock(&getJITDebugRegistry().debuginfo_asyncsafe);
+    jl_unlock_profile_wr();
 }
 
 
@@ -482,10 +456,10 @@ static int lookup_pointer(
 
     // DWARFContext/DWARFUnit update some internal tables during these queries, so
     // a lock is needed.
-    assert(0 == getJITDebugRegistry().debuginfo_asyncsafe_held);
-    uv_rwlock_wrlock(&getJITDebugRegistry().debuginfo_asyncsafe);
+    assert(0 == jl_lock_profile_rd_held());
+    jl_lock_profile_wr();
     auto inlineInfo = context->getInliningInfoForAddress(makeAddress(Section, pointer + slide), infoSpec);
-    uv_rwlock_wrunlock(&getJITDebugRegistry().debuginfo_asyncsafe);
+    jl_unlock_profile_wr();
 
     int fromC = (*frames)[0].fromC;
     int n_frames = inlineInfo.getNumberOfFrames();
@@ -508,9 +482,9 @@ static int lookup_pointer(
             info = inlineInfo.getFrame(i);
         }
         else {
-            uv_rwlock_wrlock(&getJITDebugRegistry().debuginfo_asyncsafe);
+            jl_lock_profile_wr();
             info = context->getLineInfoForAddress(makeAddress(Section, pointer + slide), infoSpec);
-            uv_rwlock_wrunlock(&getJITDebugRegistry().debuginfo_asyncsafe);
+            jl_unlock_profile_wr();
         }
 
         jl_frame_t *frame = &(*frames)[i];
@@ -1197,8 +1171,9 @@ int jl_DI_for_fptr(uint64_t fptr, uint64_t *symsize, int64_t *slide,
         object::SectionRef *Section, llvm::DIContext **context) JL_NOTSAFEPOINT
 {
     int found = 0;
-    assert(0 == getJITDebugRegistry().debuginfo_asyncsafe_held);
-    uv_rwlock_wrlock(&getJITDebugRegistry().debuginfo_asyncsafe);
+    assert(0 == jl_lock_profile_rd_held());
+    jl_lock_profile_wr();
+
     if (symsize)
         *symsize = 0;
 
@@ -1214,7 +1189,7 @@ int jl_DI_for_fptr(uint64_t fptr, uint64_t *symsize, int64_t *slide,
         }
         found = 1;
     }
-    uv_rwlock_wrunlock(&getJITDebugRegistry().debuginfo_asyncsafe);
+    jl_unlock_profile_wr();
     return found;
 }
 
@@ -1613,13 +1588,13 @@ extern "C" JL_DLLEXPORT
 uint64_t jl_getUnwindInfo_impl(uint64_t dwAddr)
 {
     // Might be called from unmanaged thread
-    jl_lock_profile_impl();
+    jl_lock_profile();
     auto &objmap = getJITDebugRegistry().getObjectMap();
     auto it = objmap.lower_bound(dwAddr);
     uint64_t ipstart = 0; // ip of the start of the section (if found)
     if (it != objmap.end() && dwAddr < it->first + it->second.SectionSize) {
         ipstart = (uint64_t)(uintptr_t)(*it).first;
     }
-    jl_unlock_profile_impl();
+    jl_unlock_profile();
     return ipstart;
 }

src/init.c

@@ -706,6 +706,7 @@ JL_DLLEXPORT void julia_init(JL_IMAGE_SEARCH rel)
     }
 
     jl_init_rand();
+    jl_init_profile_lock();
     jl_init_runtime_ccall();
     jl_init_tasks();
     jl_init_threading();

src/jl_exported_data.inc

@@ -124,10 +124,12 @@
     XX(jl_vecelement_typename) \
     XX(jl_voidpointer_type) \
     XX(jl_void_type) \
-    XX(jl_weakref_type)
+    XX(jl_weakref_type) \
 
 // Data symbols that are defined inside the public libjulia
 #define JL_EXPORTED_DATA_SYMBOLS(XX) \
     XX(jl_n_threadpools, int) \
     XX(jl_n_threads, int) \
-    XX(jl_options, jl_options_t)
+    XX(jl_options, jl_options_t) \
+
+// end of file

src/jl_exported_funcs.inc

@@ -515,10 +515,10 @@
     XX(jl_vexceptionf) \
     XX(jl_vprintf) \
     XX(jl_wakeup_thread) \
-    XX(jl_yield)
+    XX(jl_yield) \
 
 #define JL_RUNTIME_EXPORTED_FUNCS_WIN(XX) \
-    XX(jl_setjmp)
+    XX(jl_setjmp) \
 
 // use YY instead of XX to avoid jl -> ijl renaming in libjulia-codegen
 #define JL_CODEGEN_EXPORTED_FUNCS(YY) \
@@ -542,8 +542,6 @@
     YY(jl_compile_extern_c) \
     YY(jl_teardown_codegen) \
     YY(jl_jit_total_bytes) \
-    YY(jl_lock_profile) \
-    YY(jl_unlock_profile) \
     YY(jl_create_native) \
     YY(jl_dump_compiles) \
     YY(jl_dump_emitted_mi_name) \
@@ -568,4 +566,6 @@
     YY(LLVMExtraAddRemoveNIPass) \
     YY(LLVMExtraAddGCInvariantVerifierPass) \
     YY(LLVMExtraAddDemoteFloat16Pass) \
-    YY(LLVMExtraAddCPUFeaturesPass)
+    YY(LLVMExtraAddCPUFeaturesPass) \
+
+// end of file

src/julia_internal.h

@@ -135,6 +135,13 @@ JL_DLLEXPORT void jl_set_peek_cond(uintptr_t);
 JL_DLLEXPORT double jl_get_profile_peek_duration(void);
 JL_DLLEXPORT void jl_set_profile_peek_duration(double);
 
+JL_DLLEXPORT void jl_init_profile_lock(void);
+JL_DLLEXPORT uintptr_t jl_lock_profile_rd_held(void) JL_NOTSAFEPOINT;
+JL_DLLEXPORT void jl_lock_profile(void) JL_NOTSAFEPOINT;
+JL_DLLEXPORT void jl_unlock_profile(void) JL_NOTSAFEPOINT;
+JL_DLLEXPORT void jl_lock_profile_wr(void) JL_NOTSAFEPOINT;
+JL_DLLEXPORT void jl_unlock_profile_wr(void) JL_NOTSAFEPOINT;
+
 // number of cycles since power-on
 static inline uint64_t cycleclock(void) JL_NOTSAFEPOINT
 {
@@ -832,6 +839,10 @@ typedef jl_gcframe_t ***(*jl_pgcstack_key_t)(void) JL_NOTSAFEPOINT;
 #endif
 JL_DLLEXPORT void jl_pgcstack_getkey(jl_get_pgcstack_func **f, jl_pgcstack_key_t *k);
 
+#if !defined(_OS_WINDOWS_) && !defined(__APPLE__) && !defined(JL_DISABLE_LIBUNWIND)
+extern pthread_mutex_t in_signal_lock;
+#endif
+
 #if !defined(__clang_gcanalyzer__) && !defined(_OS_DARWIN_)
 static inline void jl_set_gc_and_wait(void)
 {

src/julia_threads.h

@@ -201,6 +201,7 @@ typedef struct {
 } jl_gc_mark_cache_t;
 
 struct _jl_bt_element_t;
+
 // This includes all the thread local states we care about for a thread.
 // Changes to TLS field types must be reflected in codegen.
 #define JL_MAX_BT_SIZE 80000