mailing list of musl libc
 help / color / mirror / code / Atom feed
* [PATCH 0/5] reorganize the use of weak symbols
@ 2013-02-15 23:22 Jens Gustedt
  2013-02-15 23:23 ` [PATCH 1/5] Clearly identify the readonly replacement symbols that serve as 'dummies' that could (or could not) be provided by other compilation units Jens Gustedt
                   ` (5 more replies)
  0 siblings, 6 replies; 8+ messages in thread
From: Jens Gustedt @ 2013-02-15 23:22 UTC (permalink / raw)
  To: musl

Weak symbols are used for several uses that are in fact quite distinct.

 - The first use case is the one that weak symbols were originally
   invented for: a symbol that provides some dummy data or function
   that replaces some other functionality if that functionality is not
   linked into the executable or library. There are only a few clear
   cut use cases like that for functions in musl. Mark them clearly as
   such by using the new macro _Weak.
 - A subcase of the previous one, but where the dummy action that is
   performed is a NOP. There are three different function interfaces
   that are used all over musl that fall into this category; with void
   argument list, with a void* or an int.
 - Functions that implement several interfaces at once. The macro
   weak_alias provides the additional "names" for the function.
 - Data that resides in a different compilation unit and for which a
   value that is non-zero indicates the presence of that different
   compilation unit in the current executable. We unify the coding of
   such weak symbols through one only "meta" weak reandonly symbol
   __readonly_dummy that is guaranteed to be wide enough and
   initialized by all bits 0.
 - A special subcase of the previous one, where the weak symbol also
   serves as a guard which would provoke a fault when the symbol
   would be written to. There is one such special use case in musl.

Jens Gustedt (5):
  Clearly identify the readonly replacement symbols that serve as
    'dummies'     that could (or could not) be provided by other
    compilation units.
  Clarify the implementation of the dummy alias used for pthread_self.
  identify the weak function symbols that provide a real default action
  add three macros for empty dummy functions that do nothing
  Use the weak functions that do nothing as aliases for the default
    actions

 src/aio/aio_readwrite.c     |    7 ++-----
 src/env/__init_security.c   |    6 ++----
 src/exit/exit.c             |   11 ++++-------
 src/exit/quick_exit.c       |    4 ++--
 src/internal/libc.h         |   42 ++++++++++++++++++++++++++++++++++++++++++
 src/mman/mmap.c             |    8 ++++----
 src/mman/munmap.c           |    8 ++++----
 src/process/fork.c          |    7 ++-----
 src/process/posix_spawn.c   |    8 +++-----
 src/process/system.c        |    8 +++-----
 src/signal/sigaction.c      |    4 ++--
 src/stdio/__stdio_exit.c    |    8 ++++----
 src/stdio/__toread.c        |    4 ++--
 src/stdio/fflush.c          |    4 ++--
 src/stdio/popen.c           |    8 +++-----
 src/thread/cancel_dummy.c   |   12 ++++--------
 src/thread/cancellation.c   |    9 ++++++---
 src/thread/pthread_create.c |   22 +++++++++-------------
 src/thread/pthread_join.c   |    6 ++----
 src/thread/pthread_self.c   |   11 +++++++----
 src/time/timer_create.c     |    4 ++--
 21 files changed, 111 insertions(+), 90 deletions(-)

-- 
1.7.9.5



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/5] Clearly identify the readonly replacement symbols that serve as 'dummies' that could (or could not) be provided by other compilation units.
  2013-02-15 23:22 [PATCH 0/5] reorganize the use of weak symbols Jens Gustedt
@ 2013-02-15 23:23 ` Jens Gustedt
  2013-02-15 23:23 ` [PATCH 2/5] Clarify the implementation of the dummy alias used for pthread_self Jens Gustedt
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Jens Gustedt @ 2013-02-15 23:23 UTC (permalink / raw)
  To: musl


9	0	src/internal/libc.h
4	4	src/stdio/__stdio_exit.c
2	2	src/stdio/__toread.c
2	2	src/stdio/fflush.c
3	5	src/stdio/popen.c
5	7	src/thread/pthread_create.c

diff --git a/src/internal/libc.h b/src/internal/libc.h
index c9416f0..a2f36fb 100644
--- a/src/internal/libc.h
+++ b/src/internal/libc.h
@@ -58,6 +58,15 @@ int __setxid(int, int, int, int);
 
 extern char **__environ;
 
+
+/* Provide a dummy location for all readonly symbols that are
+   weak. This is a fallback that should always have a value of all
+   zero and suitable aligned to be able to serve as an address for any
+   type. */
+#define WEAK_PROVIDE_DUMMY __attribute__((__weak__, __aligned__(32))) struct { unsigned char const _arr[32]; }  __readonly_dummy
+
+#define _Readonly_alias extern __attribute__((__weak__,__alias__("__readonly_dummy")))
+
 #undef weak_alias
 #define weak_alias(old, new) \
 	extern __typeof(old) new __attribute__((weak, alias(#old)))
diff --git a/src/stdio/__stdio_exit.c b/src/stdio/__stdio_exit.c
index 0fb3323..2a38c8d 100644
--- a/src/stdio/__stdio_exit.c
+++ b/src/stdio/__stdio_exit.c
@@ -1,9 +1,9 @@
 #include "stdio_impl.h"
 
-static FILE *const dummy_file = 0;
-weak_alias(dummy_file, __stdin_used);
-weak_alias(dummy_file, __stdout_used);
-weak_alias(dummy_file, __stderr_used);
+WEAK_PROVIDE_DUMMY;
+_Readonly_alias FILE *const __stdin_used;
+_Readonly_alias FILE *const __stdout_used;
+_Readonly_alias FILE *const __stderr_used;
 
 static void close_file(FILE *f)
 {
diff --git a/src/stdio/__toread.c b/src/stdio/__toread.c
index 2e804f6..7c5145c 100644
--- a/src/stdio/__toread.c
+++ b/src/stdio/__toread.c
@@ -13,8 +13,8 @@ int __toread(FILE *f)
 	return 0;
 }
 
-static const int dummy = 0;
-weak_alias(dummy, __towrite_used);
+WEAK_PROVIDE_DUMMY;
+_Readonly_alias int const __towrite_used;
 
 void __stdio_exit(void);
 
diff --git a/src/stdio/fflush.c b/src/stdio/fflush.c
index af70950..22debf0 100644
--- a/src/stdio/fflush.c
+++ b/src/stdio/fflush.c
@@ -19,8 +19,8 @@ static int __fflush_unlocked(FILE *f)
 }
 
 /* stdout.c will override this if linked */
-static FILE *const dummy = 0;
-weak_alias(dummy, __stdout_used);
+WEAK_PROVIDE_DUMMY;
+_Readonly_alias FILE *const __stdout_used;
 
 int fflush(FILE *f)
 {
diff --git a/src/stdio/popen.c b/src/stdio/popen.c
index e5fbc4f..a1fa149 100644
--- a/src/stdio/popen.c
+++ b/src/stdio/popen.c
@@ -6,11 +6,9 @@
 #include "pthread_impl.h"
 #include "syscall.h"
 
-static void dummy_0()
-{
-}
-weak_alias(dummy_0, __acquire_ptc);
-weak_alias(dummy_0, __release_ptc);
+WEAK_PROVIDE_VOID;
+weak_alias(__weak_dummy_void, __acquire_ptc);
+weak_alias(__weak_dummy_void, __release_ptc);
 
 pid_t __vfork(void);
 
diff --git a/src/thread/pthread_create.c b/src/thread/pthread_create.c
index d11dcfa..3f30116 100644
--- a/src/thread/pthread_create.c
+++ b/src/thread/pthread_create.c
@@ -81,13 +81,11 @@ static int start(void *p)
 #define ROUND(x) (((x)+PAGE_SIZE-1)&-PAGE_SIZE)
 
 /* pthread_key_create.c overrides this */
-static const size_t dummy = 0;
-weak_alias(dummy, __pthread_tsd_size);
-
-static FILE *const dummy_file = 0;
-weak_alias(dummy_file, __stdin_used);
-weak_alias(dummy_file, __stdout_used);
-weak_alias(dummy_file, __stderr_used);
+WEAK_PROVIDE_DUMMY;
+_Readonly_alias const size_t __pthread_tsd_size;
+_Readonly_alias FILE *const __stdin_used;
+_Readonly_alias FILE *const __stdout_used;
+_Readonly_alias FILE *const __stderr_used;
 
 static void init_file_lock(FILE *f)
 {
-- 
1.7.9.5



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/5] Clarify the implementation of the dummy alias used for pthread_self.
  2013-02-15 23:22 [PATCH 0/5] reorganize the use of weak symbols Jens Gustedt
  2013-02-15 23:23 ` [PATCH 1/5] Clearly identify the readonly replacement symbols that serve as 'dummies' that could (or could not) be provided by other compilation units Jens Gustedt
@ 2013-02-15 23:23 ` Jens Gustedt
  2013-02-15 23:24 ` [PATCH 3/5] identify the weak function symbols that provide a real default action Jens Gustedt
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Jens Gustedt @ 2013-02-15 23:23 UTC (permalink / raw)
  To: musl

In fact this points to a readonly location that is meant to crash the
executable if pthread_keys are used without having properly linked to
pthread_key_create. This can be of the correct type now.

7	4	src/thread/pthread_self.c

diff --git a/src/thread/pthread_self.c b/src/thread/pthread_self.c
index 23dbaa5..87158a7 100644
--- a/src/thread/pthread_self.c
+++ b/src/thread/pthread_self.c
@@ -2,9 +2,12 @@
 
 static struct pthread *main_thread = &(struct pthread){0};
 
-/* pthread_key_create.c overrides this */
-static const void *dummy[1] = { 0 };
-weak_alias(dummy, __pthread_tsd_main);
+/* pthread_key_create.c overrides this with a modifiable array
+   whenever the executable is linked with pthread_key_create. If it is
+   not, and an attempt is made to write to the pthread_key system, the
+   program will crash, since this is in a read-only segment. */
+WEAK_PROVIDE_DUMMY;
+_Readonly_alias void* __pthread_tsd_main[PTHREAD_KEYS_MAX];
 
 static int init_main_thread()
 {
@@ -12,7 +15,7 @@ static int init_main_thread()
 		SIGPT_SET, 0, __SYSCALL_SSLEN);
 	if (__set_thread_area(TP_ADJ(main_thread)) < 0) return -1;
 	main_thread->canceldisable = libc.canceldisable;
-	main_thread->tsd = (void **)__pthread_tsd_main;
+	main_thread->tsd = __pthread_tsd_main;
 	main_thread->errno_ptr = __errno_location();
 	main_thread->self = main_thread;
 	main_thread->tid = main_thread->pid =
-- 
1.7.9.5



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 3/5] identify the weak function symbols that provide a real default action
  2013-02-15 23:22 [PATCH 0/5] reorganize the use of weak symbols Jens Gustedt
  2013-02-15 23:23 ` [PATCH 1/5] Clearly identify the readonly replacement symbols that serve as 'dummies' that could (or could not) be provided by other compilation units Jens Gustedt
  2013-02-15 23:23 ` [PATCH 2/5] Clarify the implementation of the dummy alias used for pthread_self Jens Gustedt
@ 2013-02-15 23:24 ` Jens Gustedt
  2013-02-15 23:24 ` [PATCH 4/5] add three macros for empty dummy functions that do nothing Jens Gustedt
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Jens Gustedt @ 2013-02-15 23:24 UTC (permalink / raw)
  To: musl


2	0	src/internal/libc.h
2	2	src/signal/sigaction.c
2	3	src/thread/cancel_dummy.c
2	2	src/time/timer_create.c

diff --git a/src/internal/libc.h b/src/internal/libc.h
index a2f36fb..a097a66 100644
--- a/src/internal/libc.h
+++ b/src/internal/libc.h
@@ -67,6 +67,8 @@ extern char **__environ;
 
 #define _Readonly_alias extern __attribute__((__weak__,__alias__("__readonly_dummy")))
 
+#define _Weak extern __attribute__((__weak__))
+
 #undef weak_alias
 #define weak_alias(old, new) \
 	extern __typeof(old) new __attribute__((weak, alias(#old)))
diff --git a/src/signal/sigaction.c b/src/signal/sigaction.c
index 7a72a44..ff7ac81 100644
--- a/src/signal/sigaction.c
+++ b/src/signal/sigaction.c
@@ -9,8 +9,8 @@
 
 void __restore(), __restore_rt();
 
-static pthread_t dummy(void) { return 0; }
-weak_alias(dummy, __pthread_self_def);
+_Weak
+pthread_t __pthread_self_def(void) { return 0; }
 
 int __libc_sigaction(int sig, const struct sigaction *restrict sa, struct sigaction *restrict old)
 {
diff --git a/src/thread/cancel_dummy.c b/src/thread/cancel_dummy.c
index 047692c..7246970 100644
--- a/src/thread/cancel_dummy.c
+++ b/src/thread/cancel_dummy.c
@@ -1,12 +1,11 @@
 #include "pthread_impl.h"
 
-static long sccp(long nr, long u, long v, long w, long x, long y, long z)
+_Weak
+long (__syscall_cp)(long nr, long u, long v, long w, long x, long y, long z)
 {
 	return (__syscall)(nr, u, v, w, x, y, z);
 }
 
-weak_alias(sccp, __syscall_cp);
-
 static void dummy()
 {
 }
diff --git a/src/time/timer_create.c b/src/time/timer_create.c
index 60a18c7..0e00baf 100644
--- a/src/time/timer_create.c
+++ b/src/time/timer_create.c
@@ -14,10 +14,10 @@ struct start_args {
 	struct sigevent *sev;
 };
 
-static void dummy_1(pthread_t self)
+_Weak
+void __pthread_tsd_run_dtors(pthread_t self)
 {
 }
-weak_alias(dummy_1, __pthread_tsd_run_dtors);
 
 static void cleanup_fromsig(void *p)
 {
-- 
1.7.9.5



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 4/5] add three macros for empty dummy functions that do nothing
  2013-02-15 23:22 [PATCH 0/5] reorganize the use of weak symbols Jens Gustedt
                   ` (2 preceding siblings ...)
  2013-02-15 23:24 ` [PATCH 3/5] identify the weak function symbols that provide a real default action Jens Gustedt
@ 2013-02-15 23:24 ` Jens Gustedt
  2013-02-15 23:25 ` [PATCH 5/5] Use the weak functions that do nothing as aliases for the default actions Jens Gustedt
  2013-02-16  5:59 ` [PATCH 0/5] reorganize the use of weak symbols Rich Felker
  5 siblings, 0 replies; 8+ messages in thread
From: Jens Gustedt @ 2013-02-15 23:24 UTC (permalink / raw)
  To: musl

these are functions that receive arguments according to three different
interfaces, void, void* or int.

31	0	src/internal/libc.h

diff --git a/src/internal/libc.h b/src/internal/libc.h
index a097a66..1f15f4d 100644
--- a/src/internal/libc.h
+++ b/src/internal/libc.h
@@ -73,6 +73,37 @@ extern char **__environ;
 #define weak_alias(old, new) \
 	extern __typeof(old) new __attribute__((weak, alias(#old)))
 
+/* provide a weak function symbol for a function that receives no
+   arguments */
+#define WEAK_PROVIDE_VOID                       \
+_Weak                                           \
+void __weak_dummy_void(void) {                  \
+  /* empty */                                   \
+}                                               \
+/* syntax sugar */                              \
+_Weak void __weak_dummy_void(void)
+
+/* provide a weak function symbol for a function that receives a void
+   pointer argument */
+#define WEAK_PROVIDE_VOIDP                      \
+_Weak                                           \
+void __weak_dummy_voidp(void* _ign) {           \
+  /* empty */                                   \
+}                                               \
+/* syntax sugar */                              \
+_Weak void __weak_dummy_voidp(void*)
+
+/* provide a weak function symbol for a function that receives an int
+   argument */
+#define WEAK_PROVIDE_INT                        \
+_Weak                                           \
+void __weak_dummy_int(int _ign) {               \
+  /* empty */                                   \
+}                                               \
+/* syntax sugar */                              \
+_Weak void __weak_dummy_int(int _ign)
+
+
 #undef LFS64_2
 #define LFS64_2(x, y) weak_alias(x, y)
 
-- 
1.7.9.5



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 5/5] Use the weak functions that do nothing as aliases for the default actions
  2013-02-15 23:22 [PATCH 0/5] reorganize the use of weak symbols Jens Gustedt
                   ` (3 preceding siblings ...)
  2013-02-15 23:24 ` [PATCH 4/5] add three macros for empty dummy functions that do nothing Jens Gustedt
@ 2013-02-15 23:25 ` Jens Gustedt
  2013-02-16  5:59 ` [PATCH 0/5] reorganize the use of weak symbols Rich Felker
  5 siblings, 0 replies; 8+ messages in thread
From: Jens Gustedt @ 2013-02-15 23:25 UTC (permalink / raw)
  To: musl

 - this unifies the coding of such functions and clearly marks the intent
 - all such functions with a same type may be overlayed by the
   compiler. thereby only three such functions must be realized over all
   the musl C library
 - when compiled with -ffunction-sections and linked with
   -Wl,--gc-sections the gcc toolchain is in fact capable to rip
   superfluous copies from the final libc.so or from the statically
   linked executable.

2	5	src/aio/aio_readwrite.c
2	4	src/env/__init_security.c
4	7	src/exit/exit.c
2	2	src/exit/quick_exit.c
4	4	src/mman/mmap.c
4	4	src/mman/munmap.c
2	5	src/process/fork.c
3	5	src/process/posix_spawn.c
3	5	src/process/system.c
2	5	src/thread/cancel_dummy.c
6	3	src/thread/cancellation.c
4	6	src/thread/pthread_create.c
2	4	src/thread/pthread_join.c

diff --git a/src/aio/aio_readwrite.c b/src/aio/aio_readwrite.c
index e4c95aa..4862365 100644
--- a/src/aio/aio_readwrite.c
+++ b/src/aio/aio_readwrite.c
@@ -5,11 +5,8 @@
 #include <limits.h>
 #include "pthread_impl.h"
 
-static void dummy(void)
-{
-}
-
-weak_alias(dummy, __aio_wake);
+WEAK_PROVIDE_VOID;
+weak_alias(__weak_dummy_void, __aio_wake);
 
 static void notify_signal(struct sigevent *sev)
 {
diff --git a/src/env/__init_security.c b/src/env/__init_security.c
index 91b9b10..5b98623 100644
--- a/src/env/__init_security.c
+++ b/src/env/__init_security.c
@@ -6,10 +6,8 @@
 #include "libc.h"
 #include "atomic.h"
 
-static void dummy(void *ent)
-{
-}
-weak_alias(dummy, __init_ssp);
+WEAK_PROVIDE_VOIDP;
+weak_alias(__weak_dummy_voidp, __init_ssp);
 
 void __init_security(size_t *aux)
 {
diff --git a/src/exit/exit.c b/src/exit/exit.c
index e4932b5..a866e02 100644
--- a/src/exit/exit.c
+++ b/src/exit/exit.c
@@ -5,14 +5,11 @@
 #include "atomic.h"
 #include "syscall.h"
 
-static void dummy()
-{
-}
-
 /* __toread.c, __towrite.c, and atexit.c override these */
-weak_alias(dummy, __funcs_on_exit);
-weak_alias(dummy, __flush_on_exit);
-weak_alias(dummy, __seek_on_exit);
+WEAK_PROVIDE_VOID;
+weak_alias(__weak_dummy_void, __funcs_on_exit);
+weak_alias(__weak_dummy_void, __flush_on_exit);
+weak_alias(__weak_dummy_void, __seek_on_exit);
 
 _Noreturn void exit(int code)
 {
diff --git a/src/exit/quick_exit.c b/src/exit/quick_exit.c
index 1175d80..79c389f 100644
--- a/src/exit/quick_exit.c
+++ b/src/exit/quick_exit.c
@@ -3,8 +3,8 @@
 #include "atomic.h"
 #include "libc.h"
 
-static void dummy() { }
-weak_alias(dummy, __funcs_on_quick_exit);
+WEAK_PROVIDE_VOID;
+weak_alias(__weak_dummy_void, __funcs_on_quick_exit);
 
 _Noreturn void quick_exit(int code)
 {
diff --git a/src/mman/mmap.c b/src/mman/mmap.c
index e99271f..2391eb7 100644
--- a/src/mman/mmap.c
+++ b/src/mman/mmap.c
@@ -5,10 +5,10 @@
 #include "syscall.h"
 #include "libc.h"
 
-static void dummy1(int x) { }
-static void dummy0(void) { }
-weak_alias(dummy1, __vm_lock);
-weak_alias(dummy0, __vm_unlock);
+WEAK_PROVIDE_INT;
+WEAK_PROVIDE_VOID;
+weak_alias(__weak_dummy_int, __vm_lock);
+weak_alias(__weak_dummy_void, __vm_unlock);
 
 #define OFF_MASK ((-0x2000ULL << (8*sizeof(long)-1)) | 0xfff)
 
diff --git a/src/mman/munmap.c b/src/mman/munmap.c
index 91aefd4..2c18e7c 100644
--- a/src/mman/munmap.c
+++ b/src/mman/munmap.c
@@ -3,10 +3,10 @@
 #include "syscall.h"
 #include "libc.h"
 
-static void dummy1(int x) { }
-static void dummy0(void) { }
-weak_alias(dummy1, __vm_lock);
-weak_alias(dummy0, __vm_unlock);
+WEAK_PROVIDE_INT;
+WEAK_PROVIDE_VOID;
+weak_alias(__weak_dummy_int, __vm_lock);
+weak_alias(__weak_dummy_void, __vm_unlock);
 
 int __munmap(void *start, size_t len)
 {
diff --git a/src/process/fork.c b/src/process/fork.c
index fb8a430..4a83bc3 100644
--- a/src/process/fork.c
+++ b/src/process/fork.c
@@ -4,11 +4,8 @@
 #include "libc.h"
 #include "pthread_impl.h"
 
-static void dummy(int x)
-{
-}
-
-weak_alias(dummy, __fork_handler);
+WEAK_PROVIDE_INT;
+weak_alias(__weak_dummy_int,  __fork_handler);
 
 pid_t fork(void)
 {
diff --git a/src/process/posix_spawn.c b/src/process/posix_spawn.c
index dd45012..e1668b7 100644
--- a/src/process/posix_spawn.c
+++ b/src/process/posix_spawn.c
@@ -10,11 +10,9 @@
 #include "fdop.h"
 #include "libc.h"
 
-static void dummy_0()
-{
-}
-weak_alias(dummy_0, __acquire_ptc);
-weak_alias(dummy_0, __release_ptc);
+WEAK_PROVIDE_VOID;
+weak_alias(__weak_dummy_void, __acquire_ptc);
+weak_alias(__weak_dummy_void, __release_ptc);
 
 struct args {
 	int p[2];
diff --git a/src/process/system.c b/src/process/system.c
index 4232bef..6946b35 100644
--- a/src/process/system.c
+++ b/src/process/system.c
@@ -7,11 +7,9 @@
 #include "pthread_impl.h"
 #include "libc.h"
 
-static void dummy_0()
-{
-}
-weak_alias(dummy_0, __acquire_ptc);
-weak_alias(dummy_0, __release_ptc);
+WEAK_PROVIDE_VOID;
+weak_alias(__weak_dummy_void, __acquire_ptc);
+weak_alias(__weak_dummy_void, __release_ptc);
 
 extern char **__environ;
 
diff --git a/src/thread/cancel_dummy.c b/src/thread/cancel_dummy.c
index 7246970..655a222 100644
--- a/src/thread/cancel_dummy.c
+++ b/src/thread/cancel_dummy.c
@@ -6,8 +6,5 @@ long (__syscall_cp)(long nr, long u, long v, long w, long x, long y, long z)
 	return (__syscall)(nr, u, v, w, x, y, z);
 }
 
-static void dummy()
-{
-}
-
-weak_alias(dummy, __testcancel);
+WEAK_PROVIDE_VOID;
+weak_alias(__weak_dummy_void, __testcancel);
diff --git a/src/thread/cancellation.c b/src/thread/cancellation.c
index 9b21764..fa3208e 100644
--- a/src/thread/cancellation.c
+++ b/src/thread/cancellation.c
@@ -1,10 +1,13 @@
 #include "pthread_impl.h"
 
-static void dummy(struct __ptcb *cb)
+/* The following two are overwritten by pthread_create.c */
+_Weak
+void __weak_dummy_ptcb(struct __ptcb *cb)
 {
 }
-weak_alias(dummy, __do_cleanup_push);
-weak_alias(dummy, __do_cleanup_pop);
+
+weak_alias(__weak_dummy_ptcb, __do_cleanup_push);
+weak_alias(__weak_dummy_ptcb, __do_cleanup_pop);
 
 void _pthread_cleanup_push(struct __ptcb *cb, void (*f)(void *), void *x)
 {
diff --git a/src/thread/pthread_create.c b/src/thread/pthread_create.c
index 3f30116..77ca0cf 100644
--- a/src/thread/pthread_create.c
+++ b/src/thread/pthread_create.c
@@ -2,12 +2,10 @@
 #include "stdio_impl.h"
 #include <sys/mman.h>
 
-static void dummy_0()
-{
-}
-weak_alias(dummy_0, __acquire_ptc);
-weak_alias(dummy_0, __release_ptc);
-weak_alias(dummy_0, __pthread_tsd_run_dtors);
+WEAK_PROVIDE_VOID;
+weak_alias(__weak_dummy_void, __acquire_ptc);
+weak_alias(__weak_dummy_void, __release_ptc);
+weak_alias(__weak_dummy_void, __pthread_tsd_run_dtors);
 
 _Noreturn void pthread_exit(void *result)
 {
diff --git a/src/thread/pthread_join.c b/src/thread/pthread_join.c
index 719c91c..87eed62 100644
--- a/src/thread/pthread_join.c
+++ b/src/thread/pthread_join.c
@@ -1,14 +1,12 @@
 #include "pthread_impl.h"
 #include <sys/mman.h>
 
-static void dummy(void *p)
-{
-}
+WEAK_PROVIDE_VOIDP;
 
 int pthread_join(pthread_t t, void **res)
 {
 	int tmp;
-	while ((tmp = t->tid)) __timedwait(&t->tid, tmp, 0, 0, dummy, 0, 0);
+	while ((tmp = t->tid)) __timedwait(&t->tid, tmp, 0, 0, __weak_dummy_voidp, 0, 0);
 	if (res) *res = t->result;
 	if (t->map_base) munmap(t->map_base, t->map_size);
 	return 0;
-- 
1.7.9.5



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/5] reorganize the use of weak symbols
  2013-02-15 23:22 [PATCH 0/5] reorganize the use of weak symbols Jens Gustedt
                   ` (4 preceding siblings ...)
  2013-02-15 23:25 ` [PATCH 5/5] Use the weak functions that do nothing as aliases for the default actions Jens Gustedt
@ 2013-02-16  5:59 ` Rich Felker
  2013-02-16  8:16   ` Jens Gustedt
  5 siblings, 1 reply; 8+ messages in thread
From: Rich Felker @ 2013-02-16  5:59 UTC (permalink / raw)
  To: musl

On Sat, Feb 16, 2013 at 12:22:03AM +0100, Jens Gustedt wrote:
> Weak symbols are used for several uses that are in fact quite distinct.

First some general comments -- I think this patchset might be arising
as a result of a clash of style/philosophy. As an analogy, "int" can
be be used for several things that are distinct. If there's a
possibility that int could be the wrong type on some systems or
configurations, it makes sense to use a typedef and a distinct type
name. But if int were already the correct type in all cases, using a
typedef just creates a barrier to reading comprehension in the code --
the reader has to go lookup what the type is defined as, only to find
that it's unconditionally "int". A great example of this kind of
obfuscation is glib.

In my view, what's going on here is in some ways similar. Weak aliases
are the right mechanism in all cases, but if you hide that with
different usage-specific macros, you create a barrier to reading where
the reader might think there's something more complicated going on
behind all the different macros.

As for weak functions rather than aliases, it was a conscious choice
early on to use a minimum number of gcc- or gnu-linker-specific
features. Unlike other weak symbol related features, weak aliases are
universally available on all unix-like toolchains; they were even part
of SVID, which specifies #pragma weak to define them; a C99 compiler
following this practice would necessarily need to support _Pragma
weak, which could be used to define the weak_alias macro in a
"portable" way. Whether there's any practical value now to minimizing
the use of non-essential gcc features is open to question, but I see
no need to gratuitously increase the number used.

With that said, one thing I highly agree with in your patchset is
documenting the different weak alias tricks used in musl --
specifically, commenting on which weak aliases are expected to be
replaced by strong definitions, and under what conditions those strong
definitions come into place.

I also can see some use for having common dummy functions, but I'd
have to review how you're proposing to do it because I remember
looking into this before and finding that it didn't work as I
expected. However, the savings are likely to be very small, since
dummy functions are typically just 1-2 bytes on x86 (depending on the
-mtune mode) and 4 bytes on risc archs.

I'll give a more detailed review of individual patches later; I just
wanted to get this email off so you have ideas early on where you're
probably going to run into differences of opinion.

Rich


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/5] reorganize the use of weak symbols
  2013-02-16  5:59 ` [PATCH 0/5] reorganize the use of weak symbols Rich Felker
@ 2013-02-16  8:16   ` Jens Gustedt
  0 siblings, 0 replies; 8+ messages in thread
From: Jens Gustedt @ 2013-02-16  8:16 UTC (permalink / raw)
  To: musl

[-- Attachment #1: Type: text/plain, Size: 1637 bytes --]

Hello Rich,
yes, please see these patches mainly as an attempt for documentation
and readability.  They shouldn't change much on the reality of musl,
but perhaps by adding some bytes to some objects. I arbitrarily chose
32 bytes for the dummies. So this will perhaps add some hundred bytes
in total, unless musl is compiled with separate sections for each
symbol.

For the weak symbols versus weak alias, the patches mostly use weak
aliases as before, they only add a level of "weakness" for the dummy
objects and functions. The way I organized it, you could easily change
that to "static" again. So if weak symbols wouldn't be provided by all
tool chains it wouldn't be difficult at all to use that. (Currently I
see that the code is very gcc centric, so I don't think that this a
real issue.)

Where I really like to insist is the only case where a rw array is
aliased to a ro object, patch 2. Here the "dangerous" thing is done by
the aliasing and it should be documented there. The "wrong" type in
the local declaration of the array and the fact to later cast the
const away distracted from that fact.

So in summary, don't hesitate to modify what I wrote, give me feedback
so I'd do that, or not to take them at all, whatever you think is
appropriate for the project.

Jens

-- 
:: INRIA Nancy Grand Est :: http://www.loria.fr/~gustedt/   ::
:: AlGorille ::::::::::::::: office Nancy : +33 383593090   ::
:: ICube :::::::::::::: office Strasbourg : +33 368854536   ::
:: ::::::::::::::::::::::::::: gsm France : +33 651400183   ::
:: :::::::::::::::::::: gsm international : +49 15737185122 ::



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2013-02-16  8:16 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-15 23:22 [PATCH 0/5] reorganize the use of weak symbols Jens Gustedt
2013-02-15 23:23 ` [PATCH 1/5] Clearly identify the readonly replacement symbols that serve as 'dummies' that could (or could not) be provided by other compilation units Jens Gustedt
2013-02-15 23:23 ` [PATCH 2/5] Clarify the implementation of the dummy alias used for pthread_self Jens Gustedt
2013-02-15 23:24 ` [PATCH 3/5] identify the weak function symbols that provide a real default action Jens Gustedt
2013-02-15 23:24 ` [PATCH 4/5] add three macros for empty dummy functions that do nothing Jens Gustedt
2013-02-15 23:25 ` [PATCH 5/5] Use the weak functions that do nothing as aliases for the default actions Jens Gustedt
2013-02-16  5:59 ` [PATCH 0/5] reorganize the use of weak symbols Rich Felker
2013-02-16  8:16   ` Jens Gustedt

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).