[PATCH] weak symbols need to be "defined" weak but "declared" strong

[Bernhard Reutner-Fischer]

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