Message ID | 20230324062510.1812367-1-caiyinyu@loongson.cn |
---|---|
State | New |
Headers | show |
Series | LoongArch: Modify some member names in mcontext_t and ucontext_t structs to align them with the kernel. | expand |
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 > >>
diff --git a/sysdeps/unix/sysv/linux/loongarch/makecontext.c b/sysdeps/unix/sysv/linux/loongarch/makecontext.c index a17f6ccc51..153be5a680 100644 --- a/sysdeps/unix/sysv/linux/loongarch/makecontext.c +++ b/sysdeps/unix/sysv/linux/loongarch/makecontext.c @@ -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); } diff --git a/sysdeps/unix/sysv/linux/loongarch/sigcontextinfo.h b/sysdeps/unix/sysv/linux/loongarch/sigcontextinfo.h index 4cfb87da76..3d6fe08e57 100644 --- a/sysdeps/unix/sysv/linux/loongarch/sigcontextinfo.h +++ b/sysdeps/unix/sysv/linux/loongarch/sigcontextinfo.h @@ -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 diff --git a/sysdeps/unix/sysv/linux/loongarch/sys/ucontext.h b/sysdeps/unix/sysv/linux/loongarch/sys/ucontext.h index 8790265e74..f2af4f5abf 100644 --- a/sysdeps/unix/sysv/linux/loongarch/sys/ucontext.h +++ b/sysdeps/unix/sysv/linux/loongarch/sys/ucontext.h @@ -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; diff --git a/sysdeps/unix/sysv/linux/loongarch/ucontext_i.sym b/sysdeps/unix/sysv/linux/loongarch/ucontext_i.sym index f27afad56f..dfe5199542 100644 --- a/sysdeps/unix/sysv/linux/loongarch/ucontext_i.sym +++ b/sysdeps/unix/sysv/linux/loongarch/ucontext_i.sym @@ -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)