Re: MPS: a random backtrace while toying with gdb

[Pip Cet]

On Monday, July 1st, 2024 at 11:07, Eli Zaretskii <eliz@gnu.org> wrote:
> > Date: Sun, 30 Jun 2024 20:32:54 +0000
> 
> > From: Pip Cet pipcet@protonmail.com
> > Cc: eller.helmut@gmail.com, gerd.moellmann@gmail.com, yantar92@posteo.net, 
> > emacs-devel@gnu.org
> > 
> > On Sunday, June 30th, 2024 at 20:09, Eli Zaretskii eliz@gnu.org wrote:
> > 
> > > > Date: Sun, 30 Jun 2024 19:25:36 +0000
> > > 
> > > > From: Pip Cet pipcet@protonmail.com
> > > > Cc: Eli Zaretskii eliz@gnu.org, gerd.moellmann@gmail.com, 
> > > > yantar92@posteo.net,
> > > > emacs-devel@gnu.org
> > > > 
> > > > IIUC the consensus is you can set an "int" or "bool" to true, or write 
> > > > to a self-pipe, but that's about as much as you should ever do in a 
> > > > signal handler...
> > > 
> > > That's not true, AFAIU: you can safely call any of the dozens of
> > > functions listed by the signal-safety(7) man page.
> > 
> > I think the implication is reversed there: functions that aren't on that 
> > man page definitely aren't safe to call, but that doesn't mean that any old 
> > C code modifying complicated structures using only the listed functions is 
> > safe, at all.
> 
> I didn't say "any code"; obviously, one can write unsafe code even
> without calling any function! This is a strawman if ever there was
> one.

I'm sorry, I must have misunderstood you. I thought you said I could safely 
call any of dozens of functions when what I talked about was what signal 
handlers can actually, effectively, do. There's no contradiction there.

> > My point is that complicated C structures, such as the queue Helmut 
> > implemented, cannot usually be safely modified from signal handlers unless 
> > the non-signal code takes care to block signals while it is modifying the 
> > structure.
> 
> It goes without saying that updating a static data structure makes the
> signal handler non-reentrant, and it should at least block SIGCHLD.

Blocking SIGCHLD in a SIGCHLD handler is redundant unless SA_NODEFER is in use. 
Are we talking about a different handler here?

> > All that said, it's not like our existing code looks safe, either, so the 
> > patch is definitely an improvement!
> 
> I don't think I agree with the "improvement" part. The challenge is
> to prevent crashes without giving up useful features, not by means of
> giving them up.

One last suggestion: how about blocking those signals for most of Emacs' 
lifetime, only unblocking them in maybe_quit or at similar points? That would 
allow us to keep the existing signal handlers and make them safe...

I still think this is a simple oversight on the part of MPS, FWIW. You 
shouldn't allow other signals when handling SIGSEGV, or at least give the 
client program an option to specify a signal mask.

Pip