[PATCH v14 0/9] Add rseq extensible ABI support

[Michael Jeanson]

On 2025-01-06 13:54, Michael Jeanson wrote:
> On 2024-12-30 05:06, Florian Weimer wrote:
>> * Florian Weimer:
>>
>>> * Mathieu Desnoyers:
>>>
>>>>> So it looks like that TLS_DTV_AT_TP part doesn't work, unfortunately.
>>>>> I see many new failures (after the mentioned revert) on powerpc64le,
>>>>> which is one of those targets.  I haven't tried to reproduce them yet
>>>>> on AArch64.  The GCC compile farm has a powerpc64le test machine
>>>>> (gcc120.fsffrance.org).
>>>>
>>>> I recall that Michael did test on at least one architecture
>>>> with TLS_DTV_AT_TP (aarch64), and one with TLS_TCB_AT_TP (x86-64).
>>>>
>>>> The issue may be specific to powerpc.
>>>
>>> Maybe it's caused by a non-zero TLS_TP_OFFSET.  We use a shifted thread
>>> pointer on POWER, presumably to increase the reach of signed
>>> displacements in TLS-accessing instructions.
>>>
>>> This is alluded to in this comment:
>>>
>>> #ifdef RSEQ_SIG
>>>     /* This should be a compile-time constant, but the current
>>>        infrastructure makes it difficult to determine its value.  Not
>>>        all targets support __thread_pointer, so set __rseq_offset only
>>>        if the rseq registration may have happened because RSEQ_SIG is
>>>        defined.  */
>>>     _rseq_offset = (char *) &pd->rseq_area - (char *) __thread_pointer ();
>>> #endif
>>>
>>> If this is indeed the cause, we should define TLS_TP_OFFSET on all
>>> architectures and use it to initialize __rseq_offset.  Similar to what I
>>> started here:
>>>
>>>   [PATCH 1/4] elf: Introduce generic <dl-tls.h>
>>>   <https://inbox.sourceware.org/libc-alpha/1b470b147d0bfe51af6256b10cd38c14285a61b2.1735313702.git.fweimer@redhat.com/>
>>
>> With
>>
>> diff --git a/elf/dl-tls.c b/elf/dl-tls.c
>> index 57e72be4..1055032a 100644
>> --- a/elf/dl-tls.c
>> +++ b/elf/dl-tls.c
>> @@ -419,7 +419,7 @@ _dl_determine_tlsoffset (void)
>>      The alignment requirements of the pointer resulting from this offset and
>>      the thread pointer are enforced by 'max_align' which is used to align the
>>      tcb_offset.  */
>> -  _dl_extra_tls_set_offset(offset);
>> +  _dl_extra_tls_set_offset(offset - TLS_TP_OFFSET);
>>  
>>    /* Add the extra TLS block to the global offset.  */
>>    offset += extra_tls_size;
>>
>> the results look better, but I'm not sure if this is the correct fix.
>>
>> I see crashes on the scv 0 instruction in statically linked binaries.
>> Looks like the TCB placement is wrong and the flag that indicates
>> support for the new system call instruction (scv 0) is not loaded
>> correctly.

Your patch looks good to me but it's missing the same change for the
statically linked binary code, like this :

diff --git a/csu/libc-tls.c b/csu/libc-tls.c                                                                                                 index 20d6b48a4d..65ce9ddb3d 100644
--- a/csu/libc-tls.c
+++ b/csu/libc-tls.c
@@ -20,6 +20,7 @@
 #include <errno.h>
 #include <ldsodefs.h>
 #include <tls.h>
+#include <dl-tls.h>
 #include <unistd.h>
 #include <stdio.h>
 #include <sys/param.h>
@@ -195,7 +196,7 @@ __libc_setup_tls (void)
     The alignment requirements of the pointer resulting from this offset and
     the thread pointer are enforced by 'max_align' which is used to align the
     tcb_offset.  */
-  _dl_extra_tls_set_offset(tls_blocks_size - extra_tls_size);
+  _dl_extra_tls_set_offset(tls_blocks_size - extra_tls_size - TLS_TP_OFFSET);
 
   tlsblock = _dl_early_allocate (tls_blocks_size + max_align
                                 + TLS_PRE_TCB_SIZE


There are also minor adjustments required in the rseq tests since we can't
assume the rseq_offset to be greater than zero on architectures that have a
non-zero TLS_TP_OFFSET.

>From what I can see, all the architectures that define TLS_TP_OFFSET are of
the TLS_DTV_AT_TP type so we don't need to change the offset calculation in
the TLS_TCB_AT_TP code paths.

I ran the test suite on a ppc64el VM with the current master and then with
the RSEQ patchset and your patch plus my changes, the tests results are the
same.

	=== Summary of results ===
     18 FAIL
   5877 PASS
     21 UNSUPPORTED
     16 XFAIL
      2 XPASS

Do you want me to send an updated patchset or should I wait for further
review?

Michael