From de03507853b8fab2fb08a3f540acd5849b61684c Mon Sep 17 00:00:00 2001 From: Cody Tapscott <84105208+topolarity@users.noreply.github.com> Date: Wed, 26 Feb 2025 19:15:21 -0500 Subject: [PATCH] Remove usages of weak symbols (#57522) Inspired by a bug I hit on MinGW recently with weak symbol resolution. I'm not sure why we started using these in 70000ac7c3d5d5f21e42555cdf99e699a246f8ec, since I believe we should be able to lookup these symbols just fine in our `jl_exe_handle` as long as it was linked / compiled with `-rdynamic` Migrating away from weak symbols seems a good idea, given their uneven implementation across platforms. (cherry picked from commit 4a6ada6eb36eaeb9bde9b94c01bcf6f0e3ce7ae6) --- src/Makefile | 8 +++++--- src/julia_internal.h | 18 ------------------ src/null_sysimage.c | 15 +++++++++++++++ src/processor.cpp | 11 +++-------- src/processor.h | 12 ++++++++++++ src/staticdata.c | 27 ++++++++++++++------------- src/threading.c | 15 ++------------- 7 files changed, 51 insertions(+), 55 deletions(-) create mode 100644 src/null_sysimage.c diff --git a/src/Makefile b/src/Makefile index bf9001e5fba93..c25649d2cab7c 100644 --- a/src/Makefile +++ b/src/Makefile @@ -46,7 +46,8 @@ SRCS := \ simplevector runtime_intrinsics precompile jloptions mtarraylist \ threading scheduler stackwalk gc gc-debug gc-pages gc-stacks gc-alloc-profiler gc-page-profiler method \ jlapi signal-handling safepoint timing subtype rtutils gc-heap-snapshot \ - crc32c APInt-C processor ircode opaque_closure codegen-stubs coverage runtime_ccall + crc32c APInt-C processor ircode opaque_closure codegen-stubs coverage runtime_ccall \ + null_sysimage RT_LLVMLINK := CG_LLVMLINK := @@ -56,7 +57,8 @@ CODEGEN_SRCS := codegen jitlayers aotcompile debuginfo disasm llvm-simdloop llvm llvm-final-gc-lowering llvm-pass-helpers llvm-late-gc-lowering llvm-ptls \ llvm-lower-handlers llvm-gc-invariant-verifier llvm-propagate-addrspaces \ llvm-multiversioning llvm-alloc-opt llvm-alloc-helpers cgmemmgr llvm-remove-addrspaces \ - llvm-remove-ni llvm-julia-licm llvm-demote-float16 llvm-cpufeatures pipeline llvm_api + llvm-remove-ni llvm-julia-licm llvm-demote-float16 llvm-cpufeatures pipeline llvm_api \ + null_sysimage FLAGS += -I$(shell $(LLVM_CONFIG_HOST) --includedir) CG_LLVM_LIBS := all ifeq ($(USE_POLLY),1) @@ -160,7 +162,7 @@ endif CLANG_LDFLAGS := $(LLVM_LDFLAGS) ifeq ($(OS), Darwin) CLANG_LDFLAGS += -Wl,-undefined,dynamic_lookup -OSLIBS += -Wl,-U,__dyld_atfork_parent -Wl,-U,__dyld_atfork_prepare -Wl,-U,__dyld_dlopen_atfork_parent -Wl,-U,__dyld_dlopen_atfork_prepare -Wl,-U,_jl_image_pointers -Wl,-U,_jl_system_image_data -Wl,-U,_jl_system_image_size +OSLIBS += -Wl,-U,__dyld_atfork_parent -Wl,-U,__dyld_atfork_prepare -Wl,-U,__dyld_dlopen_atfork_parent -Wl,-U,__dyld_dlopen_atfork_prepare LIBJULIA_PATH_REL := @rpath/libjulia else LIBJULIA_PATH_REL := libjulia diff --git a/src/julia_internal.h b/src/julia_internal.h index 72d9dfdad5c7e..dff769e7e0f66 100644 --- a/src/julia_internal.h +++ b/src/julia_internal.h @@ -1710,24 +1710,6 @@ jl_sym_t *_jl_symbol(const char *str, size_t len) JL_NOTSAFEPOINT; #define JL_GC_ASSERT_LIVE(x) (void)(x) #endif -#ifdef _OS_WINDOWS_ -// On Windows, weak symbols do not default to 0 due to a GCC bug -// (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90826), use symbol -// aliases with a known value instead. -#define JL_WEAK_SYMBOL_OR_ALIAS_DEFAULT(sym) __attribute__((weak,alias(#sym))) -#define JL_WEAK_SYMBOL_DEFAULT(sym) &sym -#else -#define JL_WEAK_SYMBOL_OR_ALIAS_DEFAULT(sym) __attribute__((weak)) -#define JL_WEAK_SYMBOL_DEFAULT(sym) NULL -#endif - -//JL_DLLEXPORT float julia__gnu_h2f_ieee(half param) JL_NOTSAFEPOINT; -//JL_DLLEXPORT half julia__gnu_f2h_ieee(float param) JL_NOTSAFEPOINT; -//JL_DLLEXPORT half julia__truncdfhf2(double param) JL_NOTSAFEPOINT; -//JL_DLLEXPORT float julia__truncsfbf2(float param) JL_NOTSAFEPOINT; -//JL_DLLEXPORT float julia__truncdfbf2(double param) JL_NOTSAFEPOINT; -//JL_DLLEXPORT double julia__extendhfdf2(half n) JL_NOTSAFEPOINT; - JL_DLLEXPORT uint32_t jl_crc32c(uint32_t crc, const char *buf, size_t len); // -- exports from codegen -- // diff --git a/src/null_sysimage.c b/src/null_sysimage.c new file mode 100644 index 0000000000000..386842f0c4e77 --- /dev/null +++ b/src/null_sysimage.c @@ -0,0 +1,15 @@ +// This file is a part of Julia. License is MIT: https://julialang.org/license + +#include +#include "processor.h" + +/** + * These symbols support statically linking the sysimage with libjulia-internal. + * + * Here we provide dummy definitions that are used when these are not linked + * together (the default build configuration). The 0 value of jl_system_image_size + * is used as a sentinel to indicate that the sysimage should be loaded externally. + **/ +char jl_system_image_data = 0; +size_t jl_system_image_size = 0; +jl_image_pointers_t jl_image_pointers = { 0 }; diff --git a/src/processor.cpp b/src/processor.cpp index 730e470f4153d..ed50c609895cc 100644 --- a/src/processor.cpp +++ b/src/processor.cpp @@ -619,11 +619,6 @@ static inline llvm::SmallVector, 0> &get_cmdline_targets(F &&featu return targets; } -extern "C" { -void *image_pointers_unavailable; -extern void * JL_WEAK_SYMBOL_OR_ALIAS_DEFAULT(image_pointers_unavailable) jl_image_pointers; -} - // Load sysimg, use the `callback` for dispatch and perform all relocations // for the selected target. template @@ -633,10 +628,10 @@ static inline jl_image_t parse_sysimg(void *hdl, F &&callback) jl_image_t res{}; const jl_image_pointers_t *pointers; - if (hdl == jl_exe_handle && &jl_image_pointers != JL_WEAK_SYMBOL_DEFAULT(image_pointers_unavailable)) - pointers = (const jl_image_pointers_t *)&jl_image_pointers; - else + if (jl_system_image_size == 0) jl_dlsym(hdl, "jl_image_pointers", (void**)&pointers, 1); + else + pointers = &jl_image_pointers; // libjulia-internal and sysimage statically linked const void *ids = pointers->target_data; jl_value_t* rejection_reason = nullptr; diff --git a/src/processor.h b/src/processor.h index 82a1121aaf7c4..214f51845b12d 100644 --- a/src/processor.h +++ b/src/processor.h @@ -224,6 +224,18 @@ JL_DLLEXPORT int32_t jl_set_zero_subnormals(int8_t isZero); JL_DLLEXPORT int32_t jl_get_zero_subnormals(void); JL_DLLEXPORT int32_t jl_set_default_nans(int8_t isDefault); JL_DLLEXPORT int32_t jl_get_default_nans(void); + +/** + * System image contents. + * + * These symbols are typically dummy values, unless statically linking + * libjulia-* and the sysimage together (see null_sysimage.c), in which + * case they allow accessing the local copy of the sysimage. + **/ +extern char jl_system_image_data; +extern size_t jl_system_image_size; +extern jl_image_pointers_t jl_image_pointers; + #ifdef __cplusplus } diff --git a/src/staticdata.c b/src/staticdata.c index cac3b78545cca..793a2ccd19574 100644 --- a/src/staticdata.c +++ b/src/staticdata.c @@ -620,24 +620,25 @@ JL_DLLEXPORT int jl_running_on_valgrind(void) return RUNNING_ON_VALGRIND; } -void *system_image_data_unavailable; -extern void * JL_WEAK_SYMBOL_OR_ALIAS_DEFAULT(system_image_data_unavailable) jl_system_image_data; -extern void * JL_WEAK_SYMBOL_OR_ALIAS_DEFAULT(system_image_data_unavailable) jl_system_image_size; static void jl_load_sysimg_so(void) { - const char *sysimg_data; assert(sysimage.fptrs.ptrs); // jl_init_processor_sysimg should already be run - if (jl_sysimg_handle == jl_exe_handle && - &jl_system_image_data != JL_WEAK_SYMBOL_DEFAULT(system_image_data_unavailable)) - sysimg_data = (const char*)&jl_system_image_data; - else - jl_dlsym(jl_sysimg_handle, "jl_system_image_data", (void **)&sysimg_data, 1); + size_t *plen; - if (jl_sysimg_handle == jl_exe_handle && - &jl_system_image_size != JL_WEAK_SYMBOL_DEFAULT(system_image_data_unavailable)) - plen = (size_t *)&jl_system_image_size; - else + const char *sysimg_data; + + if (jl_system_image_size == 0) { + // in the usual case, the sysimage was not statically linked to libjulia-internal + // look up the external sysimage symbols via the dynamic linker jl_dlsym(jl_sysimg_handle, "jl_system_image_size", (void **)&plen, 1); + jl_dlsym(jl_sysimg_handle, "jl_system_image_data", (void **)&sysimg_data, 1); + } else { + // the sysimage was statically linked directly against libjulia-internal + // use the internal symbols + plen = &jl_system_image_size; + sysimg_data = &jl_system_image_data; + } + jl_restore_system_image_data(sysimg_data, *plen); } diff --git a/src/threading.c b/src/threading.c index 62af7e60bf0f1..a17d3b371acf5 100644 --- a/src/threading.c +++ b/src/threading.c @@ -250,10 +250,6 @@ void jl_set_pgcstack(jl_gcframe_t **pgcstack) JL_NOTSAFEPOINT { *jl_pgcstack_key() = pgcstack; } -# if JL_USE_IFUNC -JL_DLLEXPORT __attribute__((weak)) -void jl_register_pgcstack_getter(void); -# endif static jl_gcframe_t **jl_get_pgcstack_init(void); static jl_get_pgcstack_func *jl_get_pgcstack_cb = jl_get_pgcstack_init; static jl_gcframe_t **jl_get_pgcstack_init(void) @@ -266,15 +262,8 @@ static jl_gcframe_t **jl_get_pgcstack_init(void) // This is clearly not thread-safe but should be fine since we // make sure the tls states callback is finalized before adding // multiple threads -# if JL_USE_IFUNC - if (jl_register_pgcstack_getter) - jl_register_pgcstack_getter(); - else -# endif - { - jl_get_pgcstack_cb = jl_get_pgcstack_fallback; - jl_pgcstack_key = &jl_pgcstack_addr_fallback; - } + jl_get_pgcstack_cb = jl_get_pgcstack_fallback; + jl_pgcstack_key = &jl_pgcstack_addr_fallback; return jl_get_pgcstack_cb(); }