[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240129164702.GR4163@brightrain.aerifal.cx>
Date: Mon, 29 Jan 2024 11:47:02 -0500
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Cc: Hongliang Wang <wanghongliang@...ngson.cn>
Subject: Re: loongarch64 merge
On Thu, Jan 25, 2024 at 12:43:54PM -0500, Rich Felker wrote:
> I'm going through where everything was left on this topic and
> preparing a patch for merge. This message/thread is to document what
> I'm actually doing vs the various submitted versions of the patch
> since v5/v6 where the major review took place.
>
>
> Subsequent changes I'm reverting:
>
> - De-optimization of __get_tp. No motivation for removing the
> potentially in-place $tp was provided, and we generally use the
> arch's tp in-place unless there's a compiler bug to be worked
> around. See powerpc{,64} for an example where it's used, or1k where
> we have a probably-obsolete workaround for ancient clang being
> broken.
>
> - unsigned -> unsigned int, etc.
>
> - Gratuitous whitespace changes in headers that obscure the fact that
> a header is a complete duplicate that could eventually be shared
> between archs (e.g. bits/float.h, bits/posix.h) or just obscure
> what differs from other archs when running diff.
>
>
> Fixes from previous review that were overlooked:
>
> - Removing SA_RESTORER -- its presence defined as 0 produces wrong
> sigaction ABI.
>
>
> Additions:
>
> - Adding the reloc.h/configure case for single-only float.
>
> - The new member names for mcontext_t are all in reserved namespace,
> so there's no reason to have a separate namespace-clean version of
> mcontext_t, and I'm removing the latter.
>
> - Public member uc_flags with no __, macro for compat with any
> existing software using the __-prefixed name.
>
>
> Still TODO:
>
> I don't think I ever reviewed the apparent rewrite of sigsetjmp and
> possibly some other asm that changed between v5 and v8. I'm about to
> start looking at that and will follow up.
OK, sigsetjmp appears to have been non-working in v5 (it failed to
save the argument anywhere, so attempted it use it after it was
clobbered in the code path after calling setjmp).
v8 fixes this and seems like it should work, but I don't understand
the offsets used.
The loongarch64 __jmp_buf has 23 slots, but only appears to use 21 of
them, with sigsetjmp storing its extra data in the last 2 unused
slots. The contract here is supposed to be that the entirety of
__jmp_buf belongs to setjmp/longjmp, with sigsetjmp using the extra
space in the full jmp_buf/sigjmp_buf type with signal mask and lots of
extra space.
Were these slots added just for sigsetjmp? If so, that was an ABI
mistake, but I don't think it warrants a late change; it's probably
better to just leave them there as extensibility.
Either way, I think sigsetjmp should be modified not to rely on
storage in space that setjmp is entitled to be able to modify, i.e. it
should be using offsets 184 and 184+16, not 168 and 176.
Rich
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.
