Re: [PATCH glibc 1/3] glibc: Perform rseq registration at C startup and thread creation (v19)
- On May 26, 2020, at 10:57 AM, Florian Weimer fwei...@redhat.com wrote:
> * Mathieu Desnoyers:
>
>>> Like the attribute, it needs to come right after the struct keyword, I
>>> think. (Trailing attributes can be ambiguous, but not in this case.)
>>
>> Nope. _Alignas really _is_ special :-(
>>
>> struct _Alignas (16) blah {
>> int a;
>> };
>>
>> p.c:1:8: error: expected ‘{’ before ‘_Alignas’
>> struct _Alignas (16) blah {
>
> Meh, yet another unnecessary C++ incompatibility. C does not support
> empty structs, so I assume they didn't see the field requirement as a
> burden.
Indeed, it's weird.
>
>> One last thing I'm planning to add in sys/rseq.h to cover acessing the
>> rseq_cs pointers with both the UAPI headers and the glibc struct rseq
>> declarations:
>>
>> /* The rseq_cs_ptr macro can be used to access the pointer to the current
>>rseq critical section descriptor. */
>> #ifdef __LP64__
>> # define rseq_cs_ptr(rseq) \
>>((const struct rseq_cs *) (rseq)->rseq_cs.ptr)
>> #else /* __LP64__ */
>> # define rseq_cs_ptr(rseq) \
>>((const struct rseq_cs *) (rseq)->rseq_cs.ptr.ptr32)
>> #endif /* __LP64__ */
>>
>> Does it make sense ?
>
> Written this way, it's an aliasing violation. I don't think it's very
> useful.
OK, I'll just remove it.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
Re: [PATCH glibc 1/3] glibc: Perform rseq registration at C startup and thread creation (v19)
* Mathieu Desnoyers:
>> Like the attribute, it needs to come right after the struct keyword, I
>> think. (Trailing attributes can be ambiguous, but not in this case.)
>
> Nope. _Alignas really _is_ special :-(
>
> struct _Alignas (16) blah {
> int a;
> };
>
> p.c:1:8: error: expected ‘{’ before ‘_Alignas’
> struct _Alignas (16) blah {
Meh, yet another unnecessary C++ incompatibility. C does not support
empty structs, so I assume they didn't see the field requirement as a
burden.
> One last thing I'm planning to add in sys/rseq.h to cover acessing the
> rseq_cs pointers with both the UAPI headers and the glibc struct rseq
> declarations:
>
> /* The rseq_cs_ptr macro can be used to access the pointer to the current
>rseq critical section descriptor. */
> #ifdef __LP64__
> # define rseq_cs_ptr(rseq) \
>((const struct rseq_cs *) (rseq)->rseq_cs.ptr)
> #else /* __LP64__ */
> # define rseq_cs_ptr(rseq) \
>((const struct rseq_cs *) (rseq)->rseq_cs.ptr.ptr32)
> #endif /* __LP64__ */
>
> Does it make sense ?
Written this way, it's an aliasing violation. I don't think it's very
useful.
Thanks,
Florian
Re: [PATCH glibc 1/3] glibc: Perform rseq registration at C startup and thread creation (v19)
- On May 26, 2020, at 10:38 AM, Florian Weimer fwei...@redhat.com wrote:
> * Mathieu Desnoyers:
>
>> AFAIU, the only gain here would be to make sure we don't emit useless
>> ";" in the "/* nothing */" case. But does it matter ?
>
> I don't think C allows empty constructs like this at the top level.
>
> And something similar for _Alignas/attribute aligned,
I don't see where _Alignas is needed here ?
For attribute aligned, what would be the oldest supported C and C++
standards ?
>>>
>>> There are no standardized attributes for C, there is only _Alignas.
>>> C++11 has an alignas specifier; it's not an attribute either. I think
>>> these are syntactically similar.
>>
>> There appears to be an interesting difference between attribute aligned
>> and alignas. It seems like alignas cannot be used on a structure declaration,
>> only on fields, e.g.:
>>
>> struct blah {
>> int a;
>> } _Alignas (16);
>>
>> o.c:3:1: warning: useless ‘_Alignas’ in empty declaration
>> } _Alignas (16);
>>
>> But
>>
>> struct blah {
>> int _Alignas (16) a;
>> };
>
> Like the attribute, it needs to come right after the struct keyword, I
> think. (Trailing attributes can be ambiguous, but not in this case.)
Nope. _Alignas really _is_ special :-(
struct _Alignas (16) blah {
int a;
};
p.c:1:8: error: expected ‘{’ before ‘_Alignas’
struct _Alignas (16) blah {
Also:
struct blah _Alignas (16) {
int a;
};
p.c:1:27: error: expected identifier or ‘(’ before ‘{’ token
struct blah _Alignas (16) {
>
>> is OK. So if I change e.g. struct rseq_cs to align
>> the first field:
>>
>> struct rseq_cs
>> {
>> /* Version of this structure. */
>> uint32_t rseq_align (32) version;
>> /* enum rseq_cs_flags. */
>> uint32_t flags;
>> uint64_t start_ip;
>> /* Offset from start_ip. */
>> uint64_t post_commit_offset;
>> uint64_t abort_ip;
>> };
>>
>> It should work.
>
> Indeed.
OK, so let's go for that approach.
>
>> /* Rely on GNU extensions for older standards and tls model. */
>> #ifdef __GNUC__
>> # ifndef rseq_alignof
>> # define rseq_alignof(x) __alignof__ (x)
>> # endif
>> # ifndef rseq_alignas
>> # define rseq_alignas(x) __attribute__ ((aligned (x)))
>> # endif
>> # define rseq_tls_model_ie __attribute__ ((__tls_model__ ("initial-exec")))
>> #else
>> /* Specifying the TLS model on the declaration is optional. */
>> # define rseq_tls_model_ie /* Nothing. */
>> #endif
>>
>> /* Fall back to __thread for TLS storage class. */
>> #ifndef rseq_tls_storage_class
>> # define rseq_tls_storage_class __thread
>> #endif
>
> If they are only used in the glibc headers, they should have __rseq
> prefixes, so that application code doesn't start using them (in case we
> have to change/fix them, or move the into later).
OK will do.
>
> Rest looks fine.
One last thing I'm planning to add in sys/rseq.h to cover acessing the
rseq_cs pointers with both the UAPI headers and the glibc struct rseq
declarations:
/* The rseq_cs_ptr macro can be used to access the pointer to the current
rseq critical section descriptor. */
#ifdef __LP64__
# define rseq_cs_ptr(rseq) \
((const struct rseq_cs *) (rseq)->rseq_cs.ptr)
#else /* __LP64__ */
# define rseq_cs_ptr(rseq) \
((const struct rseq_cs *) (rseq)->rseq_cs.ptr.ptr32)
#endif /* __LP64__ */
Does it make sense ?
Thanks,
Mathieu
>
> Thanks,
> Florian
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
Re: [PATCH glibc 1/3] glibc: Perform rseq registration at C startup and thread creation (v19)
* Mathieu Desnoyers:
> AFAIU, the only gain here would be to make sure we don't emit useless
> ";" in the "/* nothing */" case. But does it matter ?
I don't think C allows empty constructs like this at the top level.
And something similar for _Alignas/attribute aligned,
>>>
>>> I don't see where _Alignas is needed here ?
>>>
>>> For attribute aligned, what would be the oldest supported C and C++
>>> standards ?
>>
>> There are no standardized attributes for C, there is only _Alignas.
>> C++11 has an alignas specifier; it's not an attribute either. I think
>> these are syntactically similar.
>
> There appears to be an interesting difference between attribute aligned
> and alignas. It seems like alignas cannot be used on a structure declaration,
> only on fields, e.g.:
>
> struct blah {
> int a;
> } _Alignas (16);
>
> o.c:3:1: warning: useless ‘_Alignas’ in empty declaration
> } _Alignas (16);
>
> But
>
> struct blah {
> int _Alignas (16) a;
> };
Like the attribute, it needs to come right after the struct keyword, I
think. (Trailing attributes can be ambiguous, but not in this case.)
> is OK. So if I change e.g. struct rseq_cs to align
> the first field:
>
> struct rseq_cs
> {
> /* Version of this structure. */
> uint32_t rseq_align (32) version;
> /* enum rseq_cs_flags. */
> uint32_t flags;
> uint64_t start_ip;
> /* Offset from start_ip. */
> uint64_t post_commit_offset;
> uint64_t abort_ip;
> };
>
> It should work.
Indeed.
> /* Rely on GNU extensions for older standards and tls model. */
> #ifdef __GNUC__
> # ifndef rseq_alignof
> # define rseq_alignof(x) __alignof__ (x)
> # endif
> # ifndef rseq_alignas
> # define rseq_alignas(x) __attribute__ ((aligned (x)))
> # endif
> # define rseq_tls_model_ie __attribute__ ((__tls_model__ ("initial-exec")))
> #else
> /* Specifying the TLS model on the declaration is optional. */
> # define rseq_tls_model_ie /* Nothing. */
> #endif
>
> /* Fall back to __thread for TLS storage class. */
> #ifndef rseq_tls_storage_class
> # define rseq_tls_storage_class __thread
> #endif
If they are only used in the glibc headers, they should have __rseq
prefixes, so that application code doesn't start using them (in case we
have to change/fix them, or move the into later).
Rest looks fine.
Thanks,
Florian
Re: [PATCH glibc 1/3] glibc: Perform rseq registration at C startup and thread creation (v19)
- On May 26, 2020, at 8:41 AM, Florian Weimer fwei...@redhat.com wrote:
> * Mathieu Desnoyers:
>
>> Something like this ?
>>
>> #ifdef __cplusplus
>> # if __cplusplus >= 201103L
>> # define rseq_static_assert (expr, diagnostic) static_assert (expr,
>> diagnostic)
>> # define rseq_alignof alignof
>> # endif
>> #elif __STDC_VERSION__ >= 201112L
>> # define rseq_static_assert (expr, diagnostic) _Static_assert (expr,
>> diagnostic)
>> # define rseq_alignof _Alignof
>> #endif
>>
>> #ifndef rseq_static_assert
>> # define rseq_static_assert (expr, diagnostic) /* nothing */
>> #endif
>
> You can't have a space in #defines like that, no matter what GNU style
> says. 8-)
Yes, I noticed when failing to build this ;)
>
>> /* Ensure the compiler supports __attribute__ ((aligned)). */
>> rseq_static_assert ((rseq_alignof (struct rseq_cs) >= 32, "alignment"));
>> rseq_static_assert ((rseq_alignof (struct rseq) >= 32, "alignment"));
>
> You need to move the ; into rseq_static_assert. And if you use explicit
> arguments, you can't use double parentheses.
Why move the ";" into the macro ?
AFAIU, the only gain here would be to make sure we don't emit useless
";" in the "/* nothing */" case. But does it matter ?
Examples I can find of "static_assert" explicitly have the ";" at the
end, so I find it weird to integrate it into the rseq_static_assert
macro, which makes it different from static_assert.
Agreed on the need to remove the double-parentheses.
>
>>> And something similar for _Alignas/attribute aligned,
>>
>> I don't see where _Alignas is needed here ?
>>
>> For attribute aligned, what would be the oldest supported C and C++
>> standards ?
>
> There are no standardized attributes for C, there is only _Alignas.
> C++11 has an alignas specifier; it's not an attribute either. I think
> these are syntactically similar.
There appears to be an interesting difference between attribute aligned
and alignas. It seems like alignas cannot be used on a structure declaration,
only on fields, e.g.:
struct blah {
int a;
} _Alignas (16);
o.c:3:1: warning: useless ‘_Alignas’ in empty declaration
} _Alignas (16);
But
struct blah {
int _Alignas (16) a;
};
is OK. So if I change e.g. struct rseq_cs to align
the first field:
struct rseq_cs
{
/* Version of this structure. */
uint32_t rseq_align (32) version;
/* enum rseq_cs_flags. */
uint32_t flags;
uint64_t start_ip;
/* Offset from start_ip. */
uint64_t post_commit_offset;
uint64_t abort_ip;
};
It should work.
>
>>> with an error for
>>> older standards and !__GNUC__ compilers (because neither the type nor
>>> __thread can be represented there).
>>
>> By "type" you mean "struct rseq" here ? What does it contain that requires
>> a __GNUC__ compiler ?
>
> __attribute__ and __thread support.
OK
>
>> About __thread, I recall other compilers have other means to declare it.
>> In liburcu, I end up with the following:
>>
>> #if defined (__cplusplus) && (__cplusplus >= 201103L)
>> # define URCU_TLS_STORAGE_CLASS thread_local
>> #elif defined (__STDC_VERSION__) && (__STDC_VERSION__ >= 201112L)
>> # define URCU_TLS_STORAGE_CLASS _Thread_local
>> #elif defined (_MSC_VER)
>> # define URCU_TLS_STORAGE_CLASS __declspec(thread)
>> #else
>> # define URCU_TLS_STORAGE_CLASS __thread
>> #endif
>>
>> Would something along those lines be OK for libc ?
>
> Yes, it would be okay (minus the Visual C++ part). This part does not
> have to go into UAPI headers first. A fallback definition of __thread
> should be okay. Outside glibc, the TLS model declaration is optional, I
> think. The glibc *definition* ensures that the variable is
> initial-exec.
AFAIU you are technically correct when stating that the tls model
on the declaration is optional, but I think it's a good thing to have
it there rather than only at the definition. It makes it clear to all
users of this variable that its model is IE. Especially in scenarios where
early-adopter libraries and applications can define their own __rseq_abi
symbol, I think it's good to explicitly keep the IE tls model attribute in
the header.
I end up with the following:
#ifdef __cplusplus
# if __cplusplus >= 201103L
# define rseq_static_assert(expr, diagnostic) static_assert (expr, diagnostic)
# define rseq_alignof(type) alignof (type)
# define rseq_alignas(x) alignas (x)
# define rseq_tls_storage_class thread_local
# endif
#elif (defined __STDC_VERSION__ ? __STDC_VERSION__ : 0) >= 201112L
# define rseq_static_assert(expr, diagnostic) _Static_assert (expr, diagnostic)
# define rseq_alignof(type)_Alignof (type)
# define rseq_alignas(x) _Alignas (x)
# define rseq_tls_storage_class_Thread_local
#endif
#ifndef rseq_static_assert
/* Try to use _Static_assert macro from
Re: [PATCH glibc 1/3] glibc: Perform rseq registration at C startup and thread creation (v19)
* Mathieu Desnoyers: > Something like this ? > > #ifdef __cplusplus > # if __cplusplus >= 201103L > # define rseq_static_assert (expr, diagnostic) static_assert (expr, > diagnostic) > # define rseq_alignof alignof > # endif > #elif __STDC_VERSION__ >= 201112L > # define rseq_static_assert (expr, diagnostic) _Static_assert (expr, > diagnostic) > # define rseq_alignof _Alignof > #endif > > #ifndef rseq_static_assert > # define rseq_static_assert (expr, diagnostic) /* nothing */ > #endif You can't have a space in #defines like that, no matter what GNU style says. 8-) > /* Ensure the compiler supports __attribute__ ((aligned)). */ > rseq_static_assert ((rseq_alignof (struct rseq_cs) >= 32, "alignment")); > rseq_static_assert ((rseq_alignof (struct rseq) >= 32, "alignment")); You need to move the ; into rseq_static_assert. And if you use explicit arguments, you can't use double parentheses. >> And something similar for _Alignas/attribute aligned, > > I don't see where _Alignas is needed here ? > > For attribute aligned, what would be the oldest supported C and C++ > standards ? There are no standardized attributes for C, there is only _Alignas. C++11 has an alignas specifier; it's not an attribute either. I think these are syntactically similar. >> with an error for >> older standards and !__GNUC__ compilers (because neither the type nor >> __thread can be represented there). > > By "type" you mean "struct rseq" here ? What does it contain that requires > a __GNUC__ compiler ? __attribute__ and __thread support. > About __thread, I recall other compilers have other means to declare it. > In liburcu, I end up with the following: > > #if defined (__cplusplus) && (__cplusplus >= 201103L) > # define URCU_TLS_STORAGE_CLASS thread_local > #elif defined (__STDC_VERSION__) && (__STDC_VERSION__ >= 201112L) > # define URCU_TLS_STORAGE_CLASS _Thread_local > #elif defined (_MSC_VER) > # define URCU_TLS_STORAGE_CLASS __declspec(thread) > #else > # define URCU_TLS_STORAGE_CLASS __thread > #endif > > Would something along those lines be OK for libc ? Yes, it would be okay (minus the Visual C++ part). This part does not have to go into UAPI headers first. A fallback definition of __thread should be okay. Outside glibc, the TLS model declaration is optional, I think. The glibc *definition* ensures that the variable is initial-exec. Thanks, Florian
Re: [PATCH glibc 1/3] glibc: Perform rseq registration at C startup and thread creation (v19)
- On May 25, 2020, at 11:20 AM, Florian Weimer fwei...@redhat.com wrote: > * Mathieu Desnoyers: > >> The larger question here is: considering that we re-implement the entire >> uapi header within glibc (which includes the uptr addition), do we still >> care about using the header provided by the Linux kernel ? > > We don't care, but our users do. Eventually, they want to include > and to get new constants that are not yet > known to glibc. Good point! > >> Having different definitions depending on whether a kernel header is >> installed or not when including a glibc header seems rather unexpected. > > Indeed. > >> *If* we want to use the uapi header, I think something is semantically >> missing. Here is the scheme I envision. We could rely on the kernel header >> version.h to figure out which of glibc or kernel uapi header is more >> recent. Any new concept we try to integrate into glibc (e.g. uptr) >> should go into the upstream Linux uapi header first. > > I think we should always prefer the uapi header. The Linux version > check does not tell you anything about backports. Fair enough. > >> For the coming glibc e.g. 2.32, we use the kernel uapi header if >> kernel version is >= 4.18.0. Within glibc, the fallback implements >> exactly the API exposed by the kernel rseq.h header. > > Agreed. > >> As we eventually introduce the uptr change into the Linux kernel, and >> say it gets merged for Linux 5.9.0, we mirror this change into glibc >> (e.g. release 2.33), and bump the Linux kernel version cutoff to 5.9.0. >> So starting from that version, we use the Linux kernel header only if >> version >= 5.9.0, else we fallback on glibc's own implementation. > > Fortunately, we don't need to settle this today. 8-) > > Let's stick to the 4.18 definitions for the fallback for now, and > discuss the incorporation of future changes later. OK > +/* Ensure the compiler supports __attribute__ ((aligned)). */ +_Static_assert (__alignof__ (struct rseq_cs) >= 32, "alignment"); +_Static_assert (__alignof__ (struct rseq) >= 32, "alignment"); >>> >>> This needs #ifndef __cplusplus or something like that. I'm surprised >>> that this passes the installed header tests. >> >> Would the following be ok ? >> >> #ifdef __cplusplus >> #define rseq_static_assert static_assert >> #else >> #define rseq_static_assert _Static_assert >> #endif >> >> /* Ensure the compiler supports __attribute__ ((aligned)). */ >> rseq_static_assert (__alignof__ (struct rseq_cs) >= 32, "alignment"); >> rseq_static_assert (__alignof__ (struct rseq) >= 32, "alignment"); > > Seems reasonable, yes. __alignof__ is still a GCC extension. C++11 has > alignof, C11 has _Alignof. So you could use something like this > (perhaps without indentation for the kernel header version): > > #ifdef __cplusplus > # if __cplusplus >= 201103L > # define rseq_static_assert(x) static_assert x; > # define rseq_alignof alignof > # endif > #elif __STDC_VERSION__ >= 201112L > # define rseq_static_assert(x) _Static_assert x; > # define rseq_alignof _Alignof > #endif > #ifndef rseq_static_assert > # define rseq_static_assert /* nothing */ > #endif > rseq_static_assert ((rseq_alignof__ (struct rseq_cs) >= 32, "alignment")) > rseq_static_assert ((rseq_alignof (struct rseq) >= 32, "alignment")) Something like this ? #ifdef __cplusplus # if __cplusplus >= 201103L # define rseq_static_assert (expr, diagnostic) static_assert (expr, diagnostic) # define rseq_alignof alignof # endif #elif __STDC_VERSION__ >= 201112L # define rseq_static_assert (expr, diagnostic) _Static_assert (expr, diagnostic) # define rseq_alignof _Alignof #endif #ifndef rseq_static_assert # define rseq_static_assert (expr, diagnostic) /* nothing */ #endif /* Ensure the compiler supports __attribute__ ((aligned)). */ rseq_static_assert ((rseq_alignof (struct rseq_cs) >= 32, "alignment")); rseq_static_assert ((rseq_alignof (struct rseq) >= 32, "alignment")); > And something similar for _Alignas/attribute aligned, I don't see where _Alignas is needed here ? For attribute aligned, what would be the oldest supported C and C++ standards ? > with an error for > older standards and !__GNUC__ compilers (because neither the type nor > __thread can be represented there). By "type" you mean "struct rseq" here ? What does it contain that requires a __GNUC__ compiler ? About __thread, I recall other compilers have other means to declare it. In liburcu, I end up with the following: #if defined (__cplusplus) && (__cplusplus >= 201103L) # define URCU_TLS_STORAGE_CLASS thread_local #elif defined (__STDC_VERSION__) && (__STDC_VERSION__ >= 201112L) # define URCU_TLS_STORAGE_CLASS _Thread_local #elif defined (_MSC_VER) # define URCU_TLS_STORAGE_CLASS __declspec(thread) #else # define URCU_TLS_STORAGE_CLASS __thread #endif Would something along those lines be OK for
Re: [PATCH glibc 1/3] glibc: Perform rseq registration at C startup and thread creation (v19)
* Mathieu Desnoyers: > The larger question here is: considering that we re-implement the entire > uapi header within glibc (which includes the uptr addition), do we still > care about using the header provided by the Linux kernel ? We don't care, but our users do. Eventually, they want to include and to get new constants that are not yet known to glibc. > Having different definitions depending on whether a kernel header is > installed or not when including a glibc header seems rather unexpected. Indeed. > *If* we want to use the uapi header, I think something is semantically > missing. Here is the scheme I envision. We could rely on the kernel header > version.h to figure out which of glibc or kernel uapi header is more > recent. Any new concept we try to integrate into glibc (e.g. uptr) > should go into the upstream Linux uapi header first. I think we should always prefer the uapi header. The Linux version check does not tell you anything about backports. > For the coming glibc e.g. 2.32, we use the kernel uapi header if > kernel version is >= 4.18.0. Within glibc, the fallback implements > exactly the API exposed by the kernel rseq.h header. Agreed. > As we eventually introduce the uptr change into the Linux kernel, and > say it gets merged for Linux 5.9.0, we mirror this change into glibc > (e.g. release 2.33), and bump the Linux kernel version cutoff to 5.9.0. > So starting from that version, we use the Linux kernel header only if > version >= 5.9.0, else we fallback on glibc's own implementation. Fortunately, we don't need to settle this today. 8-) Let's stick to the 4.18 definitions for the fallback for now, and discuss the incorporation of future changes later. >>> +/* Ensure the compiler supports __attribute__ ((aligned)). */ >>> +_Static_assert (__alignof__ (struct rseq_cs) >= 32, "alignment"); >>> +_Static_assert (__alignof__ (struct rseq) >= 32, "alignment"); >> >> This needs #ifndef __cplusplus or something like that. I'm surprised >> that this passes the installed header tests. > > Would the following be ok ? > > #ifdef __cplusplus > #define rseq_static_assert static_assert > #else > #define rseq_static_assert _Static_assert > #endif > > /* Ensure the compiler supports __attribute__ ((aligned)). */ > rseq_static_assert (__alignof__ (struct rseq_cs) >= 32, "alignment"); > rseq_static_assert (__alignof__ (struct rseq) >= 32, "alignment"); Seems reasonable, yes. __alignof__ is still a GCC extension. C++11 has alignof, C11 has _Alignof. So you could use something like this (perhaps without indentation for the kernel header version): #ifdef __cplusplus # if __cplusplus >= 201103L # define rseq_static_assert(x) static_assert x; # define rseq_alignof alignof # endif #elif __STDC_VERSION__ >= 201112L # define rseq_static_assert(x) _Static_assert x; # define rseq_alignof _Alignof #endif #ifndef rseq_static_assert # define rseq_static_assert /* nothing */ #endif rseq_static_assert ((rseq_alignof__ (struct rseq_cs) >= 32, "alignment")) rseq_static_assert ((rseq_alignof (struct rseq) >= 32, "alignment")) And something similar for _Alignas/attribute aligned, with an error for older standards and !__GNUC__ compilers (because neither the type nor __thread can be represented there). Thanks, Florian
Re: [PATCH glibc 1/3] glibc: Perform rseq registration at C startup and thread creation (v19)
- On May 20, 2020, at 7:40 AM, Florian Weimer fwei...@redhat.com wrote:
> * Mathieu Desnoyers via Libc-alpha:
>
>> diff --git a/NEWS b/NEWS
>> index 141078c319..c4e0370fc4 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -23,6 +23,16 @@ Major new features:
>>toolchains. It is recommended to use GCC 8 or newer when testing
>>this option.
>>
>> +* Support for automatically registering threads with the Linux rseq
>> + system call has been added. This system call is implemented starting
>> + from Linux 4.18. The Restartable Sequences ABI accelerates user-space
>> + operations on per-cpu data. It allows user-space to perform updates
>> + on per-cpu data without requiring heavy-weight atomic operations.
>> + Automatically registering threads allows all libraries, including libc,
>> + to make immediate use of the rseq(2) support by using the documented ABI.
>> + The GNU C Library manual has details on integration of Restartable
>> + Sequences.
>
> “rseq” instead “rseq(2)”.
OK
[...]
>
>> diff --git a/manual/threads.texi b/manual/threads.texi
>> index 0858ef8f92..a565095c43 100644
>> --- a/manual/threads.texi
>> +++ b/manual/threads.texi
>> @@ -9,8 +9,10 @@ This chapter describes functions used for managing threads.
>> POSIX threads.
>>
>> @menu
>> -* ISO C Threads:: Threads based on the ISO C specification.
>> -* POSIX Threads:: Threads based on the POSIX specification.
>> +* ISO C Threads:: Threads based on the ISO C specification.
>> +* POSIX Threads:: Threads based on the POSIX specification.
>> +* Restartable Sequences:: Linux-specific Restartable Sequences
>> +integration.
>> @end menu
>
> This should go into the extensions menu (@node Non-POSIX Extensions).
OK
>
> General comment: Please wrap the lines around 72 or so characters.
> Thanks.
OK
>
>> @@ -881,3 +883,63 @@ Behaves like @code{pthread_timedjoin_np} except that the
>> absolute time in
>> @c pthread_spin_unlock
>> @c pthread_testcancel
>> @c pthread_yield
>> +
>> +@node Restartable Sequences
>> +@section Restartable Sequences
>> +@cindex Restartable Sequences
>> +
>> +This section describes Restartable Sequences integration for
>> +@theglibc{}.
>
> “This functionality is only available on Linux.” (The @standards parts
> are not visible to readers.)
OK
>
>> +
>> +@deftypevar {struct rseq} __rseq_abi
>> +@standards{Linux, sys/rseq.h}
>> +@Theglibc{} implements a @code{__rseq_abi} TLS symbol to interact with the
>> +Restartable Sequences system call (Linux-specific). The layout of this
>> +structure is defined by the @file{sys/rseq.h} header. Registration of each
>> +thread's @code{__rseq_abi} is performed by @theglibc{} at library
>> +initialization and thread creation.
>
> Can drop “(Linux-specific)” here.
OK
>
>> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/rseq.h
>> b/sysdeps/unix/sysv/linux/aarch64/bits/rseq.h
>> new file mode 100644
>> index 00..37d83fcb4a
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/rseq.h
>
>> +#ifdef __AARCH64EB__
>> +#define RSEQ_SIG_DATA 0x00bc28d4 /* BRK #0x45E0. */
>> +#else
>> +#define RSEQ_SIG_DATA RSEQ_SIG_CODE
>> +#endif
>
> Missing indentation on the #defines (sorry!).
Sorry! My bad!
>
>> diff --git a/sysdeps/unix/sysv/linux/arm/bits/rseq.h
>> b/sysdeps/unix/sysv/linux/arm/bits/rseq.h
>> new file mode 100644
>> index 00..c132f0327c
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/arm/bits/rseq.h
>
>> +#ifdef __ARMEB__
>> +#define RSEQ_SIG0xf3def5e7 /* udf#24035; 0x5de3 (ARMv6+) */
>> +#else
>> +#define RSEQ_SIG0xe7f5def3 /* udf#24035; 0x5de3 */
>> +#endif
>
> Likewise, missing indentation.
At least I was consistently wrong. ;-)
>
>> diff --git a/sysdeps/unix/sysv/linux/bits/rseq.h
>> b/sysdeps/unix/sysv/linux/bits/rseq.h
>> new file mode 100644
>> index 00..014c08fe0f
>> --- /dev/null
>> +++ b/sysdeps/unix/sysv/linux/bits/rseq.h
>> @@ -0,0 +1,29 @@
>
>> +/* RSEQ_SIG is a signature required before each abort handler code.
>> +
>> + It is a 32-bit value that maps to actual architecture code compiled
>> + into applications and libraries. It needs to be defined for each
>> + architecture. When choosing this value, it needs to be taken into
>> + account that generating invalid instructions may have ill effects on
>> + tools like objdump, and may also have impact on the CPU speculative
>> + execution efficiency in some cases. */
>
> I wonder if we should say something somewhere that the correct way to
> check for compile-time rseq support in glibc is something like this?
>
> #if __has_include ()
> # include
> #endif
> #ifdef RSEQ_SIG
> …
> #endif
>
> Or perhaps we should suppress installation of the headers if we only
> have support for the stub.
>
> (I think this fine tuning can be deferred to later patch.)
I would prefer we keep this for a later patch.
That being said, I notice that gcc
Re: [PATCH glibc 1/3] glibc: Perform rseq registration at C startup and thread creation (v19)
* Mathieu Desnoyers via Libc-alpha:
> diff --git a/NEWS b/NEWS
> index 141078c319..c4e0370fc4 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -23,6 +23,16 @@ Major new features:
>toolchains. It is recommended to use GCC 8 or newer when testing
>this option.
>
> +* Support for automatically registering threads with the Linux rseq
> + system call has been added. This system call is implemented starting
> + from Linux 4.18. The Restartable Sequences ABI accelerates user-space
> + operations on per-cpu data. It allows user-space to perform updates
> + on per-cpu data without requiring heavy-weight atomic operations.
> + Automatically registering threads allows all libraries, including libc,
> + to make immediate use of the rseq(2) support by using the documented ABI.
> + The GNU C Library manual has details on integration of Restartable
> + Sequences.
“rseq” instead “rseq(2)”.
> diff --git a/elf/libc_early_init.c b/elf/libc_early_init.c
> index e6c64fb526..f0fcf6448e 100644
> --- a/elf/libc_early_init.c
> +++ b/elf/libc_early_init.c
> @@ -18,10 +18,14 @@
>
> #include
> #include
> +#include
>
> void
> __libc_early_init (_Bool initial)
> {
>/* Initialize ctype data. */
>__ctype_init ();
> + /* Register rseq ABI to the kernel for the main program's libc. */
> + if (initial)
> +rseq_register_current_thread ();
> }
Okay.
> diff --git a/manual/threads.texi b/manual/threads.texi
> index 0858ef8f92..a565095c43 100644
> --- a/manual/threads.texi
> +++ b/manual/threads.texi
> @@ -9,8 +9,10 @@ This chapter describes functions used for managing threads.
> POSIX threads.
>
> @menu
> -* ISO C Threads::Threads based on the ISO C specification.
> -* POSIX Threads::Threads based on the POSIX specification.
> +* ISO C Threads::Threads based on the ISO C specification.
> +* POSIX Threads::Threads based on the POSIX specification.
> +* Restartable Sequences::Linux-specific Restartable Sequences
> + integration.
> @end menu
This should go into the extensions menu (@node Non-POSIX Extensions).
General comment: Please wrap the lines around 72 or so characters.
Thanks.
> @@ -881,3 +883,63 @@ Behaves like @code{pthread_timedjoin_np} except that the
> absolute time in
> @c pthread_spin_unlock
> @c pthread_testcancel
> @c pthread_yield
> +
> +@node Restartable Sequences
> +@section Restartable Sequences
> +@cindex Restartable Sequences
> +
> +This section describes Restartable Sequences integration for
> +@theglibc{}.
“This functionality is only available on Linux.” (The @standards parts
are not visible to readers.)
> +
> +@deftypevar {struct rseq} __rseq_abi
> +@standards{Linux, sys/rseq.h}
> +@Theglibc{} implements a @code{__rseq_abi} TLS symbol to interact with the
> +Restartable Sequences system call (Linux-specific). The layout of this
> +structure is defined by the @file{sys/rseq.h} header. Registration of each
> +thread's @code{__rseq_abi} is performed by @theglibc{} at library
> +initialization and thread creation.
Can drop “(Linux-specific)” here.
> diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/rseq.h
> b/sysdeps/unix/sysv/linux/aarch64/bits/rseq.h
> new file mode 100644
> index 00..37d83fcb4a
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/aarch64/bits/rseq.h
> +#ifdef __AARCH64EB__
> +#define RSEQ_SIG_DATA0x00bc28d4 /* BRK #0x45E0. */
> +#else
> +#define RSEQ_SIG_DATARSEQ_SIG_CODE
> +#endif
Missing indentation on the #defines (sorry!).
> diff --git a/sysdeps/unix/sysv/linux/arm/bits/rseq.h
> b/sysdeps/unix/sysv/linux/arm/bits/rseq.h
> new file mode 100644
> index 00..c132f0327c
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/arm/bits/rseq.h
> +#ifdef __ARMEB__
> +#define RSEQ_SIG0xf3def5e7 /* udf#24035; 0x5de3 (ARMv6+) */
> +#else
> +#define RSEQ_SIG0xe7f5def3 /* udf#24035; 0x5de3 */
> +#endif
Likewise, missing indentation.
> diff --git a/sysdeps/unix/sysv/linux/bits/rseq.h
> b/sysdeps/unix/sysv/linux/bits/rseq.h
> new file mode 100644
> index 00..014c08fe0f
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/bits/rseq.h
> @@ -0,0 +1,29 @@
> +/* RSEQ_SIG is a signature required before each abort handler code.
> +
> + It is a 32-bit value that maps to actual architecture code compiled
> + into applications and libraries. It needs to be defined for each
> + architecture. When choosing this value, it needs to be taken into
> + account that generating invalid instructions may have ill effects on
> + tools like objdump, and may also have impact on the CPU speculative
> + execution efficiency in some cases. */
I wonder if we should say something somewhere that the correct way to
check for compile-time rseq support in glibc is something like this?
#if __has_include ()
# include
#endif
#ifdef RSEQ_SIG
…
#endif
Or perhaps we should suppress installation of the headers if we only
have support for the
[PATCH glibc 1/3] glibc: Perform rseq registration at C startup and thread creation (v19)
Register rseq TLS for each thread (including main), and unregister for each thread (excluding main). "rseq" stands for Restartable Sequences. See the rseq(2) man page proposed here: https://lkml.org/lkml/2018/9/19/647 Those are based on glibc master branch commit 6f0baacf0f89. The rseq system call was merged into Linux 4.18. CC: Carlos O'Donell CC: Florian Weimer CC: Joseph Myers CC: Szabolcs Nagy CC: Thomas Gleixner CC: Ben Maurer CC: Peter Zijlstra CC: "Paul E. McKenney" CC: Boqun Feng CC: Will Deacon CC: Dave Watson CC: Paul Turner CC: Rich Felker CC: libc-al...@sourceware.org CC: linux-kernel@vger.kernel.org CC: linux-...@vger.kernel.org --- Changes since v1: - Move __rseq_refcount to an extra field at the end of __rseq_abi to eliminate one symbol. All libraries/programs which try to register rseq (glibc, early-adopter applications, early-adopter libraries) should use the rseq refcount. It becomes part of the ABI within a user-space process, but it's not part of the ABI shared with the kernel per se. - Restructure how this code is organized so glibc keeps building on non-Linux targets. - Use non-weak symbol for __rseq_abi. - Move rseq registration/unregistration implementation into its own nptl/rseq.c compile unit. - Move __rseq_abi symbol under GLIBC_2.29. Changes since v2: - Move __rseq_refcount to its own symbol, which is less ugly than trying to play tricks with the rseq uapi. - Move __rseq_abi from nptl to csu (C start up), so it can be used across glibc, including memory allocator and sched_getcpu(). The __rseq_refcount symbol is kept in nptl, because there is no reason to use it elsewhere in glibc. Changes since v3: - Set __rseq_refcount TLS to 1 on register/set to 0 on unregister because glibc is the first/last user. - Unconditionally register/unregister rseq at thread start/exit, because glibc is the first/last user. - Add missing abilist items. - Rebase on glibc master commit a502c5294. - Add NEWS entry. Changes since v4: - Do not use "weak" symbols for __rseq_abi and __rseq_refcount. Based on "System V Application Binary Interface", weak only affects the link editor, not the dynamic linker. - Install a new sys/rseq.h system header on Linux, which contains the RSEQ_SIG definition, __rseq_abi declaration and __rseq_refcount declaration. Move those definition/declarations from rseq-internal.h to the installed sys/rseq.h header. - Considering that rseq is only available on Linux, move csu/rseq.c to sysdeps/unix/sysv/linux/rseq-sym.c. - Move __rseq_refcount from nptl/rseq.c to sysdeps/unix/sysv/linux/rseq-sym.c, so it is only defined on Linux. - Move both ABI definitions for __rseq_abi and __rseq_refcount to sysdeps/unix/sysv/linux/Versions, so they only appear on Linux. - Document __rseq_abi and __rseq_refcount volatile. - Document the RSEQ_SIG signature define. - Move registration functions from rseq.c to rseq-internal.h static inline functions. Introduce empty stubs in misc/rseq-internal.h, which can be overridden by architecture code in sysdeps/unix/sysv/linux/rseq-internal.h. - Rename __rseq_register_current_thread and __rseq_unregister_current_thread to rseq_register_current_thread and rseq_unregister_current_thread, now that those are only visible as internal static inline functions. - Invoke rseq_register_current_thread() from libc-start.c LIBC_START_MAIN rather than nptl init, so applications not linked against libpthread.so have rseq registered for their main() thread. Note that it is invoked separately for SHARED and !SHARED builds. Changes since v5: - Replace __rseq_refcount by __rseq_lib_abi, which contains two uint32_t: register_state and refcount. The "register_state" field allows inhibiting rseq registration from signal handlers nested on top of glibc registration and occuring after rseq unregistration by glibc. - Introduce enum rseq_register_state, which contains the states allowed for the struct rseq_lib_abi register_state field. Changes since v6: - Introduce bits/rseq.h to define RSEQ_SIG for each architecture. The generic bits/rseq.h does not define RSEQ_SIG, meaning that each architecture implementing rseq needs to implement bits/rseq.h. - Rename enum item RSEQ_REGISTER_NESTED to RSEQ_REGISTER_ONGOING. - Port to glibc-2.29. Changes since v7: - Remove __rseq_lib_abi symbol, including refcount and register_state fields. - Remove reference counting and nested signals handling from registration/unregistration functions. - Introduce new __rseq_handled exported symbol, which is set to 1 by glibc on C startup when it handles restartable sequences. This allows glibc to coexist with early adopter libraries and applications wishing to register restartable sequences when it is not handled by glibc. - Introduce rseq_init (), which sets __rseq_handled to 1 from C startup. - Update NEWS entry. - Update comments at the beginning of new files. - Registration depends on
