Re: [PATCH] libsupc++: Fix UB terminating on foreign exception

[Andrew Pinski <pinskia@gmail.com>]

On Sat, Jan 13, 2024 at 5:10 PM Julia DeMille <me@jdemille.com> wrote:
>
> Currently, when std::terminate() is called with a foreign exception
> active, since nothing in the path checks whether the exception matches
> the `GNUCC++\0` personality, a foreign exception can go into the verbose
> terminate handler, and get treated as though it were a C++ exception.
>
> Reflection is attempted, and boom. UB. This patch should eliminate that
> UB.

2 things, changelogs go into the email message rather than directly as
part of the patch.,
Second I wonder if you could add a multiple language testcase using
GNU objective-C exceptions as an example.
If not directly adding a testcase there, at least a simple test that
shows the issue outside of the testsuite?

Thanks,
Andrew Pinski

>
> Signed-off-by: Julia DeMille <me@jdemille.com>
> ---
>  libstdc++-v3/ChangeLog               |  9 +++++++++
>  libstdc++-v3/libsupc++/eh_type.cc    | 11 +++++++++++
>  libstdc++-v3/libsupc++/vterminate.cc | 25 ++++++++++++++++++++-----
>  3 files changed, 40 insertions(+), 5 deletions(-)
>
> diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
> index 36257cc4427..bfef0ed8ef1 100644
> --- a/libstdc++-v3/ChangeLog
> +++ b/libstdc++-v3/ChangeLog
> @@ -1,3 +1,12 @@
> +2024-01-13  Julia DeMille  <me@jdemille.com>
> +       * libsupc++/eh_type.cc (__cxa_current_exception_type):
> +       Return NULL if the current exception is not the `GNUCC++\0`
> +       personality.
> +       * libsupc++/vterminate.cc:
> +       Check for both exception header and exception type. If there
> +       is an exception header, but no exception type, state in termination
> +       message that a foreign exception was active.
> +
>  2024-01-13  Jonathan Wakely  <jwakely@redhat.com>
>
>         PR libstdc++/107466
> diff --git a/libstdc++-v3/libsupc++/eh_type.cc b/libstdc++-v3/libsupc++/eh_type.cc
> index 03c677b7e13..e0824eab4d4 100644
> --- a/libstdc++-v3/libsupc++/eh_type.cc
> +++ b/libstdc++-v3/libsupc++/eh_type.cc
> @@ -36,9 +36,20 @@ extern "C"
>  std::type_info *__cxa_current_exception_type () _GLIBCXX_NOTHROW
>  {
>    __cxa_eh_globals *globals = __cxa_get_globals ();
> +
> +  if (!globals)
> +    return 0;
> +
>    __cxa_exception *header = globals->caughtExceptions;
> +
>    if (header)
>      {
> +      // It is UB to try and inspect an exception that isn't ours.
> +      // This extends to attempting to perform run-time reflection, as the ABI
> +      // is unknown.
> +      if (!__is_gxx_exception_class (header->unwindHeader.exception_class))
> +        return 0;
> +
>        if (__is_dependent_exception (header->unwindHeader.exception_class))
>          {
>            __cxa_dependent_exception *de =
> diff --git a/libstdc++-v3/libsupc++/vterminate.cc b/libstdc++-v3/libsupc++/vterminate.cc
> index 23deeef5289..f931d951526 100644
> --- a/libstdc++-v3/libsupc++/vterminate.cc
> +++ b/libstdc++-v3/libsupc++/vterminate.cc
> @@ -25,11 +25,12 @@
>  #include <bits/c++config.h>
>
>  #if _GLIBCXX_HOSTED
> -#include <cstdlib>
> -#include <exception>
> +#include "unwind-cxx.h"
>  #include <bits/exception_defines.h>
> +#include <cstdio>
> +#include <cstdlib>
>  #include <cxxabi.h>
> -# include <cstdio>
> +#include <exception>
>
>  using namespace std;
>  using namespace abi;
> @@ -51,10 +52,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        }
>      terminating = true;
>
> +    __cxa_eh_globals *globals = __cxa_get_globals ();
> +    if (!globals)
> +      {
> +        fputs ("terminate called", stderr);
> +        abort ();
> +      }
> +
>      // Make sure there was an exception; terminate is also called for an
>      // attempt to rethrow when there is no suitable exception.
> -    type_info *t = __cxa_current_exception_type();
> -    if (t)
> +    type_info *t = __cxa_current_exception_type ();
> +    __cxa_exception *header = globals->caughtExceptions;
> +
> +    if (t && header)
>        {
>         // Note that "name" is the mangled name.
>         char const *name = t->name();
> @@ -89,6 +99,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>  #endif
>         __catch(...) { }
>        }
> +    else if (header)
> +      {
> +        fputs ("terminate called after a foreign exception was thrown\n",
> +               stderr);
> +      }
>      else
>        fputs("terminate called without an active exception\n", stderr);
>
> --
> 2.43.0
>