LoongArch: Ensure consistency with kernel by using union for struct members in mcontext_t and ucontext_t.

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 decided
to use union to keep these member names consistency.

Reference: 4fa9b3bfe6759c82beb4b043a54a3598ca467289

Co-authored-by: Xi Ruoyao <xry111@xry111.site>
---
 .../unix/sysv/linux/loongarch/makecontext.c   | 26 ++++++++--------
 .../sysv/linux/loongarch/sigcontextinfo.h     |  2 +-
 .../unix/sysv/linux/loongarch/sys/ucontext.h  | 31 ++++++++++++++++---
 .../unix/sysv/linux/loongarch/ucontext_i.sym  |  6 ++--
 4 files changed, 43 insertions(+), 22 deletions(-)

The 04/03/2023 20:01, 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 decided
> to use union to keep these member names consistency.

there is no musl api compat requirement at the moment (the musl
loongarch port is not committed yet so surely that can be
changed to be consistent).

there is an api difference between glibc and linux uapi headers.
i personally don't think this is an issue. (the c abi must match
but struct sigcontext and mcontext_t are not used interchangably
in code. the c++ mangling abi is different even after your patch)

there is also a difference in ucontext compared to other targets:
they have uc_flags field while loongarch has  __uc_flags. i don't
know if this may cause a portability issue.

in any case the commit message does not do a good job describing
the reasoning.

在 2023/4/5 上午1:38, Szabolcs Nagy 写道:
> The 04/03/2023 20:01, 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 decided
>> to use union to keep these member names consistency.
> there is no musl api compat requirement at the moment (the musl
> loongarch port is not committed yet so surely that can be
> changed to be consistent).


We want to ensure that the structure member names in mcontext_t are

consistent with the kernel (like other architectures)

before submitting LoongArch musl (Currently under development) to the 
upstream community.

This will allow for consistency in the structure member names

across musl, glibc, and the kernel for related structures.


> there is an api difference between glibc and linux uapi headers.
> i personally don't think this is an issue. (the c abi must match
> but struct sigcontext and mcontext_t are not used interchangably
> in code. the c++ mangling abi is different even after your patch)


I am currently facing some confusion.

As far as I know, in C++, the member names of a struct or union are not 
subject to mangling.

Mangling is only applied to the names of C++ entities such as

functions, variables, classes, and namespaces.

The member names of a struct or union are simply a part of

the struct or union, and their names are not encoded or modified.

Therefore, even if the member names of a struct or union are duplicated 
in different translation units,

they will not be subject to mangling since they are different symbols.


could you please provide a more detailed explanation about this issue?


BTW, I have written a test case[1] and the result is as expected:

The symbol name of function cyyprint which takes mcontext_t as args

after C++ mangling are all _Z8cyyprint10mcontext_t.


[1]

compiled with  g++ tst1/2.cpp -shared -o tst1/2.so

and nm tst1/2.so | grep cyy


   ➜ cat tst1.cpp

#include <iostream>

typedef struct mcontext_t {
   unsigned long long __pc;
   unsigned long long __gregs[32];
   unsigned int __flags;
   unsigned long long __extcontext[0] __attribute__((__aligned__(16)));
} mcontext_t;

void cyyprint(mcontext_t mx) { std::cout << mx.__pc << std::endl; }

   ➜ cat tst2.cpp


#include <iostream>
#include <features.h>

#ifdef __USE_MISC
#define __ctx(fld) fld
#else
#define __ctx(fld) __##fld
#endif

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;

void cyyprint(mcontext_t mx) { std::cout << mx.__pc << std::endl; }


> there is also a difference in ucontext compared to other targets:
> they have uc_flags field while loongarch has  __uc_flags. i don't
> know if this may cause a portability issue.
>
> in any case the commit message does not do a good job describing
> the reasoning.

The 04/06/2023 17:59, caiyinyu wrote:
> 在 2023/4/5 上午1:38, Szabolcs Nagy 写道:
> > The 04/03/2023 20:01, 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 decided
> > > to use union to keep these member names consistency.
> > there is no musl api compat requirement at the moment (the musl
> > loongarch port is not committed yet so surely that can be
> > changed to be consistent).
> 
> 
> We want to ensure that the structure member names in mcontext_t are
> 
> consistent with the kernel (like other architectures)
> 
> before submitting LoongArch musl (Currently under development) to the
> upstream community.
> 
> This will allow for consistency in the structure member names
> 
> across musl, glibc, and the kernel for related structures.
> 
> 
> > there is an api difference between glibc and linux uapi headers.
> > i personally don't think this is an issue. (the c abi must match
> > but struct sigcontext and mcontext_t are not used interchangably
> > in code. the c++ mangling abi is different even after your patch)
> 
> 
> I am currently facing some confusion.
> 
> As far as I know, in C++, the member names of a struct or union are not
> subject to mangling.

sorry i did not mean to say you are breaking c++ abi compared to
the current glibc code, but that the linux sigcontext struct has
different c++ abi even if the field names are the same (because
sigcontext vs mcontext_t struct tag).

this tells me that you cannot use those two types interchangably
in c++ library apis anyway and i would expect that the two types
don't get confused in source code, so the field names don't need
to be consistent (of course it's nicer if they are the same).

one minor issue is that in principle this is a posix api and the
header is supposed to work with any c99 compiler, but unnamed
union and struct members are not c99. in practice i don't think
this is an issue: all loongarch c compilers will support it
and glibc already relies on extensions (like attribute aligned).

i'm not against the patch, but to me the commit message implied
the change was because of musl, while it is for being more
consistent with linux uapi and other targets.