| Message ID | 20160818203335.GL22524@waldemar-brodkorb.de |
|---|---|
| State | Not Applicable |
| Headers | show |
Hello, On Thu, 18 Aug 2016 22:33:35 +0200, Waldemar Brodkorb wrote: > What about following patch, which creates dummies for > pthread_mutex_* functions in the !SHARED case: > > From 8d11aa1b9a983e0422dffa84eb1a7b71c616a096 Mon Sep 17 00:00:00 2001 > From: Waldemar Brodkorb <wbx@uclibc-ng.org> > Date: Thu, 18 Aug 2016 08:17:36 +0200 > Subject: [PATCH] add dummies > > > Signed-off-by: Waldemar Brodkorb <wbx@uclibc-ng.org> > --- > libc/misc/internals/__uClibc_main.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/libc/misc/internals/__uClibc_main.c b/libc/misc/internals/__uClibc_main.c > index 9bb81fc..9320039 100644 > --- a/libc/misc/internals/__uClibc_main.c > +++ b/libc/misc/internals/__uClibc_main.c > @@ -81,6 +81,9 @@ static int __pthread_return_0 (pthread_mutex_t *unused) { return 0; } > weak_alias (__pthread_return_0, __pthread_mutex_lock) > weak_alias (__pthread_return_0, __pthread_mutex_trylock) > weak_alias (__pthread_return_0, __pthread_mutex_unlock) > +weak_alias (__pthread_return_0, pthread_mutex_lock) > +weak_alias (__pthread_return_0, pthread_mutex_trylock) > +weak_alias (__pthread_return_0, pthread_mutex_unlock) Hum, why not, but this raises a few questions: - Why does it work in the shared case and not in the static case? - What are those __pthread_mutex_* variants? Do we have a list of the pthread functions that libc.so/libc.a is supposed to provide? See the mail I just send about the axel/libintl issue where libintl also uses pthread_rwlock_*() without being linked with libpthread.so. Thomas
Hi Thomas, Thomas Petazzoni wrote, > Hello, > > On Thu, 18 Aug 2016 22:33:35 +0200, Waldemar Brodkorb wrote: > > > What about following patch, which creates dummies for > > pthread_mutex_* functions in the !SHARED case: > > > > From 8d11aa1b9a983e0422dffa84eb1a7b71c616a096 Mon Sep 17 00:00:00 2001 > > From: Waldemar Brodkorb <wbx@uclibc-ng.org> > > Date: Thu, 18 Aug 2016 08:17:36 +0200 > > Subject: [PATCH] add dummies > > > > > > Signed-off-by: Waldemar Brodkorb <wbx@uclibc-ng.org> > > --- > > libc/misc/internals/__uClibc_main.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/libc/misc/internals/__uClibc_main.c b/libc/misc/internals/__uClibc_main.c > > index 9bb81fc..9320039 100644 > > --- a/libc/misc/internals/__uClibc_main.c > > +++ b/libc/misc/internals/__uClibc_main.c > > @@ -81,6 +81,9 @@ static int __pthread_return_0 (pthread_mutex_t *unused) { return 0; } > > weak_alias (__pthread_return_0, __pthread_mutex_lock) > > weak_alias (__pthread_return_0, __pthread_mutex_trylock) > > weak_alias (__pthread_return_0, __pthread_mutex_unlock) > > +weak_alias (__pthread_return_0, pthread_mutex_lock) > > +weak_alias (__pthread_return_0, pthread_mutex_trylock) > > +weak_alias (__pthread_return_0, pthread_mutex_unlock) > > Hum, why not, but this raises a few questions: > > - Why does it work in the shared case and not in the static case? > > - What are those __pthread_mutex_* variants? > > Do we have a list of the pthread functions that libc.so/libc.a is > supposed to provide? See the mail I just send about the axel/libintl > issue where libintl also uses pthread_rwlock_*() without being linked > with libpthread.so. I discussed the issues with Rich Felker and he is suggesting _not_ to do some wacky weak/strong handling of these symbols to provide some hackish way for applications to save some space. We should just link with -lpthread if any pthread_* function is in use to avoid any surprises when actually running the application. So please add explicit -lpthread for these static linking failures and add in libpthread.a functions. It might be just by accident that libc.so provides these function dummies for external uses and may be we should fix this instead. Thanks Waldemar
Hello, [Adding Rich in Cc.] On Fri, 19 Aug 2016 19:34:43 +0200, Waldemar Brodkorb wrote: > > Do we have a list of the pthread functions that libc.so/libc.a is > > supposed to provide? See the mail I just send about the axel/libintl > > issue where libintl also uses pthread_rwlock_*() without being linked > > with libpthread.so. > > I discussed the issues with Rich Felker and he is suggesting _not_ > to do some wacky weak/strong handling of these symbols to provide > some hackish way for applications to save some space. > We should just link with -lpthread if any pthread_* function is in > use to avoid any surprises when actually running the application. > > So please add explicit -lpthread for these static linking failures > and add in libpthread.a functions. It might be just by accident that > libc.so provides these function dummies for external uses and may be > we should fix this instead. I'm sorry but I disagree with Rich's proposal here. Lots of libraries rely on this behavior, and we will not be able to upstream the change that consists in linking with -lpthread, because it means a performance degradation when those libraries are used in mono-threaded applications. It is *not* about saving some space like Rich said. It is about making the mutex lock/unlock operations no-ops when they are not needed. So I'm sorry, but uClibc should implement this behavior, or you will have dozens of upstream projects to convince :-/ Thomas
On Fri, Aug 19, 2016 at 07:53:20PM +0200, Thomas Petazzoni wrote: > Hello, > > [Adding Rich in Cc.] > > On Fri, 19 Aug 2016 19:34:43 +0200, Waldemar Brodkorb wrote: > > > > Do we have a list of the pthread functions that libc.so/libc.a is > > > supposed to provide? See the mail I just send about the axel/libintl > > > issue where libintl also uses pthread_rwlock_*() without being linked > > > with libpthread.so. > > > > I discussed the issues with Rich Felker and he is suggesting _not_ > > to do some wacky weak/strong handling of these symbols to provide > > some hackish way for applications to save some space. > > We should just link with -lpthread if any pthread_* function is in > > use to avoid any surprises when actually running the application. > > > > So please add explicit -lpthread for these static linking failures > > and add in libpthread.a functions. It might be just by accident that > > libc.so provides these function dummies for external uses and may be > > we should fix this instead. > > I'm sorry but I disagree with Rich's proposal here. Lots of libraries > rely on this behavior, and we will not be able to upstream the change > that consists in linking with -lpthread, because it means a performance > degradation when those libraries are used in mono-threaded applications. > > It is *not* about saving some space like Rich said. It is about making > the mutex lock/unlock operations no-ops when they are not needed. > > So I'm sorry, but uClibc should implement this behavior, or you will > have dozens of upstream projects to convince :-/ If they want to keep taking advantage of the nop-out hack for dynamic linking glibc, a suitable solution would be a configure check something like: checking whether -larchive needs -lpthread... yes However, I'm not sure glibc will even be keeping this behavior long-term, as it's silent-error-prone. Some programmers have been very unpleasantly surprised to find out that the dummy lock functions in glibc's libc.{so,a} don't even enforce any exclusion. I believe this was visible in trylock succeeding on an already-locked mutex where it should have failed. I just did a quick search to see if I could find this discussion on the bug tracker but I didn't find it; it might be on the mailing list or somewhere else, or just buried somewhere that's hard to search. Note that because there exist process-shared mutexes, the dummy implementation is actually unsafe. An application might omit linking -lpthread because it sees that it links fine without it, but then mmap some shared memory and use pthread_mutex_lock for inter-process synchronization. Very bad silent breakage then happens. IMO the right fix is not to link a dummy implementation but instead to have the real implementation provide fast-paths for (!multi_threaded && !process_shared) cases. Rich
Hello, On Fri, 19 Aug 2016 14:03:45 -0400, Rich Felker wrote: > If they want to keep taking advantage of the nop-out hack for dynamic > linking glibc, a suitable solution would be a configure check > something like: > > checking whether -larchive needs -lpthread... yes That's a whole lot of work in potentially numerous upstream packages :-/ Also, there's still the odd thing that with uClibc, linking an application using pthread_mutex_lock/unlock with just libc works fine when dynamic linking, but not when static linking. If the decision is indeed that there is no fallback implementation in libc, then there shouldn't be any fallback implementation, regardless of whether we're static linking or not. > However, I'm not sure glibc will even be keeping this behavior > long-term, as it's silent-error-prone. Some programmers have been very > unpleasantly surprised to find out that the dummy lock functions in > glibc's libc.{so,a} don't even enforce any exclusion. I believe this > was visible in trylock succeeding on an already-locked mutex where it > should have failed. I just did a quick search to see if I could find > this discussion on the bug tracker but I didn't find it; it might be > on the mailing list or somewhere else, or just buried somewhere that's > hard to search. > > Note that because there exist process-shared mutexes, the dummy > implementation is actually unsafe. An application might omit linking > -lpthread because it sees that it links fine without it, but then mmap > some shared memory and use pthread_mutex_lock for inter-process > synchronization. Very bad silent breakage then happens. Indeed. > IMO the right fix is not to link a dummy implementation but instead to > have the real implementation provide fast-paths for (!multi_threaded > && !process_shared) cases. OK, makes sense. Waldemar, how do you see things moving forward in uClibc? Removing the fallback implementation entirely? Thomas
Hi Thomas, Thomas Petazzoni wrote, > Hello, > > On Fri, 19 Aug 2016 14:03:45 -0400, Rich Felker wrote: > > > If they want to keep taking advantage of the nop-out hack for dynamic > > linking glibc, a suitable solution would be a configure check > > something like: > > > > checking whether -larchive needs -lpthread... yes > > That's a whole lot of work in potentially numerous upstream packages :-/ > > Also, there's still the odd thing that with uClibc, linking an > application using pthread_mutex_lock/unlock with just libc works fine > when dynamic linking, but not when static linking. > > If the decision is indeed that there is no fallback implementation in > libc, then there shouldn't be any fallback implementation, regardless > of whether we're static linking or not. > > > However, I'm not sure glibc will even be keeping this behavior > > long-term, as it's silent-error-prone. Some programmers have been very > > unpleasantly surprised to find out that the dummy lock functions in > > glibc's libc.{so,a} don't even enforce any exclusion. I believe this > > was visible in trylock succeeding on an already-locked mutex where it > > should have failed. I just did a quick search to see if I could find > > this discussion on the bug tracker but I didn't find it; it might be > > on the mailing list or somewhere else, or just buried somewhere that's > > hard to search. > > > > Note that because there exist process-shared mutexes, the dummy > > implementation is actually unsafe. An application might omit linking > > -lpthread because it sees that it links fine without it, but then mmap > > some shared memory and use pthread_mutex_lock for inter-process > > synchronization. Very bad silent breakage then happens. > > Indeed. > > > IMO the right fix is not to link a dummy implementation but instead to > > have the real implementation provide fast-paths for (!multi_threaded > > && !process_shared) cases. > > OK, makes sense. > > Waldemar, how do you see things moving forward in uClibc? Removing the > fallback implementation entirely? Removing the visibility of the pthread_mutex_* dummies would be the best, so that dynamic linking is failing, when -lpthread is missing. Isn't it like with librt? Usually you use -lrt when using any advanced realtime functions from the C library. And any tutorial you find with your favourite internet search engine mentions to use -lpthread or -pthread when linking an application which uses pthread.h. So who is actually to blame for this situation that application programmers try to use pthread.h without linking libpthread? :) http://linux.die.net/man/7/pthreads "On Linux, programs that use the Pthreads API should be compiled using cc -pthread." best regards Waldemar
On Fri, Aug 19, 2016 at 09:46:44PM +0200, Waldemar Brodkorb wrote: > Hi Thomas, > Thomas Petazzoni wrote, > > > Hello, > > > > On Fri, 19 Aug 2016 14:03:45 -0400, Rich Felker wrote: > > > > > If they want to keep taking advantage of the nop-out hack for dynamic > > > linking glibc, a suitable solution would be a configure check > > > something like: > > > > > > checking whether -larchive needs -lpthread... yes > > > > That's a whole lot of work in potentially numerous upstream packages :-/ > > > > Also, there's still the odd thing that with uClibc, linking an > > application using pthread_mutex_lock/unlock with just libc works fine > > when dynamic linking, but not when static linking. > > > > If the decision is indeed that there is no fallback implementation in > > libc, then there shouldn't be any fallback implementation, regardless > > of whether we're static linking or not. > > > > > However, I'm not sure glibc will even be keeping this behavior > > > long-term, as it's silent-error-prone. Some programmers have been very > > > unpleasantly surprised to find out that the dummy lock functions in > > > glibc's libc.{so,a} don't even enforce any exclusion. I believe this > > > was visible in trylock succeeding on an already-locked mutex where it > > > should have failed. I just did a quick search to see if I could find > > > this discussion on the bug tracker but I didn't find it; it might be > > > on the mailing list or somewhere else, or just buried somewhere that's > > > hard to search. > > > > > > Note that because there exist process-shared mutexes, the dummy > > > implementation is actually unsafe. An application might omit linking > > > -lpthread because it sees that it links fine without it, but then mmap > > > some shared memory and use pthread_mutex_lock for inter-process > > > synchronization. Very bad silent breakage then happens. > > > > Indeed. > > > > > IMO the right fix is not to link a dummy implementation but instead to > > > have the real implementation provide fast-paths for (!multi_threaded > > > && !process_shared) cases. > > > > OK, makes sense. > > > > Waldemar, how do you see things moving forward in uClibc? Removing the > > fallback implementation entirely? > > Removing the visibility of the pthread_mutex_* dummies would be the > best, so that dynamic linking is failing, when -lpthread is > missing. This breakage (and the need for upstream application fixes) can be avoided by moving the lock functions into libc.{a,so}. IIRC for glibc that's a bit messy because of symbol versioning. Not sure how hard it would be for uclibc[-ng]. > Isn't it like with librt? Usually you use -lrt when using any > advanced realtime functions from the C library. > And any tutorial you find with your favourite internet search engine > mentions to use -lpthread or -pthread when linking an application > which uses pthread.h. > > So who is actually to blame for this situation that application > programmers try to use pthread.h without linking libpthread? :) > > http://linux.die.net/man/7/pthreads > "On Linux, programs that use the Pthreads API should be compiled > using cc -pthread." I agree but I think Thomas wants to avoid the need to patch/fix all the broken application build systems out there. Rich
Hello, On Fri, 19 Aug 2016 21:46:44 +0200, Waldemar Brodkorb wrote: > Isn't it like with librt? Usually you use -lrt when using any > advanced realtime functions from the C library. > And any tutorial you find with your favourite internet search engine > mentions to use -lpthread or -pthread when linking an application > which uses pthread.h. > > So who is actually to blame for this situation that application > programmers try to use pthread.h without linking libpthread? :) > > http://linux.die.net/man/7/pthreads > "On Linux, programs that use the Pthreads API should be compiled > using cc -pthread." The key part here is *programs*. Here, the situation is about a library that uses some aspects of the pthread API without knowing if the program is multi-threaded or not. I could find some old references about libxml2 (which uses pthread_* symbols but doesn't link with libpthread): """ > I've since chatted with Daniel (the upstream maintainer of > libxml2). He's in agreement with me that linking libxml2 with pthreads > is an unacceptable performance hit. I also have the issue that I do not > want to link my application with pthreads, and I don't think I should > have to. """ See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=142754. But there is some other relevant discussion at https://bugzilla.gnome.org/show_bug.cgi?id=704904. This is really my main concern: how many packages will we have to patch? Will those patches make their way upstream? This is especially true if we diverge from glibc's behavior, which (for good or bad) is generally considered by most developers as the "right" behavior. Thomas
Hello, On Fri, 19 Aug 2016 16:14:31 -0400, Rich Felker wrote: > > Isn't it like with librt? Usually you use -lrt when using any > > advanced realtime functions from the C library. > > And any tutorial you find with your favourite internet search engine > > mentions to use -lpthread or -pthread when linking an application > > which uses pthread.h. > > > > So who is actually to blame for this situation that application > > programmers try to use pthread.h without linking libpthread? :) > > > > http://linux.die.net/man/7/pthreads > > "On Linux, programs that use the Pthreads API should be compiled > > using cc -pthread." > > I agree but I think Thomas wants to avoid the need to patch/fix all > the broken application build systems out there. I'm all for fixing things. But my fear is that it will probably be very difficult to convince the different upstream project maintainers that they *must* now link with libpthread. Unless of course, we each time come up with a mechanism (autoconf logic, CMake logic, etc.) that determines whether linking with libpthread is needed or not. Thomas
Hi Thomas, Thomas Petazzoni wrote, > Hello, > > On Fri, 19 Aug 2016 21:46:44 +0200, Waldemar Brodkorb wrote: > > > Isn't it like with librt? Usually you use -lrt when using any > > advanced realtime functions from the C library. > > And any tutorial you find with your favourite internet search engine > > mentions to use -lpthread or -pthread when linking an application > > which uses pthread.h. > > > > So who is actually to blame for this situation that application > > programmers try to use pthread.h without linking libpthread? :) > > > > http://linux.die.net/man/7/pthreads > > "On Linux, programs that use the Pthreads API should be compiled > > using cc -pthread." > > The key part here is *programs*. > > Here, the situation is about a library that uses some aspects of the > pthread API without knowing if the program is multi-threaded or not. > > I could find some old references about libxml2 (which uses pthread_* > symbols but doesn't link with libpthread): > > """ > > I've since chatted with Daniel (the upstream maintainer of > > libxml2). He's in agreement with me that linking libxml2 with pthreads > > is an unacceptable performance hit. I also have the issue that I do not > > want to link my application with pthreads, and I don't think I should > > have to. > """ > > See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=142754. > > But there is some other relevant discussion at > https://bugzilla.gnome.org/show_bug.cgi?id=704904. > > This is really my main concern: how many packages will we have to > patch? Will those patches make their way upstream? This is especially > true if we diverge from glibc's behavior, which (for good or bad) is > generally considered by most developers as the "right" behavior. The problem here is, we try to support static linking, which isn't enabled for Buildroot GNU libc toolchains. So you do not see the problems we are facing with uClibc-ng now. If you really do not want to diverge from GNU libc behaviour, we can just keep uClibc-ng as is, but you still need to add -lpthread in case of static linking. This would be even required when using GNU libc for static linking, which is technically possible and supported by upstream. (with any known limitations regarding pthread_cancel+libgcc_s.so.1 and nss_* functions) I am not entirely sure, what is the best way to do. But I am willing to help getting these pthread bugfixes into upstream projects, if we go this way. Or we make a new option: UCLIBC_COMBINED_LIBC which can be enabled to mimic musl behaviour and adds librt/libm/libpthread into a single libc :) If libxml2 isn't supporting static linking, we have one less problem to solve ;) best regards Waldemar
diff --git a/libc/misc/internals/__uClibc_main.c b/libc/misc/internals/__uClibc_main.c index 9bb81fc..9320039 100644 --- a/libc/misc/internals/__uClibc_main.c +++ b/libc/misc/internals/__uClibc_main.c @@ -81,6 +81,9 @@ static int __pthread_return_0 (pthread_mutex_t *unused) { return 0; } weak_alias (__pthread_return_0, __pthread_mutex_lock) weak_alias (__pthread_return_0, __pthread_mutex_trylock) weak_alias (__pthread_return_0, __pthread_mutex_unlock) +weak_alias (__pthread_return_0, pthread_mutex_lock) +weak_alias (__pthread_return_0, pthread_mutex_trylock) +weak_alias (__pthread_return_0, pthread_mutex_unlock) int weak_function __pthread_mutex_init(pthread_mutex_t *mutex, const pthread_mutexattr_t *attr)