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

[Bernhard Reutner-Fischer]

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.
>
> 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>

Applied, thanks!