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

Florian Weimer fweimer@redhat.com
Tue Jan 7 11:15:28 GMT 2025


* Michael Jeanson:

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

Ah, makes sense.

I've worked on posting a patch for exposing TLS_TP_OFFSET
unconditionally, but the test case for it hits a snag: we still don't
have <thread_pointer.h> on all architectures.  I see build failures on:

  alpha
  arc
  arm
  mips
  s390
  s390x
  sh

Of these, only mips and sh do not yet assume that GCC's
__builtin_thread_pointer work.

We could disable the new test, but perhaps we should complete the
<thread_pointer.h> implementation?  Not sure where this puts us in terms
of inclusion in the upcoming release.  I'll try to add a generic
<thread_pointer.h> and implementations for mips and sh.

I also looked at an alternative implementation: make the size of struct
pthread dynamic.  This minimizes changes on architectures such as
x86-64, at least if we assume that 32-byte alignment of the rseq area is
enough.  It's more complicated on architectures such as AArch64, where
THREAD_SELF would need to perform variable adjustment.  It does not look
like an overall win in terms of complexity.

Michael, before reposting your changes, you could fix some style nits?
Mostly missing space before '(' in function calls, missing two spaces
after '.' at the end of the comment.  Copyright years will need
updating, too.

grep -E '^\+.*(Contributed by|http://|Copyright.*(19[0-9][0-9]|20[01][0-9]|202[0-4])|XXX|FIXME|TODO|\?\?\?|internal_function|\bu_(char|int)|the\s+the|[^	 *$({~]\(|\b(if|for|while|do)\b.*\{|[[:space:]]$|\($|\. \*/)'

Thanks,
Florian



More information about the Libc-alpha mailing list