Skip to content

Commit 4a6ada6

Browse files
authored
Remove usages of weak symbols (#57523)
Inspired by a bug I hit on MinGW recently with weak symbol resolution. I'm not sure why we started using these in 70000ac, 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.
1 parent 406e704 commit 4a6ada6

File tree

7 files changed

+49
-49
lines changed

7 files changed

+49
-49
lines changed

src/Makefile

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ SRCS := \
5959
jltypes gf typemap smallintset ast builtins module interpreter symbol \
6060
dlload sys init task array genericmemory staticdata toplevel jl_uv datatype \
6161
simplevector runtime_intrinsics precompile jloptions mtarraylist \
62-
threading scheduler stackwalk \
62+
threading scheduler stackwalk null_sysimage \
6363
method jlapi signal-handling safepoint timing subtype rtutils \
6464
crc32c APInt-C processor ircode opaque_closure codegen-stubs coverage runtime_ccall engine \
6565
$(GC_SRCS)
@@ -77,8 +77,7 @@ else
7777
GC_CODEGEN_SRCS += llvm-late-gc-lowering-stock
7878
endif
7979
CODEGEN_SRCS := codegen jitlayers aotcompile debuginfo disasm llvm-simdloop \
80-
llvm-pass-helpers llvm-ptls \
81-
llvm-propagate-addrspaces \
80+
llvm-pass-helpers llvm-ptls llvm-propagate-addrspaces null_sysimage \
8281
llvm-multiversioning llvm-alloc-opt llvm-alloc-helpers cgmemmgr llvm-remove-addrspaces \
8382
llvm-remove-ni llvm-julia-licm llvm-demote-float16 llvm-cpufeatures pipeline llvm_api \
8483
$(GC_CODEGEN_SRCS)
@@ -187,7 +186,7 @@ endif
187186
CLANG_LDFLAGS := $(LLVM_LDFLAGS)
188187
ifeq ($(OS), Darwin)
189188
CLANG_LDFLAGS += -Wl,-undefined,dynamic_lookup
190-
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
189+
OSLIBS += -Wl,-U,__dyld_atfork_parent -Wl,-U,__dyld_atfork_prepare -Wl,-U,__dyld_dlopen_atfork_parent -Wl,-U,__dyld_dlopen_atfork_prepare
191190
LIBJULIA_PATH_REL := @rpath/libjulia
192191
else
193192
LIBJULIA_PATH_REL := libjulia

src/julia_internal.h

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1912,17 +1912,6 @@ jl_sym_t *_jl_symbol(const char *str, size_t len) JL_NOTSAFEPOINT;
19121912
#define JL_GC_ASSERT_LIVE(x) (void)(x)
19131913
#endif
19141914

1915-
#ifdef _OS_WINDOWS_
1916-
// On Windows, weak symbols do not default to 0 due to a GCC bug
1917-
// (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90826), use symbol
1918-
// aliases with a known value instead.
1919-
#define JL_WEAK_SYMBOL_OR_ALIAS_DEFAULT(sym) __attribute__((weak,alias(#sym)))
1920-
#define JL_WEAK_SYMBOL_DEFAULT(sym) &sym
1921-
#else
1922-
#define JL_WEAK_SYMBOL_OR_ALIAS_DEFAULT(sym) __attribute__((weak))
1923-
#define JL_WEAK_SYMBOL_DEFAULT(sym) NULL
1924-
#endif
1925-
19261915
//JL_DLLEXPORT float julia__gnu_h2f_ieee(half param) JL_NOTSAFEPOINT;
19271916
//JL_DLLEXPORT half julia__gnu_f2h_ieee(float param) JL_NOTSAFEPOINT;
19281917
//JL_DLLEXPORT half julia__truncdfhf2(double param) JL_NOTSAFEPOINT;

src/null_sysimage.c

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// This file is a part of Julia. License is MIT: https://julialang.org/license
2+
3+
#include <stddef.h>
4+
#include "processor.h"
5+
6+
/**
7+
* These symbols support statically linking the sysimage with libjulia-internal.
8+
*
9+
* Here we provide dummy definitions that are used when these are not linked
10+
* together (the default build configuration). The 0 value of jl_system_image_size
11+
* is used as a sentinel to indicate that the sysimage should be loaded externally.
12+
**/
13+
char jl_system_image_data = 0;
14+
size_t jl_system_image_size = 0;
15+
jl_image_pointers_t jl_image_pointers = { 0 };

src/processor.cpp

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -619,11 +619,6 @@ static inline llvm::SmallVector<TargetData<n>, 0> &get_cmdline_targets(F &&featu
619619
return targets;
620620
}
621621

622-
extern "C" {
623-
void *image_pointers_unavailable;
624-
extern void * JL_WEAK_SYMBOL_OR_ALIAS_DEFAULT(image_pointers_unavailable) jl_image_pointers;
625-
}
626-
627622
// Load sysimg, use the `callback` for dispatch and perform all relocations
628623
// for the selected target.
629624
template<typename F>
@@ -633,10 +628,10 @@ static inline jl_image_t parse_sysimg(void *hdl, F &&callback)
633628
jl_image_t res{};
634629

635630
const jl_image_pointers_t *pointers;
636-
if (hdl == jl_exe_handle && &jl_image_pointers != JL_WEAK_SYMBOL_DEFAULT(image_pointers_unavailable))
637-
pointers = (const jl_image_pointers_t *)&jl_image_pointers;
638-
else
631+
if (jl_system_image_size == 0)
639632
jl_dlsym(hdl, "jl_image_pointers", (void**)&pointers, 1);
633+
else
634+
pointers = &jl_image_pointers; // libjulia-internal and sysimage statically linked
640635

641636
const void *ids = pointers->target_data;
642637
jl_value_t* rejection_reason = nullptr;

src/processor.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,18 @@ JL_DLLEXPORT int32_t jl_set_zero_subnormals(int8_t isZero);
224224
JL_DLLEXPORT int32_t jl_get_zero_subnormals(void);
225225
JL_DLLEXPORT int32_t jl_set_default_nans(int8_t isDefault);
226226
JL_DLLEXPORT int32_t jl_get_default_nans(void);
227+
228+
/**
229+
* System image contents.
230+
*
231+
* These symbols are typically dummy values, unless statically linking
232+
* libjulia-* and the sysimage together (see null_sysimage.c), in which
233+
* case they allow accessing the local copy of the sysimage.
234+
**/
235+
extern char jl_system_image_data;
236+
extern size_t jl_system_image_size;
237+
extern jl_image_pointers_t jl_image_pointers;
238+
227239
#ifdef __cplusplus
228240
}
229241

src/staticdata.c

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -645,24 +645,25 @@ JL_DLLEXPORT int jl_running_on_valgrind(void)
645645
return RUNNING_ON_VALGRIND;
646646
}
647647

648-
void *system_image_data_unavailable;
649-
extern void * JL_WEAK_SYMBOL_OR_ALIAS_DEFAULT(system_image_data_unavailable) jl_system_image_data;
650-
extern void * JL_WEAK_SYMBOL_OR_ALIAS_DEFAULT(system_image_data_unavailable) jl_system_image_size;
651648
static void jl_load_sysimg_so(void)
652649
{
653-
const char *sysimg_data;
654650
assert(sysimage.fptrs.ptrs); // jl_init_processor_sysimg should already be run
655-
if (jl_sysimg_handle == jl_exe_handle &&
656-
&jl_system_image_data != JL_WEAK_SYMBOL_DEFAULT(system_image_data_unavailable))
657-
sysimg_data = (const char*)&jl_system_image_data;
658-
else
659-
jl_dlsym(jl_sysimg_handle, "jl_system_image_data", (void **)&sysimg_data, 1);
651+
660652
size_t *plen;
661-
if (jl_sysimg_handle == jl_exe_handle &&
662-
&jl_system_image_size != JL_WEAK_SYMBOL_DEFAULT(system_image_data_unavailable))
663-
plen = (size_t *)&jl_system_image_size;
664-
else
653+
const char *sysimg_data;
654+
655+
if (jl_system_image_size == 0) {
656+
// in the usual case, the sysimage was not statically linked to libjulia-internal
657+
// look up the external sysimage symbols via the dynamic linker
665658
jl_dlsym(jl_sysimg_handle, "jl_system_image_size", (void **)&plen, 1);
659+
jl_dlsym(jl_sysimg_handle, "jl_system_image_data", (void **)&sysimg_data, 1);
660+
} else {
661+
// the sysimage was statically linked directly against libjulia-internal
662+
// use the internal symbols
663+
plen = &jl_system_image_size;
664+
sysimg_data = &jl_system_image_data;
665+
}
666+
666667
jl_gc_notify_image_load(sysimg_data, *plen);
667668
jl_restore_system_image_data(sysimg_data, *plen);
668669
}

src/threading.c

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -228,10 +228,6 @@ void jl_set_pgcstack(jl_gcframe_t **pgcstack) JL_NOTSAFEPOINT
228228
{
229229
*jl_pgcstack_key() = pgcstack;
230230
}
231-
# if JL_USE_IFUNC
232-
JL_DLLEXPORT __attribute__((weak))
233-
void jl_register_pgcstack_getter(void);
234-
# endif
235231
static jl_gcframe_t **jl_get_pgcstack_init(void);
236232
static jl_get_pgcstack_func *jl_get_pgcstack_cb = jl_get_pgcstack_init;
237233
static jl_gcframe_t **jl_get_pgcstack_init(void)
@@ -244,15 +240,8 @@ static jl_gcframe_t **jl_get_pgcstack_init(void)
244240
// This is clearly not thread-safe but should be fine since we
245241
// make sure the tls states callback is finalized before adding
246242
// multiple threads
247-
# if JL_USE_IFUNC
248-
if (jl_register_pgcstack_getter)
249-
jl_register_pgcstack_getter();
250-
else
251-
# endif
252-
{
253-
jl_get_pgcstack_cb = jl_get_pgcstack_fallback;
254-
jl_pgcstack_key = &jl_pgcstack_addr_fallback;
255-
}
243+
jl_get_pgcstack_cb = jl_get_pgcstack_fallback;
244+
jl_pgcstack_key = &jl_pgcstack_addr_fallback;
256245
return jl_get_pgcstack_cb();
257246
}
258247

0 commit comments

Comments
 (0)