[Bug libgcc/113337] New: Rethrown uncaught exceptions don't invoke std::terminate if SEH-based unwinding is used

[Bug libgcc/113337] New: Rethrown uncaught exceptions don't invoke std::terminate if SEH-based unwinding is used

[matteo at mitalia dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113337

            Bug ID: 113337
           Summary: Rethrown uncaught exceptions don't invoke
                    std::terminate if SEH-based unwinding is used
           Product: gcc
           Version: 13.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libgcc
          Assignee: unassigned at gcc dot gnu.org
          Reporter: matteo at mitalia dot net
  Target Milestone: ---

Created attachment 57046
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57046&action=edit
Test program to reproduce the bug

Sample code:

```
#include <exception>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

static void custom_terminate_handler() {
    fprintf(stderr, "custom_terminate_handler invoked\n");
    std::exit(1);
}

int main(int argc, char *argv[]) {
    std::set_terminate(&custom_terminate_handler);
    if (argc < 2) return 1;
    const char *mode = argv[1];
    fprintf(stderr, "%s\n", mode);
    if (strcmp(mode, "throw") == 0) {
        throw std::exception();
    } else if (strcmp(mode, "rethrow") == 0) {
        try {
            throw std::exception();
        } catch (...) {
            throw;
        }
    } else {
        return 1;
    }
    return 0;
}
```

Compiling and running this on e.g. Linux, it prints "custom_terminate_handler
invoked" both when invoked as `./a.out throw` and `./a.out rethrow`. If instead
this is compiled with a 64 bit gcc+mingw64 build that uses SEH exceptions, it
behaves correctly in the "throw" case, but in the rethrow case it crashes in
std::abort (so, you get an "abnormal program termination"/the JIT debugger is
invoked).

Diving in the problem, I think I found the culprit: on rethrow,
`__cxxabiv1::__cxa_rethrow` (libstdc++-v3/libsupc++/eh_throw.cc) invokes
`_Unwind_Resume_or_Rethrow` (libgcc/unwind-seh.c), which goes like this:

```
_Unwind_Reason_Code
_Unwind_Resume_or_Rethrow (struct _Unwind_Exception *exc)
{
  if (exc->private_[0] == 0)
    _Unwind_RaiseException (exc);
  else
    _Unwind_ForcedUnwind_Phase2 (exc);
  abort ();
}
```

The problem here is that unconditional abort(); I don't know exactly about the
else branch, but I think that the "regular", first branch case should read
`return _Unwind_RaiseException (exc);` as, if no handler is found,
`_Unwind_RaiseException` does return to allow the runtime to call
`std::terminate`, as per the relevant comment

```
  /* The exception handler installed in crt0 will continue any GCC
     exception that reaches there (and isn't marked non-continuable).
     Returning allows the C++ runtime to call std::terminate.  */
  return _URC_END_OF_STACK;
```

Indeed, patching the binary on the fly to change the `std::abort` call to a
return fixes the problem, as `__cxa_rethrow` does call `std::terminate` if
`_Unwind_Resume_or_Rethrow` returns.

Comparing with the code from libgcc/unwind.inc (used in the SjLj and Dwarf2
case, from what I gather)

```
/* Resume propagation of an FORCE_UNWIND exception, or to rethrow
   a normal exception that was handled.  */

_Unwind_Reason_Code LIBGCC2_UNWIND_ATTRIBUTE
_Unwind_Resume_or_Rethrow (struct _Unwind_Exception *exc)
{
  struct _Unwind_Context this_context, cur_context;
  _Unwind_Reason_Code code;
  unsigned long frames;

  /* Choose between continuing to process _Unwind_RaiseException
     or _Unwind_ForcedUnwind.  */
  if (exc->private_1 == 0)
    return _Unwind_RaiseException (exc);

  uw_init_context (&this_context);
  cur_context = this_context;

  code = _Unwind_ForcedUnwind_Phase2 (exc, &cur_context, &frames);

  gcc_assert (code == _URC_INSTALL_CONTEXT);

  uw_install_context (&this_context, &cur_context, frames);
}
```

I see that the `_Unwind_RaiseException` case is indeed implemented forwarding
the error code back to the caller, while the `_Unwind_ForcedUnwind_Phase2` case
terminates either in a failed `gcc_assert`, or in a `uw_install_context` (which
is noreturn and boils down to a longjmp, at least in the sjlj case).

So again, I'm not entirely sure about the `_UA_FORCE_UNWIND` case, but the
"regular rethrown exception" branch should surely forward back the error code
instead of crashing on `abort`.

[Bug libgcc/113337] Uncaught rethrown exceptions don't invoke std::terminate if SEH-based unwinding is used

[matteo at mitalia dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113337

--- Comment #1 from Matteo Italia <matteo at mitalia dot net> ---
Looking at this a bit deeper, I think that just putting a `return` in front of
`_Unwind_RaiseException (exc);` should be enough.

The bulk of the phase2 stuff that is done in unwind.inc for
`_Unwind_Resume_or_Rethrow`, i.e.

```
  gcc_assert (code == _URC_INSTALL_CONTEXT);

  uw_install_context (&this_context, &cur_context, frames);
```

is done directly in `_GCC_specific_handler`, and indeed if _that_ doesn't work
`_Unwind_ForcedUnwind_Phase2` can return only `_URC_END_OF_STACK` anyhow, which
would go to the `gcc_assert` codepath in unwind.inc.

[Bug libgcc/113337] Uncaught rethrown exceptions don't invoke std::terminate if SEH-based unwinding is used

[matteo at mitalia dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113337

--- Comment #2 from Matteo Italia <matteo at mitalia dot net> ---
Created attachment 57086
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57086&action=edit
Proposed patch (plain return in front of _Unwind_RaiseException)

[Bug libgcc/113337] Uncaught rethrown exceptions don't invoke std::terminate if SEH-based unwinding is used

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113337

--- Comment #3 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Yong <jyong@gcc.gnu.org>:

https://gcc.gnu.org/g:16774daa597f7633ae2f64efef20cad744b877b9

commit r14-8819-g16774daa597f7633ae2f64efef20cad744b877b9
Author: Matteo Italia <matteo@mitalia.net>
Date:   Wed Jan 17 12:51:44 2024 +0100

    libgcc: fix SEH C++ rethrow semantics [PR113337]

    SEH _Unwind_Resume_or_Rethrow invokes abort directly if
    _Unwind_RaiseException doesn't manage to find a handler for the rethrown
    exception; this is incorrect, as in this case std::terminate should be
    invoked, allowing an application-provided terminate handler to handle
    the situation instead of straight crashing the application through
    abort.

    The bug can be demonstrated with this simple test case:
    ===
    static void custom_terminate_handler() {
        fprintf(stderr, "custom_terminate_handler invoked\n");
        std::exit(1);
    }

    int main(int argc, char *argv[]) {
        std::set_terminate(&custom_terminate_handler);
        if (argc < 2) return 1;
        const char *mode = argv[1];
        fprintf(stderr, "%s\n", mode);
        if (strcmp(mode, "throw") == 0) {
            throw std::exception();
        } else if (strcmp(mode, "rethrow") == 0) {
            try {
                throw std::exception();
            } catch (...) {
                throw;
            }
        } else {
            return 1;
        }
        return 0;
    }
    ===

    On all gcc builds with non-SEH exceptions, this will print
    "custom_terminate_handler invoked" both if launched as ./a.out throw or
    as ./a.out rethrow, on SEH builds instead if will work as expected only
    with ./a.exe throw, but will crash with the "built-in" abort message
    with ./a.exe rethrow.

    This patch fixes the problem, forwarding back the error code to the
    caller (__cxa_rethrow), that calls std::terminate if
    _Unwind_Resume_or_Rethrow returns.

    The change makes the code path coherent with SEH _Unwind_RaiseException,
    and with the generic _Unwind_Resume_or_Rethrow from libgcc/unwind.inc
    (used for SjLj and Dw2 exception backend).

    libgcc/ChangeLog:

            PR libgcc/113337

            * unwind-seh.c (_Unwind_Resume_or_Rethrow): forward
            _Unwind_RaiseException return code back to caller instead of
            calling abort, allowing __cxa_rethrow to invoke std::terminate
            in case of uncaught rethrown exception

[Bug libgcc/113337] Uncaught rethrown exceptions don't invoke std::terminate if SEH-based unwinding is used

[jyong at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113337

jyong at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
                 CC|                            |jyong at gcc dot gnu.org
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #4 from jyong at gcc dot gnu.org ---
Marking as resolved.