Re: [PATCH v6 10/13] ARC: ABI lists

[Adhemerval Zanella via Libc-alpha <libc-alpha@sourceware.org>]

On 04/06/2020 23:22, Vineet Gupta wrote:
> On 6/3/20 12:29 PM, Adhemerval Zanella via Libc-alpha wrote:
>>
>>
>> On 22/04/2020 22:41, Vineet Gupta via Libc-alpha wrote:
>>> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
>>
>> LGTM with a just isseu regarding the pulled memcpy from libgcc.
> 
> Just to be clear
> 
> This is libgcc routine calling memcpy from gcc and libgcc-static linked into libc
> pulling in this PLT. And this is only for soft float + Os.  And there's just this
> one instance in libc which uses PLT, all other internal memcpy calls are direct.
> 
> $ arc-linux-objdump -d lib/libc-2.31.9000.so
> 
> 0001b014 <.plt>:
> ...
>    1b0e4:	ld	r12,[pcl,0xe6f50] ;102034 <memcpy@@GLIBC_2.32+0x9b020>
>    1b0ec:	j.d	[r12]
>    1b0f0:	mov	r12,pcl
> 
> 000d0fd8 <_fpadd_parts>:
> ...
>    d17ba:	bl	1b0e4 <.plt+0xd0>	<-- memcpy via PLT
> 
> 
> 000655a4 <__libc_alloc_buffer_copy_bytes>:
> ...
>    655c4:	bl.d	6736	;67014 <memcpy>
> ...
> 
> 00067014 <memcpy>:
>    67014:	push_s	blink
> ...
> 

Yes, I understood the issue.

>>> diff --git a/sysdeps/unix/sysv/linux/arc/localplt.data b/sysdeps/unix/sysv/linux/arc/localplt.data
>>> new file mode 100644
>>> index 000000000000..4479e8ee8a26
>>> --- /dev/null
>>> +++ b/sysdeps/unix/sysv/linux/arc/localplt.data
>>> @@ -0,0 +1,12 @@
>>> +libc.so: realloc
>>> +libc.so: malloc
>>> +libc.so: memalign
>>> +libc.so: calloc
>>> +libc.so: free
>>> +# At -Os, a struct assignment in libgcc-static pulls this in
>>> +libc.so: memcpy ?
> 
>> This is mosty likely an issue 
> 
> Care to elaborate what the issue is exactly: that memcpy being local to libc dso
> is still routed via PLT ? Or that the entry is only present sometimes ?

Besides the interposition and performance issue I think the main
drawback by whitelisting memcpy is potentially hides other possible
intra PLT memcpy in localplt.data future testing (although glibc does
some effort to avoid the issue of compiler generate memcpy/memmove).

> 
> 
>> and we handle this issue with an
>> assembly hack to redirect the autogenerate memcpy/memmove calls
>> to internal definitions (sysdeps/generic/symbol-hacks.h).
>>
>> I am not sure exactly if this would be avoided in ARC configuration,
>> maybe it could adapt the sparc32 hack for the libgcc .udiv symbol
>> (sysdeps/sparc/sparc32/Makefile).
> 
> I looked at the code and this seems different than our issue.
> 

I think it is similar: for sparc32 the divdi3 is pulled from libgcc
and it in turns calls an libgcc provided symbol (.udiv) which is
also provided by glibc.

What I am not sure is if we can use the the same hack to force
the linker bind the memcpy libgcc internal usage to glibc provided 
symbol.

I would try to do something like:

diff --git a/sysdeps/arc/Makefile b/sysdeps/arc/Makefile
index 8d2fbe00c5..ed2f43d2a6 100644
--- a/sysdeps/arc/Makefile
+++ b/sysdeps/arc/Makefile
@@ -20,6 +20,8 @@
 # this variable; it's here mostly for documentational purposes.
 long-double-fcts = no
 
+libc.so-gnulib += -Wl,--wrap=memcpy
+
 ifeq ($(subdir),debug)
 CFLAGS-backtrace.c += -funwind-tables
 endif

And add a sysdeps/arc/symbol-hacks.h with something like:

#if !defined __ASSEMBLER__ && IS_IN (libc) && defined SHARED
asm ("memcpy = __wrap_memcpy");
#endif


I also tried to build a soft-float BE with -Os using the latest
tools at the link you added in cover-letter [1] but I couldn't
trigger the issue.  Is is triggering some more recent gcc only?

[1] https://github.com/foss-for-synopsys-dwc-arc-processors/toolchain/releases