LoongArch: Modify some member names in mcontext_t and ucontext_t structs to align them with the kernel.
Checks
| Context |
Check |
Description |
| dj/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
| dj/TryBot-32bit |
success
|
Build for i686
|
Commit Message
During the construction of the LoongArch Alpine system,
we found that there is an inconsistency in the member
names of mcontext_t and ucontext_t between musl and glibc,
which can cause compilation errors. After testing, we have
decided to modify some member names.
This patch will be backported to glibc versions 2.36 and 2.37.
Reference: 4fa9b3bfe6759c82beb4b043a54a3598ca467289
---
.../unix/sysv/linux/loongarch/makecontext.c | 26 +++++++++----------
.../sysv/linux/loongarch/sigcontextinfo.h | 2 +-
.../unix/sysv/linux/loongarch/sys/ucontext.h | 17 ++++++++----
.../unix/sysv/linux/loongarch/ucontext_i.sym | 6 ++---
4 files changed, 29 insertions(+), 22 deletions(-)
Comments
On Fri, 2023-03-24 at 14:25 +0800, caiyinyu wrote:
> During the construction of the LoongArch Alpine system,
> we found that there is an inconsistency in the member
> names of mcontext_t and ucontext_t between musl and glibc,
> which can cause compilation errors. After testing, we have
> decided to modify some member names.
>
> This patch will be backported to glibc versions 2.36 and 2.37.
Oh don't do that. This will be a public API break.
I'm not sure if it's allowed to break the public API in Glibc, but at
least it's strictly prohibited for a backport.
On Fri, 2023-03-24 at 14:25 +0800, caiyinyu wrote:
> typedef struct mcontext_t
> {
> - unsigned long long __pc;
> - unsigned long long __gregs[32];
> - unsigned int __flags;
> - unsigned long long __extcontext[0] __attribute__((__aligned__(16)));
> + unsigned long long __ctx(sc_pc);
> + unsigned long long __ctx(sc_regs)[32];
> + unsigned int __ctx(sc_flags);
> + unsigned long long __ctx(sc_extcontext)[0] __attribute__((__aligned__(16)));
> } mcontext_t;
How about this?
__extension__ typedef struct mcontext_t
{
union
{
/* Backward compatibility names. */
struct
{
unsigned long long __pc;
unsigned long long __gregs[32];
unsigned int __flags;
unsigned long long __extcontext[0] __attribute__((__aligned__(16)));
};
struct
{
unsigned long long __ctx(sc_pc);
unsigned long long __ctx(sc_regs)[32];
unsigned int __ctx(sc_flags);
unsigned long long __ctx(sc_extcontext)[0] __attribute__((__aligned__(16)));
};
};
} mcontext_t;
在 2023/3/24 下午3:40, Xi Ruoyao 写道:
> On Fri, 2023-03-24 at 14:25 +0800, caiyinyu wrote:
>> typedef struct mcontext_t
>> {
>> - unsigned long long __pc;
>> - unsigned long long __gregs[32];
>> - unsigned int __flags;
>> - unsigned long long __extcontext[0] __attribute__((__aligned__(16)));
>> + unsigned long long __ctx(sc_pc);
>> + unsigned long long __ctx(sc_regs)[32];
>> + unsigned int __ctx(sc_flags);
>> + unsigned long long __ctx(sc_extcontext)[0] __attribute__((__aligned__(16)));
>> } mcontext_t;
> How about this?
>
> __extension__ typedef struct mcontext_t
> {
> union
> {
> /* Backward compatibility names. */
> struct
> {
> unsigned long long __pc;
> unsigned long long __gregs[32];
> unsigned int __flags;
> unsigned long long __extcontext[0] __attribute__((__aligned__(16)));
> };
> struct
> {
> unsigned long long __ctx(sc_pc);
> unsigned long long __ctx(sc_regs)[32];
> unsigned int __ctx(sc_flags);
> unsigned long long __ctx(sc_extcontext)[0] __attribute__((__aligned__(16)));
> };
> };
> } mcontext_t;
Your plan caused fails in glibc conform tests.
We wanted to inform you that we are aware that the proposed
modification[1] will affect the public API. However, since LoongArch
Musl has not yet been released, and the LoongArch Euler system, Debian
system, Alpine, and other systems are still in early development stages,
we have not yet widely released the system. Therefore, we believe it is
necessary to proceed with the modification.
We do not want to carry the burden of history and remain committed to
proceeding with the modification. Additionally, we also plan to backport
this patch[1] to versions 2.36 and 2.37 to address the issue of
inconsistent naming of musl/glibc/kernel sigcontext/mcontext_t members
from the source, resolving this issue at the root. This will eliminate
potential problems for future software and system development.
[1] patch:
https://sourceware.org/pipermail/libc-alpha/2023-March/146607.html
Gentle ping.
The previous patch was missing a line of code: #undef __ctx. Here's the
updated patch:
https://sourceware.org/pipermail/libc-alpha/2023-April/146859.html
在 2023/3/31 上午11:52, caiyinyu 写道:
>
> 在 2023/3/24 下午3:40, Xi Ruoyao 写道:
>> On Fri, 2023-03-24 at 14:25 +0800, caiyinyu wrote:
>>> typedef struct mcontext_t
>>> {
>>> - unsigned long long __pc;
>>> - unsigned long long __gregs[32];
>>> - unsigned int __flags;
>>> - unsigned long long __extcontext[0] __attribute__((__aligned__(16)));
>>> + unsigned long long __ctx(sc_pc);
>>> + unsigned long long __ctx(sc_regs)[32];
>>> + unsigned int __ctx(sc_flags);
>>> + unsigned long long __ctx(sc_extcontext)[0]
>>> __attribute__((__aligned__(16)));
>>> } mcontext_t;
>> How about this?
>>
>> __extension__ typedef struct mcontext_t
>> {
>> union
>> {
>> /* Backward compatibility names. */
>> struct
>> {
>> unsigned long long __pc;
>> unsigned long long __gregs[32];
>> unsigned int __flags;
>> unsigned long long __extcontext[0] __attribute__((__aligned__(16)));
>> };
>> struct
>> {
>> unsigned long long __ctx(sc_pc);
>> unsigned long long __ctx(sc_regs)[32];
>> unsigned int __ctx(sc_flags);
>> unsigned long long __ctx(sc_extcontext)[0]
>> __attribute__((__aligned__(16)));
>> };
>> };
>> } mcontext_t;
>
> Your plan caused fails in glibc conform tests.
>
> We wanted to inform you that we are aware that the proposed
> modification[1] will affect the public API. However, since LoongArch
> Musl has not yet been released, and the LoongArch Euler system, Debian
> system, Alpine, and other systems are still in early development
> stages, we have not yet widely released the system. Therefore, we
> believe it is necessary to proceed with the modification.
>
> We do not want to carry the burden of history and remain committed to
> proceeding with the modification. Additionally, we also plan to
> backport this patch[1] to versions 2.36 and 2.37 to address the issue
> of inconsistent naming of musl/glibc/kernel sigcontext/mcontext_t
> members from the source, resolving this issue at the root. This will
> eliminate potential problems for future software and system development.
>
> [1] patch:
>
> https://sourceware.org/pipermail/libc-alpha/2023-March/146607.html
>
On Fri, 2023-03-31 at 11:52 +0800, caiyinyu wrote:
/* snip */
> Your plan caused fails in glibc conform tests.
Let me try something...
> We wanted to inform you that we are aware that the proposed
> modification[1] will affect the public API. However, since LoongArch
> Musl has not yet been released
Then why not change the Musl instead? It's not released so you won't
break existing code.
在 2023/4/3 下午4:56, Xi Ruoyao 写道:
> On Fri, 2023-03-31 at 11:52 +0800, caiyinyu wrote:
>
> /* snip */
>
>> Your plan caused fails in glibc conform tests.
> Let me try something...
>
>> We wanted to inform you that we are aware that the proposed
>> modification[1] will affect the public API. However, since LoongArch
>> Musl has not yet been released
> Then why not change the Musl instead? It's not released so you won't
> break existing code.
This is also a viable solution, but we want to take advantage of the
early development
stage to standardize the member names of mcontext_t/sigcontext in musl
and glibc2.36/37...
to match those in the kernel in order to get rid of historical baggage,
as mentioned in the previous email.
>
On Mon, 2023-04-03 at 17:18 +0800, caiyinyu wrote:
>
> 在 2023/4/3 下午4:56, Xi Ruoyao 写道:
> > On Fri, 2023-03-31 at 11:52 +0800, caiyinyu wrote:
> >
> > /* snip */
> >
> > > Your plan caused fails in glibc conform tests.
> > Let me try something...
> >
> > > We wanted to inform you that we are aware that the proposed
> > > modification[1] will affect the public API. However, since
> > > LoongArch
> > > Musl has not yet been released
> > Then why not change the Musl instead? It's not released so you
> > won't
> > break existing code.
>
> This is also a viable solution, but we want to take advantage of the
> early development
>
> stage to standardize the member names of mcontext_t/sigcontext in musl
> and glibc2.36/37...
>
> to match those in the kernel in order to get rid of historical
> baggage,
>
> as mentioned in the previous email.
Give me several hours trying to make the anonymous union work. I see
similar things in other ports' mcontext_t definition so I guess I can
make it work...
And please don't change Glibc 2.36/37. The reason is:
If some downstream work decides get rid of historical things, the
maintainer can just say "Glibc < 2.38 is not supported".
Or they can use #if __GLIBC_PREREQ (2, 38) to support both releases if
they want to support both old and new releases.
But by backporting the change into 2.36 or 37, there will be no trivial
way to support both "old" definition and "new" definition because there
is no #ifdef can distinguish "patched" 2.37 and "unpatched" 2.37. And
you cannot tell distro maintainers to update there Glibc headers in
/usr/include because doing so is *very* dangerous.
>
在 2023/4/3 下午5:25, Xi Ruoyao 写道:
> On Mon, 2023-04-03 at 17:18 +0800, caiyinyu wrote:
>> 在 2023/4/3 下午4:56, Xi Ruoyao 写道:
>>> On Fri, 2023-03-31 at 11:52 +0800, caiyinyu wrote:
>>>
>>> /* snip */
>>>
>>>> Your plan caused fails in glibc conform tests.
>>> Let me try something...
>>>
>>>> We wanted to inform you that we are aware that the proposed
>>>> modification[1] will affect the public API. However, since
>>>> LoongArch
>>>> Musl has not yet been released
>>> Then why not change the Musl instead? It's not released so you
>>> won't
>>> break existing code.
>> This is also a viable solution, but we want to take advantage of the
>> early development
>>
>> stage to standardize the member names of mcontext_t/sigcontext in musl
>> and glibc2.36/37...
>>
>> to match those in the kernel in order to get rid of historical
>> baggage,
>>
>> as mentioned in the previous email.
> Give me several hours trying to make the anonymous union work. I see
> similar things in other ports' mcontext_t definition so I guess I can
> make it work...
>
> And please don't change Glibc 2.36/37. The reason is:
>
> If some downstream work decides get rid of historical things, the
> maintainer can just say "Glibc < 2.38 is not supported".
>
> Or they can use #if __GLIBC_PREREQ (2, 38) to support both releases if
> they want to support both old and new releases.
>
> But by backporting the change into 2.36 or 37, there will be no trivial
> way to support both "old" definition and "new" definition because there
> is no #ifdef can distinguish "patched" 2.37 and "unpatched" 2.37. And
> you cannot tell distro maintainers to update there Glibc headers in
> /usr/include because doing so is *very* dangerous.
OK, thanks a lot.
On Mon, 2023-04-03 at 17:41 +0800, caiyinyu wrote:
> > Give me several hours trying to make the anonymous union work. I see
> > similar things in other ports' mcontext_t definition so I guess I can
> > make it work...
I tried this (w/o __extension__: anyway we are already using "zero-
length array" which is an extension, and -pedantic should not send alarm
for system headers):
typedef struct mcontext_t
{
union
{
struct
{
unsigned long long __ctx(sc_pc);
unsigned long long __ctx(sc_regs)[32];
unsigned int __ctx(sc_flags);
unsigned long long __ctx(sc_extcontext)[0] __attribute__((__aligned__(16)));
};
struct
{
unsigned long long __pc;
unsigned long long __gregs[32];
unsigned int __flags;
unsigned long long __extcontext[0] __attribute__((__aligned__(16)));
};
};
} mcontext_t;
There is no conformance test failing. Can you try this? If there is
still conformance test failing please send me the error message so I can
know why it fails.
在 2023/4/3 下午6:12, Xi Ruoyao 写道:
> On Mon, 2023-04-03 at 17:41 +0800, caiyinyu wrote:
>
>>> Give me several hours trying to make the anonymous union work. I see
>>> similar things in other ports' mcontext_t definition so I guess I can
>>> make it work...
> I tried this (w/o __extension__: anyway we are already using "zero-
> length array" which is an extension, and -pedantic should not send alarm
> for system headers):
>
> typedef struct mcontext_t
> {
> union
> {
> struct
> {
> unsigned long long __ctx(sc_pc);
> unsigned long long __ctx(sc_regs)[32];
> unsigned int __ctx(sc_flags);
> unsigned long long __ctx(sc_extcontext)[0] __attribute__((__aligned__(16)));
> };
> struct
> {
> unsigned long long __pc;
> unsigned long long __gregs[32];
> unsigned int __flags;
> unsigned long long __extcontext[0] __attribute__((__aligned__(16)));
> };
> };
> } mcontext_t;
>
> There is no conformance test failing. Can you try this? If there is
> still conformance test failing please send me the error message so I can
> know why it fails.
Results: all conform tests passed.
XPASS: conform/UNIX98/ndbm.h/linknamespace
XPASS: conform/XOPEN2K/ndbm.h/linknamespace
XPASS: conform/XOPEN2K8/ndbm.h/linknamespace
XPASS: conform/XPG42/ndbm.h/linknamespace
Summary of test results:
1049 PASS
10 XFAIL
4 XPASS
My bad. I stoped at testing your plan without the "__extension__".
>
在 2023/4/3 下午6:45, caiyinyu 写道:
>
> 在 2023/4/3 下午6:12, Xi Ruoyao 写道:
>> On Mon, 2023-04-03 at 17:41 +0800, caiyinyu wrote:
>>
>>>> Give me several hours trying to make the anonymous union work. I see
>>>> similar things in other ports' mcontext_t definition so I guess I can
>>>> make it work...
>> I tried this (w/o __extension__: anyway we are already using "zero-
>> length array" which is an extension, and -pedantic should not send alarm
>> for system headers):
>>
>> typedef struct mcontext_t
>> {
>> union
>> {
>> struct
>> {
>> unsigned long long __ctx(sc_pc);
>> unsigned long long __ctx(sc_regs)[32];
>> unsigned int __ctx(sc_flags);
>> unsigned long long __ctx(sc_extcontext)[0]
>> __attribute__((__aligned__(16)));
>> };
>> struct
>> {
>> unsigned long long __pc;
>> unsigned long long __gregs[32];
>> unsigned int __flags;
>> unsigned long long __extcontext[0]
>> __attribute__((__aligned__(16)));
>> };
>> };
>> } mcontext_t;
>>
>> There is no conformance test failing. Can you try this? If there is
>> still conformance test failing please send me the error message so I can
>> know why it fails.
>
> Results: all conform tests passed.
>
> XPASS: conform/UNIX98/ndbm.h/linknamespace
> XPASS: conform/XOPEN2K/ndbm.h/linknamespace
> XPASS: conform/XOPEN2K8/ndbm.h/linknamespace
> XPASS: conform/XPG42/ndbm.h/linknamespace
> Summary of test results:
> 1049 PASS
> 10 XFAIL
> 4 XPASS
>
> My bad. I stoped at testing your plan without the "__extension__".
>
>
New patch:
https://sourceware.org/pipermail/libc-alpha/2023-April/146897.html
>
>>
@@ -41,19 +41,19 @@ __makecontext (ucontext_t *ucp, void (*func) (void), int argc, long int a0,
ra = s0 = 0, terminating the stack for backtracing purposes.
s1 = the function we must call.
s2 = the subsequent context to run. */
- ucp->uc_mcontext.__gregs[LARCH_REG_RA] = (uintptr_t) 0;
- ucp->uc_mcontext.__gregs[LARCH_REG_S0] = (uintptr_t) 0;
- ucp->uc_mcontext.__gregs[LARCH_REG_S1] = (uintptr_t) func;
- ucp->uc_mcontext.__gregs[LARCH_REG_S2] = (uintptr_t) ucp->uc_link;
- ucp->uc_mcontext.__gregs[LARCH_REG_SP] = (uintptr_t) sp;
- ucp->uc_mcontext.__pc = (uintptr_t) &__start_context;
+ ucp->uc_mcontext.sc_regs[LARCH_REG_RA] = (uintptr_t) 0;
+ ucp->uc_mcontext.sc_regs[LARCH_REG_S0] = (uintptr_t) 0;
+ ucp->uc_mcontext.sc_regs[LARCH_REG_S1] = (uintptr_t) func;
+ ucp->uc_mcontext.sc_regs[LARCH_REG_S2] = (uintptr_t) ucp->uc_link;
+ ucp->uc_mcontext.sc_regs[LARCH_REG_SP] = (uintptr_t) sp;
+ ucp->uc_mcontext.sc_pc = (uintptr_t) &__start_context;
/* Put args in a0-a7, then put any remaining args on the stack. */
- ucp->uc_mcontext.__gregs[LARCH_REG_A0 + 0] = (uintptr_t) a0;
- ucp->uc_mcontext.__gregs[LARCH_REG_A0 + 1] = (uintptr_t) a1;
- ucp->uc_mcontext.__gregs[LARCH_REG_A0 + 2] = (uintptr_t) a2;
- ucp->uc_mcontext.__gregs[LARCH_REG_A0 + 3] = (uintptr_t) a3;
- ucp->uc_mcontext.__gregs[LARCH_REG_A0 + 4] = (uintptr_t) a4;
+ ucp->uc_mcontext.sc_regs[LARCH_REG_A0 + 0] = (uintptr_t) a0;
+ ucp->uc_mcontext.sc_regs[LARCH_REG_A0 + 1] = (uintptr_t) a1;
+ ucp->uc_mcontext.sc_regs[LARCH_REG_A0 + 2] = (uintptr_t) a2;
+ ucp->uc_mcontext.sc_regs[LARCH_REG_A0 + 3] = (uintptr_t) a3;
+ ucp->uc_mcontext.sc_regs[LARCH_REG_A0 + 4] = (uintptr_t) a4;
if (__glibc_unlikely (argc > 5))
{
@@ -62,14 +62,14 @@ __makecontext (ucontext_t *ucp, void (*func) (void), int argc, long int a0,
long int reg_args = argc < LARCH_REG_NARGS ? argc : LARCH_REG_NARGS;
for (long int i = 5; i < reg_args; i++)
- ucp->uc_mcontext.__gregs[LARCH_REG_A0 + i] = va_arg (vl, unsigned long int);
+ ucp->uc_mcontext.sc_regs[LARCH_REG_A0 + i] = va_arg (vl, unsigned long int);
long int stack_args = argc - reg_args;
if (stack_args > 0)
{
sp = (unsigned long int *)
(((uintptr_t) sp - stack_args * sizeof (long int)) & ALMASK);
- ucp->uc_mcontext.__gregs[LARCH_REG_SP] = (uintptr_t) sp;
+ ucp->uc_mcontext.sc_regs[LARCH_REG_SP] = (uintptr_t) sp;
for (long int i = 0; i < stack_args; i++)
sp[i] = va_arg (vl, unsigned long int);
}
@@ -26,7 +26,7 @@
static inline uintptr_t
sigcontext_get_pc (const ucontext_t *ctx)
{
- return ctx->uc_mcontext.__pc;
+ return ctx->uc_mcontext.sc_pc;
}
#endif
@@ -43,18 +43,25 @@ typedef unsigned long int greg_t;
typedef greg_t gregset_t[32];
#endif
+#ifdef __USE_MISC
+# define __ctx(fld) fld
+#else
+# define __ctx(fld) __ ## fld
+#endif
+
+
typedef struct mcontext_t
{
- unsigned long long __pc;
- unsigned long long __gregs[32];
- unsigned int __flags;
- unsigned long long __extcontext[0] __attribute__((__aligned__(16)));
+ unsigned long long __ctx(sc_pc);
+ unsigned long long __ctx(sc_regs)[32];
+ unsigned int __ctx(sc_flags);
+ unsigned long long __ctx(sc_extcontext)[0] __attribute__((__aligned__(16)));
} mcontext_t;
/* Userlevel context. */
typedef struct ucontext_t
{
- unsigned long int __uc_flags;
+ unsigned long int __ctx(uc_flags);
struct ucontext_t *uc_link;
stack_t uc_stack;
sigset_t uc_sigmask;
@@ -15,7 +15,7 @@ _NSIG8 (_NSIG / 8)
#define stack(member) ucontext (uc_stack.member)
#define mcontext(member) ucontext (uc_mcontext.member)
-UCONTEXT_FLAGS ucontext (__uc_flags)
+UCONTEXT_FLAGS ucontext (uc_flags)
UCONTEXT_LINK ucontext (uc_link)
UCONTEXT_STACK ucontext (uc_stack)
UCONTEXT_MCONTEXT ucontext (uc_mcontext)
@@ -25,7 +25,7 @@ STACK_SP stack (ss_sp)
STACK_SIZE stack (ss_size)
STACK_FLAGS stack (ss_flags)
-MCONTEXT_PC mcontext (__pc)
-MCONTEXT_GREGS mcontext (__gregs)
+MCONTEXT_PC mcontext (sc_pc)
+MCONTEXT_GREGS mcontext (sc_regs)
UCONTEXT_SIZE sizeof (ucontext_t)