Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20170831225230.GJ1627@brightrain.aerifal.cx>
Date: Thu, 31 Aug 2017 18:52:30 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: simplification of __aeabi_read_tp

On Fri, Sep 01, 2017 at 12:26:05AM +0200, Jörg Mische wrote:
> Hi,
> 
> I am trying to adapt the ARM assembler parts to ARMv6-M (the thumb2
> subset of the Cortex-M0) without breaking ARMv4T compatibility. One
> issue is the function __aeabi_read_tp(), which may not clobber any
> registers except the return value in r0.
> 
> Register saving code that avoids "pop {lr}" (which is not supported
> by ARMv6-M) and "pop {pc}" (which is not supported by ARMv4T) is
> very ugly, therefore I took a closer look at its internals and
> discovered the following:
> 
> __aeabi_read_tp() calls __aeabi_read_tp_c() which inlines the
> function __pthread_self(). With ARMv7 and above, __pthread_self()
> simply reads the coprocessor register c13 without clobbering any
> registers. Below ARMv7, the function pointer __a_gettp_ptr is
> called. __a_gettp_ptr either points to __a_gettp_cp15() (a routine
> that reads c13) or to the kuser_get_tls function provided by the
> kernel.
> 
> The interesting point is that neither __a_gettp_cp15() (only one
> instruction and a return) nor kuser_get_tls (according to the kernel
> spec) clobber any registers. The only reason for saving the
> registers is the indirection via the C-function __aeabi_read_tp_c(),
> where the compiler is allowed to clobber r0-r3.
> 
> Since inline functions cannot be called from assembler and any C
> code must be avoided, I rewrote the code of __pthread_self()
> directly in assembler in __aeabi_read_tp.S. With these modifications
> the binary code is not only faster, it also works on the Cortex-M0
> processor.

If you look at the commit text for commit
29237f7f5c09c436825a7a12b68ab4143b0ebd1f which added the indirection
through C code, one of the goals was to make the arm target
fdpic-ready (to support shareable text on cortex-m).

> diff --git a/src/thread/arm/__aeabi_read_tp.S
> b/src/thread/arm/__aeabi_read_tp.S
> new file mode 100644
> index 0000000..897b4f8
> --- /dev/null
> +++ b/src/thread/arm/__aeabi_read_tp.S
> @@ -0,0 +1,22 @@
> +.syntax unified
> +.global __a_gettp_ptr
> +.hidden __a_gettp_ptr
> +.global __aeabi_read_tp
> +.type __aeabi_read_tp,%function
> +__aeabi_read_tp:
> +
> +#if ((__ARM_ARCH_6K__ || __ARM_ARCH_6ZK__) && !__thumb__) ||
> __ARM_ARCH_7A__ || __ARM_ARCH_7R__ || __ARM_ARCH >= 7
> +
> +	mrc p15,0,r0,c13,c0,3
> +	bx lr
> +
> +#else
> +
> +	ldr r0,2f
> +	add r0,r0,pc
> +	ldr r0,[r0]
> +1:	bx r0
> +	.align 2
> +2:	.word __a_gettp_ptr-1b
> +
> +#endif

Here, __a_gettp_ptr-1b is not position-independent on fdpic, since
__a_gettp_ptr lies in the data segment and 1b lies in the text
segment, whose base addresses can float relative to one another. Of
course we could write an fdpic-specific version of the asm function to
access the hidden GOT pointer argument and do a GOT-relative load, but
the intent here was for the compiler to take care of all the ABI
issues.

OTOH, relying on the compiler not to generate code that could clobber
other normally-call-clobbered registers (the floating point ones) is
probably wrong, so maybe what I did here is just a bad idea and your
approach (with an extra special case for fdpic once it's added) is the
right way.

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.