[PATCH] weak symbols need to be "defined" weak but "declared" strong
Bernhard Reutner-Fischer
rep.dot.nop at gmail.com
Wed Apr 16 12:03:10 UTC 2014
Joern, Vineet,
On 16 April 2014 12:36, Vineet Gupta <Vineet.Gupta1 at synopsys.com> 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 <christian.ruppert at abilis.com>
> CC: Francois Bedard <Francois.Bedard at synopsys.com>
> Cc: Anton Kolesov <Anton.Kolesov at synopsys.com>
> Cc: Joern Rennecke <joern.rennecke at embecosm.com>
> Cc: Jeremy Bennett <jeremy.bennett at embecosm.com>
> Cc: Thomas Petazzoni <thomas.petazzoni at free-electrons.com>
> Cc: Peter Korsgaard <peter at korsgaard.com>
> Cc: Khem Raj <raj.khem at gmail
> Signed-off-by: Vineet Gupta <vgupta at synopsys.com>
> ---
> 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
> uClibc at uclibc.org
> http://lists.busybox.net/mailman/listinfo/uclibc
More information about the uClibc
mailing list