Joern, Vineet, On 16 April 2014 12:36, Vineet Gupta <[email protected]> wrote: > Patch "LT.old: Make __errno_location/__h_errno_location thread safe" > uncovered yet another bug with static linking and errno (hopefully this > is last of them all). > > Currently, __errno_location is declared weak but is defined strong. > While this provides with the desired weak semantics in dso, it > is subtly broken in static links. > > Quoting Joern Rennecke (ARC gcc expert): > > | I think the issue is that you declare the function as weak in the > | header file. That is a rare instance where you want the reference > | use declaration that differs a bit from the definition. > | If the reference uses a weakly declared function, that creates a > | weakref, i.e. the linker won't bother to look for this symbol at > | all - if it gets linked in for some other reason, fine, > | otherwise, it stays zero.
Sounds like this is, once again, http://gcc.gnu.org/PR36282 ; see patch referenced in that PR and the reason for our not_null_ptr() workaround, no? thanks, > > So the solution to declare strong, define weak. > > Supporting data > ----------------- > orig code: ARM mmap wrapper (LT.old build + my prev patch for errno) > > _mmap: > @ args = 8, pretend = 0, frame = 0 > @ frame_needed = 0, uses_anonymous_args = 0 > stmfd sp!, {r4, r5, r7, lr} > ldr r5, [sp, #20] > movs ip, r5, asl #20 > beq .L2 > bl __errno_location(PLT) > mov r3, #22 > str r3, [r0, #0] > mvn r0, #0 > ... > ... > .weak __errno_location > > A statically linked hello world program which uses mmap too. > As we can see__errno_location is completely gone - which is > semantically wrong - we need functional errno. > > 00008274 <__GI_mmap>: > 8274: e92d40b0 push {r4, r5, r7, lr} > 8278: e59d5014 ldr r5, [sp, #20] > 827c: e1b0ca05 lsls ip, r5, #20 > 8280: 0a000004 beq 8298 > 8284: e320f000 nop {0} > ^^^^^^^^^^ > 8288: e3a03016 mov r3, #22 > 828c: e5803000 str r3, [r0] > 8290: e3e00000 mvn r0, #0 > > This in turn is due to a fixup in ARM ld which transforms branch-to-null > into a nop. It is better than crashing but still wrong since errno > handling is removed. > > With the patch, errno_location is restored back in test program. > > 00008274 <__GI_mmap>: > 8274: e92d40b0 push {r4, r5, r7, lr} > 8278: e59d5014 ldr r5, [sp, #20] > 827c: e1b0ca05 lsls ip, r5, #20 > 8280: 0a000004 beq 8298 <__GI_mmap+0x24> > 8284: eb000010 bl 82cc <__errno_location> > 8288: e3a03016 mov r3, #22 > 828c: e5803000 str r3, [r0] > > Cc: Christian Ruppert <[email protected]> > CC: Francois Bedard <[email protected]> > Cc: Anton Kolesov <[email protected]> > Cc: Joern Rennecke <[email protected]> > Cc: Jeremy Bennett <[email protected]> > Cc: Thomas Petazzoni <[email protected]> > Cc: Peter Korsgaard <[email protected]> > Cc: Khem Raj <raj.khem@gmail > Signed-off-by: Vineet Gupta <[email protected]> > --- > include/netdb.h | 3 --- > libc/misc/internals/__errno_location.c | 2 +- > libc/misc/internals/__h_errno_location.c | 2 +- > libc/sysdeps/linux/common/bits/errno.h | 3 --- > 4 files changed, 2 insertions(+), 8 deletions(-) > > diff --git a/include/netdb.h b/include/netdb.h > index 0f361bb6f662..7ce01c24d8b4 100644 > --- a/include/netdb.h > +++ b/include/netdb.h > @@ -58,9 +58,6 @@ __BEGIN_DECLS > > /* Function to get address of global `h_errno' variable. */ > extern int *__h_errno_location (void) __THROW __attribute__ ((__const__)); > -#ifdef _LIBC > -extern int weak_const_function *__h_errno_location(void); > -#endif > > /* Macros for accessing h_errno from inside libc. */ > #ifdef _LIBC > diff --git a/libc/misc/internals/__errno_location.c > b/libc/misc/internals/__errno_location.c > index 9bbc2d779653..6c359f933d16 100644 > --- a/libc/misc/internals/__errno_location.c > +++ b/libc/misc/internals/__errno_location.c > @@ -12,7 +12,7 @@ > extern int errno; > #endif > > -int *__errno_location(void) > +int weak_const_function *__errno_location(void) > { > return &errno; > } > diff --git a/libc/misc/internals/__h_errno_location.c > b/libc/misc/internals/__h_errno_location.c > index b30859e81f19..c510c8143eae 100644 > --- a/libc/misc/internals/__h_errno_location.c > +++ b/libc/misc/internals/__h_errno_location.c > @@ -12,7 +12,7 @@ > extern int h_errno; > #endif > > -int *__h_errno_location(void) > +int weak_const_function *__h_errno_location(void) > { > return &h_errno; > } > diff --git a/libc/sysdeps/linux/common/bits/errno.h > b/libc/sysdeps/linux/common/bits/errno.h > index 611b8359001a..7c0aeb179a75 100644 > --- a/libc/sysdeps/linux/common/bits/errno.h > +++ b/libc/sysdeps/linux/common/bits/errno.h > @@ -42,9 +42,6 @@ > # ifndef __ASSEMBLER__ > /* Function to get address of global `errno' variable. */ > extern int *__errno_location (void) __THROW __attribute__ ((__const__)); > -# ifdef _LIBC > -extern int weak_const_function *__errno_location(void); > -# endif > > # ifdef __UCLIBC_HAS_THREADS__ > /* When using threads, errno is a per-thread value. */ > -- > 1.8.3.2 > > _______________________________________________ > uClibc mailing list > [email protected] > http://lists.busybox.net/mailman/listinfo/uclibc _______________________________________________ uClibc mailing list [email protected] http://lists.busybox.net/mailman/listinfo/uclibc
