ÔÚ 2024/1/30 ÉÏÎç12:47, Rich Felker дµÀ: > 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. Yes, the purpose of the last 2 slots of __jmp_buf just used for sigsetjmp to storing its extra data. Now leave them there as extensibility is OK. Hongliang Wang > > 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 > I understand. I attached a patch to fix this issue. please review it. diff --git a/src/signal/loongarch64/sigsetjmp.s b/src/signal/loongarch64/sigsetjmp.s index 992ab1a4..9c0e3ae2 100644 --- a/src/signal/loongarch64/sigsetjmp.s +++ b/src/signal/loongarch64/sigsetjmp.s @@ -5,8 +5,8 @@ sigsetjmp: __sigsetjmp: beq $a1, $zero, 1f - st.d $ra, $a0, 168 - st.d $s0, $a0, 176 + st.d $ra, $a0, 184 + st.d $s0, $a0, 200 #184+8+8 move $s0, $a0 la.global $t0, setjmp @@ -14,8 +14,8 @@ __sigsetjmp: move $a1, $a0 # Return from 'setjmp' or 'longjmp' move $a0, $s0 - ld.d $ra, $a0, 168 - ld.d $s0, $a0, 176 + ld.d $ra, $a0, 184 + ld.d $s0, $a0, 200 #184+8+8 .hidden __sigsetjmp_tail la.global $t0, __sigsetjmp_tail (END) Hongliang Wang