|
|
Message-ID: <20261104131903.GS23438@brightrain.aerifal.cx> Date: Wed, 4 Nov 2026 08:19:03 -0500 From: Rich Felker <dalias@...c.org> To: Imre Kaloz <kaloz@...nel.org> Cc: musl@...ts.openwall.com Subject: Re: [PATCH 04/10] add SYSCALL_RT_SIGACTION_RESTORER hook for sparc On Wed, Nov 04, 2026 at 08:15:01AM -0500, Rich Felker wrote: > On Wed, Nov 04, 2026 at 10:12:03AM +0100, Imre Kaloz wrote: > > Hi Rich, > > > > On Wed, Nov 4, 2026, at 04:44, Rich Felker wrote: > > > > > I think this patch misses other places SYS_rt_sigaction is used, and > > > puts arch-specific ifdefs we shouldn't be doing inline each place the > > > syscall is used. > > > > Right, abort() still makes the generic call, which on sparc passes > > _NSIG/8 as the restorer and no valid sigsetsize, so resetting SIGABRT > > to SIG_DFL fails. > > > > > Probably we should either (or both) replace direct use of the syscall > > > with a __sys_sigaction (like __sys_open, etc.) defined in > > > src/internal/syscall.h if the arch doesn't first define it, or #undef > > > SYS_rt_sigaction on sparc and have a case for where that's not > > > defined, to be safe against missing instances of its use or adding new > > > ones with silent breakage. > > > > I'd do both. The generic part is below as an RFC; with it, sparc64 > > would define > > > > #define __sys_sigaction(sig,sa,old) \ > > __syscall(__NR_rt_sigaction,sig,sa,old,__restore,_NSIG/8) > > #undef SYS_rt_sigaction > > I'm pretty sure this breaks expansion of __sys_sigaction. > > > > Note that since __restore and __restore_rt are the same, > > > __sys_sigaction does not need a conditional on flags. It can just > > > always pass __restore in the extra slot. > > > > They are on sparc64. 32-bit sparc builds a different frame for > > handlers without SA_SIGINFO and returns from it with sigreturn rather > > than rt_sigreturn, so there the restorer has to follow SA_SIGINFO: > > > > #define __sys_sigaction(sig,sa,old) \ > > __syscall(__NR_rt_sigaction,sig,sa,old, \ > > (long)(sa) && ((sa)->flags & SA_SIGINFO) ? __restore_rt : __restore, \ > > _NSIG/8) > > #undef SYS_rt_sigaction > > > > (sa is tested as an integer because gcc warns on the address of > > abort's compound literal.) Does this look right before I fold it > > into v2? > > Which warning is this? I don't like casts to avoid warnings. > > It might be better to write an inline function by the same name and > just define the macro to itself. That gets rid of the multi-line > macros too. The inline function would also fix the issue with undefining the SYS_rt_sigaction macro, but I think it makes a new issue by depending on _NSIG. Maybe the macro can add _NSIG in the arguments to the inline function. 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.