Re: [musl] add loongarch64 port v3

[Szabolcs Nagy <nsz@port70.net>]

* 王洪亮 <wanghongliang@loongson.cn> [2022-05-26 11:07:42 +0800]:
> 在 2022/5/25 下午8:32, Rich Felker 写道:
> > On Wed, May 25, 2022 at 06:08:23PM +0800, 王洪亮 wrote:
> > > 在 2022/5/24 下午8:32, Rich Felker 写道:
> > > > What we've been trying to say is that there are several cases, none of
> > > > which seem to need it:
> > > > 
> > > > 1. You create an object with declared type struct sigcontext. In this
> > > >     case, the flexible array member at the end is not present at all
> > > >     (because that's how C works) which means there's no extended
> > > >     context which needs additional alignment and probably also means
> > > >     this is not a usable way of creating sigcontext structs.
> > > > 
> > > > 2. You malloc storage for the object with space for the flexible array
> > > >     member. In this case the allocation has alignment max_align_t and
> > > >     everything is fine.
> I don't understand what is alignment max_align_t? I found the max_align_t
> definition in musl,is this it?

it is specified in iso c11. malloc aligns at least to this alignment.

> 
> TYPEDEF struct { long long __ll; long double __ld; } max_align_t;
> 
> I understand if FAM is not specified alignment,FAM is aligned according to
> its type size,why is max_align_t?
> > > > 
> > > > 3. You get the object from the kernel pushing it onto the stack in a
> > > >     signal frame. This is probably actually the only case the type is
> > > >     usable in, and of course it has whatever alignment the kernel gave
> > > >     it.
> > > Specify the __attribute__((__aligned__(16))) in musl,is used to be
> > > consistent with kernel.if I removed the __attribute__((__aligned__(16))),
> > > there is a libc-test fail in pthread_cancel.exe.the reason is that the
> > > offset of uc->uc_mcontext from the start of uc obtained in cancel_handler
> > > is inconsistent with kernel pushing it onto the stack in a signal frame.
> > > so I understand the __attribute__((__aligned__(16))) is necessary in musl.
> > > 
> > > src/thread/pthread_cancel.c
> > > 
> > > static void cancel_handler(int sig, siginfo_t *si, void *ctx)
> > > {
> > >          pthread_t self = __pthread_self();
> > >          ucontext_t *uc = ctx;
> > >          uintptr_t pc = uc->uc_mcontext.MC_PC;
> > >           ...
> > > }
> > > 
> > > musl/arch/loongarch64/bits/signal.h:
> > > 
> > > typedef unsigned long greg_t, gregset_t[32];
> > > typedef struct sigcontext {
> > >          unsigned long pc;
> > >          gregset_t gregs;
> > >          unsigned int flags;
> > >          unsigned long extcontext[];
> > > }mcontext_t;
> > > 
> > > linux/arch/loongarch/include/uapi/asm/sigcontext.h:
> > > 
> > > struct sigcontext {
> > >          __u64   sc_pc;
> > >          __u64   sc_regs[32];
> > >          __u32   sc_flags;
> > >          __u64   sc_extcontext[0] __attribute__((__aligned__(16)));
> > > };
> > This is because ucontext_t is defined without explicit padding before
> > uc_mcontext. Add "long __uc_pad;" or similar before it so that the
> > offset is explicitly what it's supposed to be rather than a
> > consequence ot overalignment.
> 
> Add "long __uc_pad;" before uc_mcontext can resolve offset error,
> why it is better than sc_extcontext[] __attribute__((__aligned__(16)))?
> isn't it more direct to be consistent with kernel?

the aligned attribute is not standard (iso c11 has _Alignas instead).

the kernel does not care, but libc headers must work with all compilers
and c language parsing tools, so if we can specify a struct with the
same abi that the kernel wants but in a standard conform way, then we
should do it that way.

stating the padding explicitly is useful anyway when the exact layout
of a type matters. it is not just about declaring the type but
documenting the abi.