Re: [FFmpeg-devel] [PATCH] checkasm: Generalize crash handling

[Henrik Gramner via ffmpeg-devel <ffmpeg-devel@ffmpeg.org>]

On Fri, Dec 22, 2023 at 7:20 AM Rémi Denis-Courmont <remi@remlab.net> wrote:
> >> > +        checkasm_fail_func("%s",
> >> > +                           s == SIGFPE ? "fatal arithmetic error" :
> >> > +                           s == SIGILL ? "illegal instruction" :
> >> > +                           s == SIGBUS ? "bus error" :
> >> > +                                         "segmentation fault");
> >>
> >> The current code for the error print-out is both simpler and more versatile,
> >> so I don't get this.
> >
> >IMO "illegal instruction" is a far better error message than "fatal
> >signal 4" (with an implementation-defined number which nobody knows
> >the meaning of without having to look it up).
>
> The current code prints the number and the name.

Not on all supported systems. And even when it does it's in an
implementation-defined and locale-dependent form, which isn't great.

> >> +    const struct sigaction sa = {
> >> +        .sa_handler = signal_handler,
> >> +        .sa_flags = SA_NODEFER,
> >
> >That does not look very sane to me. If a recursive signal occurs, processing
> >it recursively is NOT a good idea.
>
> Following that, it actually seems safer to automatically reset the handler, using `signal()` or equivalently passing the `SA_RESETHAND` flag. Then the handler can rearm its own self if and *only* if it was able to actually handle the signal by observing a long jump. Resetting to default explicitly is no longer useful then.

Sure, that approach sounds fine.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".