public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] LoongArch: Modify some member names in mcontext_t and ucontext_t structs to align them with the kernel.
@ 2023-03-24  6:25 caiyinyu
  2023-03-24  7:00 ` Xi Ruoyao
  2023-03-24  7:40 ` Xi Ruoyao
  0 siblings, 2 replies; 15+ messages in thread
From: caiyinyu @ 2023-03-24  6:25 UTC (permalink / raw)
  To: libc-alpha; +Cc: adhemerval.zanella, xry111, caiyinyu

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

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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] LoongArch: Modify some member names in mcontext_t and ucontext_t structs to align them with the kernel.
  2023-03-24  6:25 [PATCH] LoongArch: Modify some member names in mcontext_t and ucontext_t structs to align them with the kernel caiyinyu
@ 2023-03-24  7:00 ` Xi Ruoyao
  2023-03-24  7:40 ` Xi Ruoyao
  1 sibling, 0 replies; 15+ messages in thread
From: Xi Ruoyao @ 2023-03-24  7:00 UTC (permalink / raw)
  To: caiyinyu, libc-alpha; +Cc: adhemerval.zanella

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.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] LoongArch: Modify some member names in mcontext_t and ucontext_t structs to align them with the kernel.
  2023-03-24  6:25 [PATCH] LoongArch: Modify some member names in mcontext_t and ucontext_t structs to align them with the kernel caiyinyu
  2023-03-24  7:00 ` Xi Ruoyao
@ 2023-03-24  7:40 ` Xi Ruoyao
  2023-03-31  3:52   ` caiyinyu
  1 sibling, 1 reply; 15+ messages in thread
From: Xi Ruoyao @ 2023-03-24  7:40 UTC (permalink / raw)
  To: caiyinyu, libc-alpha; +Cc: adhemerval.zanella

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;

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] LoongArch: Modify some member names in mcontext_t and ucontext_t structs to align them with the kernel.
  2023-03-24  7:40 ` Xi Ruoyao
@ 2023-03-31  3:52   ` caiyinyu
  2023-04-03  2:52     ` caiyinyu
  2023-04-03  8:56     ` Xi Ruoyao
  0 siblings, 2 replies; 15+ messages in thread
From: caiyinyu @ 2023-03-31  3:52 UTC (permalink / raw)
  To: Xi Ruoyao, libc-alpha
  Cc: adhemerval.zanella, Chenghua Xu, 王洪虎, lixing


在 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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] LoongArch: Modify some member names in mcontext_t and ucontext_t structs to align them with the kernel.
  2023-03-31  3:52   ` caiyinyu
@ 2023-04-03  2:52     ` caiyinyu
  2023-04-03  8:56     ` Xi Ruoyao
  1 sibling, 0 replies; 15+ messages in thread
From: caiyinyu @ 2023-04-03  2:52 UTC (permalink / raw)
  To: Xi Ruoyao, libc-alpha
  Cc: adhemerval.zanella, Chenghua Xu, 王洪虎, lixing

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
>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] LoongArch: Modify some member names in mcontext_t and ucontext_t structs to align them with the kernel.
  2023-03-31  3:52   ` caiyinyu
  2023-04-03  2:52     ` caiyinyu
@ 2023-04-03  8:56     ` Xi Ruoyao
  2023-04-03  9:18       ` caiyinyu
  1 sibling, 1 reply; 15+ messages in thread
From: Xi Ruoyao @ 2023-04-03  8:56 UTC (permalink / raw)
  To: caiyinyu, libc-alpha
  Cc: adhemerval.zanella, Chenghua Xu, 王洪虎, lixing

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.


-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] LoongArch: Modify some member names in mcontext_t and ucontext_t structs to align them with the kernel.
  2023-04-03  8:56     ` Xi Ruoyao
@ 2023-04-03  9:18       ` caiyinyu
  2023-04-03  9:25         ` Xi Ruoyao
  0 siblings, 1 reply; 15+ messages in thread
From: caiyinyu @ 2023-04-03  9:18 UTC (permalink / raw)
  To: Xi Ruoyao, libc-alpha
  Cc: adhemerval.zanella, Chenghua Xu, 王洪虎, lixing


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

>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] LoongArch: Modify some member names in mcontext_t and ucontext_t structs to align them with the kernel.
  2023-04-03  9:18       ` caiyinyu
@ 2023-04-03  9:25         ` Xi Ruoyao
  2023-04-03  9:41           ` caiyinyu
  0 siblings, 1 reply; 15+ messages in thread
From: Xi Ruoyao @ 2023-04-03  9:25 UTC (permalink / raw)
  To: caiyinyu, libc-alpha
  Cc: adhemerval.zanella, Chenghua Xu, 王洪虎, lixing

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

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] LoongArch: Modify some member names in mcontext_t and ucontext_t structs to align them with the kernel.
  2023-04-03  9:25         ` Xi Ruoyao
@ 2023-04-03  9:41           ` caiyinyu
  2023-04-03 10:12             ` Xi Ruoyao
  0 siblings, 1 reply; 15+ messages in thread
From: caiyinyu @ 2023-04-03  9:41 UTC (permalink / raw)
  To: Xi Ruoyao, libc-alpha
  Cc: adhemerval.zanella, Chenghua Xu, 王洪虎,
	lixing, wuxiaotian


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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] LoongArch: Modify some member names in mcontext_t and ucontext_t structs to align them with the kernel.
  2023-04-03  9:41           ` caiyinyu
@ 2023-04-03 10:12             ` Xi Ruoyao
  2023-04-03 10:45               ` caiyinyu
  0 siblings, 1 reply; 15+ messages in thread
From: Xi Ruoyao @ 2023-04-03 10:12 UTC (permalink / raw)
  To: caiyinyu, libc-alpha
  Cc: adhemerval.zanella, Chenghua Xu, 王洪虎,
	lixing, wuxiaotian

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.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] LoongArch: Modify some member names in mcontext_t and ucontext_t structs to align them with the kernel.
  2023-04-03 10:12             ` Xi Ruoyao
@ 2023-04-03 10:45               ` caiyinyu
  2023-04-03 12:03                 ` caiyinyu
  0 siblings, 1 reply; 15+ messages in thread
From: caiyinyu @ 2023-04-03 10:45 UTC (permalink / raw)
  To: Xi Ruoyao, libc-alpha
  Cc: adhemerval.zanella, Chenghua Xu, 王洪虎,
	lixing, wuxiaotian


在 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__".



>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] LoongArch: Modify some member names in mcontext_t and ucontext_t structs to align them with the kernel.
  2023-04-03 10:45               ` caiyinyu
@ 2023-04-03 12:03                 ` caiyinyu
  0 siblings, 0 replies; 15+ messages in thread
From: caiyinyu @ 2023-04-03 12:03 UTC (permalink / raw)
  To: Xi Ruoyao, libc-alpha
  Cc: adhemerval.zanella, Chenghua Xu, 王洪虎,
	lixing, wuxiaotian


在 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

>
>>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] LoongArch: Modify some member names in mcontext_t and ucontext_t structs to align them with the kernel.
  2023-04-03 12:29 ` Adhemerval Zanella Netto
@ 2023-04-03 12:33   ` caiyinyu
  0 siblings, 0 replies; 15+ messages in thread
From: caiyinyu @ 2023-04-03 12:33 UTC (permalink / raw)
  To: Adhemerval Zanella Netto, libc-alpha


在 2023/4/3 下午8:29, Adhemerval Zanella Netto 写道:
>
> On 02/04/23 23:51, 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.
> You can't do it without breaking the API and I don't think we should allow
> the precedence.  Afaik loongarch is not yet on musl, so why can't you mimic
> what loongarch already defined in glibc on musl?

The latest patch:

https://sourceware.org/pipermail/libc-alpha/2023-April/146897.html

The latest emails:

https://sourceware.org/pipermail/libc-alpha/2023-April/thread.html (bottom)


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] LoongArch: Modify some member names in mcontext_t and ucontext_t structs to align them with the kernel.
  2023-04-03  2:51 caiyinyu
@ 2023-04-03 12:29 ` Adhemerval Zanella Netto
  2023-04-03 12:33   ` caiyinyu
  0 siblings, 1 reply; 15+ messages in thread
From: Adhemerval Zanella Netto @ 2023-04-03 12:29 UTC (permalink / raw)
  To: caiyinyu, libc-alpha



On 02/04/23 23:51, 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.

You can't do it without breaking the API and I don't think we should allow
the precedence.  Afaik loongarch is not yet on musl, so why can't you mimic
what loongarch already defined in glibc on musl?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH] LoongArch: Modify some member names in mcontext_t and ucontext_t structs to align them with the kernel.
@ 2023-04-03  2:51 caiyinyu
  2023-04-03 12:29 ` Adhemerval Zanella Netto
  0 siblings, 1 reply; 15+ messages in thread
From: caiyinyu @ 2023-04-03  2:51 UTC (permalink / raw)
  To: libc-alpha; +Cc: caiyinyu

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  | 19 ++++++++++----
 .../unix/sysv/linux/loongarch/ucontext_i.sym  |  6 ++---
 4 files changed, 31 insertions(+), 22 deletions(-)

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..ecc68c8378 100644
--- a/sysdeps/unix/sysv/linux/loongarch/sys/ucontext.h
+++ b/sysdeps/unix/sysv/linux/loongarch/sys/ucontext.h
@@ -43,22 +43,31 @@ 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;
   mcontext_t uc_mcontext;
 } ucontext_t;
 
+#undef __ctx
+
 #endif /* sys/ucontext.h */
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)
-- 
2.31.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2023-04-03 12:33 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-24  6:25 [PATCH] LoongArch: Modify some member names in mcontext_t and ucontext_t structs to align them with the kernel caiyinyu
2023-03-24  7:00 ` Xi Ruoyao
2023-03-24  7:40 ` Xi Ruoyao
2023-03-31  3:52   ` caiyinyu
2023-04-03  2:52     ` caiyinyu
2023-04-03  8:56     ` Xi Ruoyao
2023-04-03  9:18       ` caiyinyu
2023-04-03  9:25         ` Xi Ruoyao
2023-04-03  9:41           ` caiyinyu
2023-04-03 10:12             ` Xi Ruoyao
2023-04-03 10:45               ` caiyinyu
2023-04-03 12:03                 ` caiyinyu
2023-04-03  2:51 caiyinyu
2023-04-03 12:29 ` Adhemerval Zanella Netto
2023-04-03 12:33   ` caiyinyu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).