Re: Dynamic modules: MODULE_HANDLE_SIGNALS etc.

[Daniel Colascione <dancol@dancol.org>]

[-- Attachment #1: Type: text/plain, Size: 4224 bytes --]

On 01/03/2016 08:31 AM, Paul Eggert wrote:
> Daniel Colascione wrote:
>> In 2014, Emacs gained a new path in the SIGSEGV handler that attempts to
>> detect C stack oerflow and longjmp back to toplevel. It's important to
>> note that we don't just longjmp when we're in a safe position: we
>> longjmp from *anywhere*, even if we're, say, in the middle of malloc.
> 
> Although that particular code path may have been introduced recently,
> for decades Emacs has longjmped from arbitrary locations due to other
> signals, so adding a longjmp for SIGSEGV does not introduce new issues.

What do you mean? We can lisp-signal from lots of places (but not
arbitrary values of the program counter), but we don't respond to *unix*
signals by longjmping.

When I added the SIGUSR2 debug-break support, I made handle_user_signal
set Vquit_flag instead of longjmping in order to avoid exactly the
problem I'm highlighting here.

We don't longjmp from SIGIO either, by the way: we set a flag that we
inspect later, at a safe place.

>> The code is fundamentally flawed and cannot be made to work
>> properly on any platform.
> 
> The code is part of Emacs 24.5 and does not appear to be causing
> problems; at least, I don't recall any bug reports from the field. The
> other longjmps, which are fundamentally flawed in the same way, have
> been in Emacs for decades, and also seem to work well enough in practice.
> 
>> No other program attempts to recover from
>> stack overflow this way.
> 
> True, not *exactly* in this way, but Emacs is pretty special.

Not in this respect. In particular, Emacs has no special magic that
makes it safe to longjmp out of arbitrary C program sequences.

>> In practice, the Lisp stack depth limits provide enough protection
> 
> That won't be true once once people link in dynamic modules, since the
> modules may not use Lisp and may exhaust the C stack. And even without
> modules, I recall people running into stack-overflow issues in the
> regular-expression code. Did that ever get fixed? Even if so, most
> likely other such issues are still lurking in the regexp code.
> 
>> The existing auto-save logic
>> is good enough for data recovery, especially if we run the sigsegv
>> handler on the alternate signal stack (which we can make as large as we
>> want) when possible.
> 
> Something like that could help, yes. But the existing auto-save logic
> can longjmp out, so I don't see how this would address your concern
> about longjmp.

It can Fsignal. That's not the same thing. Fsignal is a supported,
controlled mechanism with defined semantics, not a free-for-all like the
SIGSEGV handler.

> One possible way forward here is the approach recommended by GNU
> libsigsegv. See <https://www.gnu.org/software/libsigsegv/> "About stack
> overflow handlers". In the past we've avoided libsigsegv's approach
> because it was considered to be too heavyweight, but it would be safer
> to do something along the lines that it suggests, or perhaps even to use
> libsigsegv if available.

The libsigsegv approach is a problem too: pthread cancellation in
practice is unsupported on some platforms, does not execute C++
destructors on some platforms even where it's supported, is lightly
tested, and conflicts with other uses of sigsegv. In the case of Emacs,
pthread cancellation is additionally risky because we don't even try to
PTHREAD_CANCEL_DISABLE around critical code sequences.

Neither you nor Eli have provided any evidence that we need to worry
about C stack overflow at all and that any of the proposed recovery
schemes is worth the technical complexity and risk. We should treat C
stack overflow exactly like we treat a NULL dereference.

I've previously proposed alternate, safe, ways we can recover from C
stack overflow, but I don't think we should use any of these, because I
don't think it's worth attempting to recover from this bad state at all.

>> If we keep this code in Emacs, it sets a precedent for other terrible
>> forms of crash recovery, like silently ignoring writes to NULL ...
> 
> Naah. We're pragmatic, not stupid.

People who call themselves pragmatic have had worse ideas.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]