| Message ID | 1394633133-15657-1-git-send-email-vgupta@synopsys.com |
|---|---|
| State | Superseded |
| Headers | show |
>>>>> "Vineet" == Vineet Gupta <Vineet.Gupta1@synopsys.com> writes: > [summary: Get rid of libc alias __GI___errno_location] > It seems with Linuxthreads.old (yet to confirm NPTL) errno is not thread > safe. > A simple pthread linked test program (at the bottom) which makes a > failing syscall e.g. open("/not-exist") fails to observe the right errno > in the thread (main is OK) > Conceptually uClibc defines weak __errno_location() while libpthread > defines astrong variant. This arrangement shd work when using -pthread > links. The spoil sport is __GI___errno_location, intended to bypass PLT > for intra-libc callers. It gets called even in case of LT.old links > given the syscall wrappers in libpthread (LT.old). e.g. > open [ in libpthread ] > pthreadsetcanceltype() > __libc_open() > __GI__open() > ... > - __GI___errno_location [ existing ] > + __errno_location [ intended ] > .... > pthreadsetcanceltype() > So the solution is to get rid of GI alias for errno_location > altogether. That sounds pretty much like: http://lists.uclibc.org/pipermail/uclibc/2010-July/044176.html Which afaik never got committed.
On Wednesday 12 March 2014 07:42 PM, Peter Korsgaard wrote: > That sounds pretty much like: > http://lists.uclibc.org/pipermail/uclibc/2010-July/044176.html Which afaik never > got committed. Indeed and the changelog is one million times better than mine :-) So looking back at the orig bug log, it seems it was concluded that fix was legit so why was it not merged - @Bernhard you made a last comment 'This works with NPTL" but it is broken in LT.old Thx, -Vineet
On Wed, Mar 12, 2014 at 02:20:20PM +0000, Vineet Gupta wrote: > On Wednesday 12 March 2014 07:42 PM, Peter Korsgaard wrote: > > That sounds pretty much like: > > http://lists.uclibc.org/pipermail/uclibc/2010-July/044176.html Which afaik never > > got committed. > > Indeed and the changelog is one million times better than mine :-) > > So looking back at the orig bug log, it seems it was concluded that fix was legit > so why was it not merged - @Bernhard you made a last comment 'This works with > NPTL" but it is broken in LT.old Abovementioned patch was applied by Austin but quickly reverted since it broke static (IIRC) use.
On Thursday 13 March 2014 01:33 AM, Bernhard Reutner-Fischer wrote: > On Wed, Mar 12, 2014 at 02:20:20PM +0000, Vineet Gupta wrote: >> On Wednesday 12 March 2014 07:42 PM, Peter Korsgaard wrote: >>> That sounds pretty much like: >>> http://lists.uclibc.org/pipermail/uclibc/2010-July/044176.html Which afaik never >>> got committed. >> Indeed and the changelog is one million times better than mine :-) >> >> So looking back at the orig bug log, it seems it was concluded that fix was legit >> so why was it not merged - @Bernhard you made a last comment 'This works with >> NPTL" but it is broken in LT.old > Abovementioned patch was applied by Austin but quickly reverted since > it broke static (IIRC) use. I forgot to mention that I've tested static links on ARC as well as ARM. That breakage must be due to an earlier issue I fixed where __errno_location was not weak in libc hence would collide when linking with libpthread -Vineet
On Thursday 13 March 2014 09:55 AM, Vineet Gupta wrote: > On Thursday 13 March 2014 01:33 AM, Bernhard Reutner-Fischer wrote: >> On Wed, Mar 12, 2014 at 02:20:20PM +0000, Vineet Gupta wrote: >>> On Wednesday 12 March 2014 07:42 PM, Peter Korsgaard wrote: >>>> That sounds pretty much like: >>>> http://lists.uclibc.org/pipermail/uclibc/2010-July/044176.html Which afaik never >>>> got committed. >>> Indeed and the changelog is one million times better than mine :-) >>> >>> So looking back at the orig bug log, it seems it was concluded that fix was legit >>> so why was it not merged - @Bernhard you made a last comment 'This works with >>> NPTL" but it is broken in LT.old >> Abovementioned patch was applied by Austin but quickly reverted since >> it broke static (IIRC) use. > > I forgot to mention that I've tested static links on ARC as well as ARM. > That breakage must be due to an earlier issue I fixed where __errno_location was > not weak in libc hence would collide when linking with libpthread > > -Vineet > Ping !
============Test case ===================
/* $CROSS-gcc -o errno-arc -lpthread */
void *th(void *str)
{
int fd;
printf("\n[%s] Initial errno = %d\n", (char *)str, errno);
fd = open("/nothing", O_RDONLY);
if (fd >= 0) {
close(fd);
//printf("success\n");
} else
printf("errno @%x = %d (%s)\n", &errno, errno, errno ? "OK" : "BROKEN");
errno = 66;
return NULL;
}
int main(void)
{
pthread_t t;
void *r;
int err;
errno = 99;
th("Main thread");
err = pthread_create(&t, NULL, th, "Child Thread");
if (err) {
fprintf(stderr, "could not spawn thread: %s\n", strerror(err));
return 1;
}
pthread_join(t, &r);
printf("Errno in main after child %d\n", errno);
return 0;
}
Cc: Christian Ruppert <christian.ruppert@abilis.com>
CC: Francois Bedard <Francois.Bedard@synopsys.com>
Cc: Joern Rennecke <joern.rennecke@embecosm.com>
Cc: Jeremy Bennett <jeremy.bennett@embecosm.com>
Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
libc/misc/internals/__errno_location.c | 1 -
libc/sysdeps/linux/common/bits/errno.h | 3 ---
2 files changed, 4 deletions(-)
diff --git a/libc/misc/internals/__errno_location.c b/libc/misc/internals/__errno_location.c
index be7a9093efa3..9bbc2d779653 100644
--- a/libc/misc/internals/__errno_location.c
+++ b/libc/misc/internals/__errno_location.c
@@ -16,4 +16,3 @@ int *__errno_location(void)
{
return &errno;
}
-libc_hidden_weak(__errno_location)
diff --git a/libc/sysdeps/linux/common/bits/errno.h b/libc/sysdeps/linux/common/bits/errno.h
index 777338fb1e0a..611b8359001a 100644
--- a/libc/sysdeps/linux/common/bits/errno.h
+++ b/libc/sysdeps/linux/common/bits/errno.h
@@ -43,11 +43,8 @@
/* Function to get address of global `errno' variable. */
extern int *__errno_location (void) __THROW __attribute__ ((__const__));
# ifdef _LIBC
-# if !defined(__UCLIBC_HAS_TLS__) && !defined(__UCLIBC_HAS_THREADS__)
extern int weak_const_function *__errno_location(void);
-# endif
# endif
-libc_hidden_proto(__errno_location)
# ifdef __UCLIBC_HAS_THREADS__
/* When using threads, errno is a per-thread value. */
[summary: Get rid of libc alias __GI___errno_location] It seems with Linuxthreads.old (yet to confirm NPTL) errno is not thread safe. A simple pthread linked test program (at the bottom) which makes a failing syscall e.g. open("/not-exist") fails to observe the right errno in the thread (main is OK) Conceptually uClibc defines weak __errno_location() while libpthread defines astrong variant. This arrangement shd work when using -pthread links. The spoil sport is __GI___errno_location, intended to bypass PLT for intra-libc callers. It gets called even in case of LT.old links given the syscall wrappers in libpthread (LT.old). e.g. open [ in libpthread ] pthreadsetcanceltype() __libc_open() __GI__open() ... - __GI___errno_location [ existing ] + __errno_location [ intended ] .... pthreadsetcanceltype() So the solution is to get rid of GI alias for errno_location altogether. I tested this on master for ARC as well as an ARMv7 buildroot LT.old build (daily snapshot of uClibc) and fix works on both. -------- before ------->8---------- [Main thread] Initial errno = 99 errno @b6f6c338 = 2 (OK) [Child Thread] Initial errno = 0 errno @be1ffee8 = 0 (BROKEN) Errno in main after child 4 ------- after ----->8------------------ [Main thread] Initial errno = 99 errno @b6f9a338 = 2 (OK) [Child Thread] Initial errno = 0 errno @be1ffee8 = 2 (OK) Errno in main after child 4