|
Message-ID: <20220529063402.GS1320090@port70.net> Date: Sun, 29 May 2022 08:34:02 +0200 From: Szabolcs Nagy <nsz@...t70.net> To: 王洪亮 <wanghongliang@...ngson.cn> Cc: musl@...ts.openwall.com Subject: Re: add loongarch64 port v3 * 王洪亮 <wanghongliang@...ngson.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.
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.