svn commit: r345696 - head/lib/libvgl

[Bruce Evans]

On Fri, 29 Mar 2019, Konstantin Belousov wrote:

> On Fri, Mar 29, 2019 at 03:57:09PM +0000, Bruce Evans wrote:
>> Author: bde
>> Date: Fri Mar 29 15:57:08 2019
>> New Revision: 345696
>> URL: https://svnweb.freebsd.org/changeset/base/345696
>>
>> Log:
>>   Fix endless loops for handling SIGBUS and SIGSEGV.
>>
>>   r80270 has the usual wrong fix for unsafe signal handling -- just set
>>   a flag and return to let an event loop check the flag and do safe
>>   handling.  This never works for signals like SIGBUS and SIGSEGV that
>>   repeat and works poorly for others unless the application has an event
>>   loop designed to support this.
>>
>>   For these signals, clean up unsafely as before, except for arranging that
>>   nested signals are fatal and forcing a nested signal if the cleanup doesn't
>>   cause one.
>>
>> Modified:
>>   head/lib/libvgl/main.c
>>
>> Modified: head/lib/libvgl/main.c
>> ==============================================================================
>> --- head/lib/libvgl/main.c	Fri Mar 29 15:20:48 2019	(r345695)
>> +++ head/lib/libvgl/main.c	Fri Mar 29 15:57:08 2019	(r345696)
>> ...
>> @@ -107,14 +107,22 @@ struct vt_mode smode;
>>  }
>>
>>  static void
>> -VGLAbort(int arg __unused)
>> +VGLAbort(int arg)
>>  {
>> +  sigset_t mask;
>> +
>>    VGLAbortPending = 1;
>>    signal(SIGINT, SIG_IGN);
>>    signal(SIGTERM, SIG_IGN);
>> -  signal(SIGSEGV, SIG_IGN);
>> -  signal(SIGBUS, SIG_IGN);
>>    signal(SIGUSR2, SIG_IGN);
>> +  if (arg == SIGBUS || arg == SIGSEGV) {
>> +    signal(arg, SIG_DFL);
>> +    sigemptyset(&mask);
>> +    sigaddset(&mask, arg);
>> +    sigprocmask(SIG_UNBLOCK, &mask, NULL);
>> +    VGLEnd();
>> +    kill(getpid(), arg);
> This of course misses the siginfo information from the real fault.

It is in the nested signal frame.

> Why SIGBUS/SIGSEGV are caught at all ?

Otherwise, the screen is left in an application mode that the kernel
doesn't support (except to not write to the screen, so that the
application has full control).

Of course, it is bad for libraries to catch signals.  The man page warns
about installing signal handlers and trying to call VGLEnd() and exit(3)
[from the signal handler] to end the program, although it is important
to use VGLEnd() to end the program.  I haven't tried installing signal
handlers in programs, but my main application is another graphics library
that wraps libvgl, and it installs an atexit() handler which calls its
cleanup function which calls VGLEnd().  libvgl should do the same.  The
cleanup is safe in exit() provided calling exit() is safe.

The man page has general nonsense recommending setting a flag and not
terminating the program in signal handlers.  This was added in the same
commit that completely broke the SIGBUS and SIGSEGV handling using this
method.  A much longer lesson is needed to describe how to use this
method correctly.  SA_RESTART must be turned off for all signals, and
this gives the complexity of handling EINTR returns from all syscalls
affected by SA_RESTART ...

The man page has almost enough details about which signals the library
catches.  It doesn't mention SIGUSR1 (used for screen switches) or
SIGUSR2 (used for mouse signals).  I plan to add use of SIGIO to fix
busy-waiting for keyboard events.  select() might work for the keyboard,
but I doubt that it works for the mouse.

What should happen is if the application installs signal handlers and
does unsafe things not including calling VGLEnd(), then nested SIGBUS's
and SIGSEGV's should reach the above cleanup.  Applications should not
catch SIGBUS or SIGSEGV unless they can clean up better than the above.
It is possible to clean up better than the above, but not in applications.
The above can check a soft signal mask flag like the one set by INTOFF()
and not do so much when it is set.  When it is not set, I think restoring
the screen mode is safe in signal handlers, and only things like free()
and printf() are not safe in signal handlers.

>
>> +  }
>>  }
>>
>>  static void
>

Bruce