[Exception Handling] Could we mark __cxa_end_catch as nounwind conditionally?

Hi all,

Let me introduce about the background:
I find that the compiler couldn’t mark foo as nounwind in the following example:

void bar();
void foo() {
try {
bar();
} catch(...) {}
}

This pattern occurs frequently in C++20 coroutine. So I tried to handle coroutine specially before in: ⚙ D108277 [Coroutines] Mark coroutine as nounwind if all the calls outside the try-catch wouldn't throw..
But the all the reviewers strongly suggested that we should handle this case generally for all of functions instead of coroutines only.

Then when I looked into the details in IR, I found the reason is that __cxa_end_catch isn’t nounwind.
Here is the IR generated:

; Function Attrs: mustprogress uwtable
define dso_local void @_Z3foov() local_unnamed_addr #0 personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
invoke void @_Z3barv()
to label %5 unwind label %1

1: ; preds = %0
%2 = landingpad { i8*, i32 }
catch i8* null
%3 = extractvalue { i8*, i32 } %2, 0
%4 = tail call i8* @__cxa_begin_catch(i8* %3) #2 ; nounwind
tail call void @__cxa_end_catch()
br label %5

5: ; preds = %0, %1
ret void
}

I found that if I marked the call to __cxa_end_catch() as nounwind, the foo could be marked as nounwind. So I start to survey why __cxa_end_catch() isn’t ‘nounwind’.
First is the comment on __cxa_end_catch() in libcxxabi:

Upon exit for any reason, a handler must call:
void __cxa_end_catch ();

This routine can be called for either a native or foreign exception.
For a native exception:
* Locates the most recently caught exception and decrements its handler count.
* Removes the exception from the caught exception stack, if the handler count goes to zero.
* If the handler count goes down to zero, and the exception was not re-thrown
by throw, it locates the primary exception (which may be the same as the one
it's handling) and decrements its reference count. If that reference count
goes to zero, the function destroys the exception. In any case, if the current
exception is a dependent exception, it destroys that.

For a foreign exception:
* If it has been rethrown, there is nothing to do.
* Otherwise delete the exception and pop the catch stack to empty.

I am not familiar with exception handling. But from the comment above, it looks like that __cxa_end_catch wouldn’t throw.
Then in clang::ItaniumCXXABI, I found this:

A cleanup to call __cxa_end_catch. In many cases, the caught
exception type lets us state definitively that the thrown exception
type does not have a destructor. In particular:
- Catch-alls tell us nothing, so we have to conservatively
assume that the thrown exception might have a destructor.
- Catches by reference behave according to their base types.
- Catches of non-record types will only trigger for exceptions
of non-record types, which never have destructors.
- Catches of record types can trigger for arbitrary subclasses
of the caught type, so we have to assume the actual thrown
exception type might have a throwing destructor, even if the
caught type's destructor is trivial or nothrow.

It looks like that __cxa_end_catch would throw only if the exception caught has an destructor which may throw.
But I think the situation is rare. First as the comment says, an exception type doesn’t have a destructor usually.
Then if it has a destructor, it is also rare that it may throw. Finally, it is a bad practice to throw from destructor which occurs in catch block.
So I want to provide an option to tell the compiler whether the exceptions in current project has a may-throw destructor. In this way, we could optimize the example in the beginning.

Thanks,
Chuanqi

Hi all,

  Let me introduce about the background:
  I find that the compiler couldn't mark `foo` as `nounwind` in the following example:

void bar();
void foo() {
   try {
       bar();
   } catch(...) {}
}

  From my perspective, it is clear that foo wouldn't throw any exception. So it is natural to me that the compiler could mark foo as nounwind as an optimization. But it didn't.
  This pattern occurs frequently in C++20 coroutine. So I tried to handle coroutine specially before in: ⚙ D108277 [Coroutines] Mark coroutine as nounwind if all the calls outside the try-catch wouldn't throw..
  But the all the reviewers strongly suggested that we should handle this case generally for all of functions instead of coroutines only.

  Then when I looked into the details in IR, I found the reason is that __cxa_end_catch isn't nounwind.
  Here is the IR generated:

; Function Attrs: mustprogress uwtable
define dso_local void @_Z3foov() local_unnamed_addr #0 personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
 invoke void @_Z3barv()
         to label %5 unwind label %1

1:                                                ; preds = %0
 %2 = landingpad { i8*, i32 }
         catch i8* null
 %3 = extractvalue { i8*, i32 } %2, 0
 %4 = tail call i8* @__cxa_begin_catch(i8* %3) #2 ; nounwind
 tail call void @__cxa_end_catch()
 br label %5

5:                                                ; preds = %0, %1
 ret void
}

   I found that if I marked the call to __cxa_end_catch() as `nounwind`, the foo could be marked as `nounwind`. So I start to survey why __cxa_end_catch() isn't 'nounwind'.
   First is the comment on __cxa_end_catch() in libcxxabi:

Upon exit for any reason, a handler must call:
   void __cxa_end_catch ();

This routine can be called for either a native or foreign exception.
For a native exception:
* Locates the most recently caught exception and decrements its handler count.
* Removes the exception from the caught exception stack, if the handler count goes to zero.
* If the handler count goes down to zero, and the exception was not re-thrown
 by throw, it locates the primary exception (which may be the same as the one
 it's handling) and decrements its reference count. If that reference count
 goes to zero, the function destroys the exception. In any case, if the current
 exception is a dependent exception, it destroys that.

For a foreign exception:
* If it has been rethrown, there is nothing to do.
* Otherwise delete the exception and pop the catch stack to empty.

  I am not familiar with exception handling. But from the comment above, it looks like that __cxa_end_catch wouldn't throw.
  Then in clang::ItaniumCXXABI, I found this:

A cleanup to call __cxa_end_catch.  In many cases, the caught
exception type lets us state definitively that the thrown exception
type does not have a destructor.  In particular:
 - Catch-alls tell us nothing, so we have to conservatively
   assume that the thrown exception might have a destructor.
 - Catches by reference behave according to their base types.
 - Catches of non-record types will only trigger for exceptions
   of non-record types, which never have destructors.
 - Catches of record types can trigger for arbitrary subclasses
   of the caught type, so we have to assume the actual thrown
   exception type might have a throwing destructor, even if the
   caught type's destructor is trivial or nothrow.

    It looks like that __cxa_end_catch would throw only if the exception caught has an destructor which may throw.

Yes...

    But I think the situation is rare. First as the comment says, an exception type doesn't have a destructor usually.
Then if it has a destructor, it is also rare that it may throw. Finally, it is a bad practice to throw from destructor which occurs in catch block.
    So I want to provide an option to tell the compiler whether the exceptions in current project has a may-throw destructor. In this way, we could optimize the example in the beginning.

From GCC produced .gcc_except_table, it seems that GCC unconditionally
assumes that __cxa_begin_catch/__cxa_end_catch do not throw. GCC does
not emit call site records for the region with __cxa_end_catch.

So I think we should unconditionally assume that __cxa_begin_catch/__cxa_end_catch don't throw as well.
Sent ⚙ D108905 [ItaniumCXXABI] Make __cxa_end_catch calls unconditionally nounwind

That is not how this works. Here’s the path we need to take.

First, as I mentioned in the review, we need to figure out what the language standard expects to happen when the destructor for an exception throws at the end of a catch. Currently we are essentially treating this as undefined behavior, which I think is highly unlikely to be the intent of the standard.

If it’s allowed to unwind, then __cxa_end_catch can throw, and so the second step is to:
(1) fix the bugs in the language runtimes to handle this situation correctly and not leak the exception object, and then
(2) fix GCC to stop unconditionally assuming that __cxa_end_catch does not throw.
We could then add an opt-in flag to globally assume that exception destructors never throw as an optimization if that’s really important for getting good code.

If it’s supposed to call std::terminate, then we need to come to an agreement with the language runtimes about whose responsibility it is to ensure that the exception is caught and std::terminate is called. That means opening an issue with the Itanium C++ ABI. If we decide it’s the compiler’s responsibility to pass a noexcept function as the exception destructor, then we need to fix Clang to wrap the destructor in a noexcept function when it’s not noexcept. If we decide it’s the runtime’s responsibility, we need to fix the runtimes to catch this exception and call std::terminate. In either of those casess, __cxa_end_catch is known not to throw. We could also decide it’s the compiler’s responsibility to call std::terminate if __cxa_end_catch throws, and if so then we have to do that; however, I think that would be a bad outcome.

John.

        Hi all,

        Let me introduce about the background:
        I find that the compiler couldn't mark `foo` as `nounwind` in
        the following example:

        ```
        void bar();
        void foo() {
        try {
        bar();
        } catch(...) {}
        }
        ```

        From my perspective, it is clear that foo wouldn't throw any
        exception. So it is natural to me that the compiler could mark
        foo as nounwind as an optimization. But it didn't.
        This pattern occurs frequently in C++20 coroutine. So I tried
        to handle coroutine specially before in:
        ⚙ D108277 [Coroutines] Mark coroutine as nounwind if all the calls outside the try-catch wouldn't throw..
        But the all the reviewers strongly suggested that we should
        handle this case generally for all of functions instead of
        coroutines only.

        Then when I looked into the details in IR, I found the reason
        is that __cxa_end_catch isn't nounwind.
        Here is the IR generated:

        ```
        ; Function Attrs: mustprogress uwtable
        define dso_local void @_Z3foov() local_unnamed_addr #0
        personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to
        i8*) {
        invoke void @_Z3barv()
        to label %5 unwind label %1

        1: ; preds = %0
        %2 = landingpad { i8*, i32 }
        catch i8* null
        %3 = extractvalue { i8*, i32 } %2, 0
        %4 = tail call i8* @__cxa_begin_catch(i8* %3) #2 ; nounwind
        tail call void @__cxa_end_catch()
        br label %5

        5: ; preds = %0, %1
        ret void
        }
        ```

        I found that if I marked the call to __cxa_end_catch() as
        `nounwind`, the foo could be marked as `nounwind`. So I start
        to survey why __cxa_end_catch() isn't 'nounwind'.
        First is the comment on __cxa_end_catch() in libcxxabi:

        ```
        Upon exit for any reason, a handler must call:
        void __cxa_end_catch ();

        This routine can be called for either a native or foreign
        exception.
        For a native exception:
        * Locates the most recently caught exception and decrements
        its handler count.
        * Removes the exception from the caught exception stack, if
        the handler count goes to zero.
        * If the handler count goes down to zero, and the exception
        was not re-thrown
        by throw, it locates the primary exception (which may be the
        same as the one
        it's handling) and decrements its reference count. If that
        reference count
        goes to zero, the function destroys the exception. In any
        case, if the current
        exception is a dependent exception, it destroys that.

        For a foreign exception:
        * If it has been rethrown, there is nothing to do.
        * Otherwise delete the exception and pop the catch stack to empty.
        ```

        I am not familiar with exception handling. But from the
        comment above, it looks like that __cxa_end_catch wouldn't throw.
        Then in clang::ItaniumCXXABI, I found this:

        ```
        A cleanup to call __cxa_end_catch. In many cases, the caught
        exception type lets us state definitively that the thrown
        exception
        type does not have a destructor. In particular:
        - Catch-alls tell us nothing, so we have to conservatively
        assume that the thrown exception might have a destructor.
        - Catches by reference behave according to their base types.
        - Catches of non-record types will only trigger for exceptions
        of non-record types, which never have destructors.
        - Catches of record types can trigger for arbitrary subclasses
        of the caught type, so we have to assume the actual thrown
        exception type might have a throwing destructor, even if the
        caught type's destructor is trivial or nothrow.
        ```

        It looks like that __cxa_end_catch would throw only if the
        exception caught has an destructor which may throw.

    Yes...

        But I think the situation is rare. First as the comment says,
        an exception type doesn't have a destructor usually.
        Then if it has a destructor, it is also rare that it may
        throw. Finally, it is a bad practice to throw from destructor
        which occurs in catch block.
        So I want to provide an option to tell the compiler whether
        the exceptions in current project has a may-throw destructor.
        In this way, we could optimize the example in the beginning.

    From GCC produced .gcc_except_table, it seems that GCC unconditionally
    assumes that __cxa_begin_catch/__cxa_end_catch do not throw. GCC does
    not emit call site records for the region with __cxa_end_catch.

    So I think we should unconditionally assume that
    __cxa_begin_catch/__cxa_end_catch don't throw as well.
    Sent ⚙ D108905 [ItaniumCXXABI] Make __cxa_end_catch calls unconditionally nounwind

That is not how this works. Here’s the path we need to take.

First, as I mentioned in the review, we need to figure out what the language standard expects to happen when the destructor for an exception throws at the end of a |catch|. Currently we are essentially treating this as undefined behavior, which I think is highly unlikely to be the intent of the standard.

If it’s allowed to unwind, then |__cxa_end_catch| can throw, and so the second step is to:
(1) fix the bugs in the language runtimes to handle this situation correctly and not leak the exception object, and then
(2) fix GCC to stop unconditionally assuming that |__cxa_end_catch| does not throw.
We could then add an opt-in flag to globally assume that exception destructors never throw as an optimization if that’s really important for getting good code.

FWIW, although I cannot read the GCC exception table to check what that contains, when looking at what happens at runtime GCC appears to handle this correctly (except possibly for a memory leak, no comment on that). Here is a full example which returns 0 when compiled with GCC, regardless of optimisation level:

void bar();
void foo() {
try {
bar();
} catch (...) {}
}

struct S {
~S() noexcept(false) { throw 0; }
};

void bar() {
throw S();
}

int main() {
try {
foo();
} catch (int) {
return 0;
}
return 1;
}

clang is in agreement with GCC, except that this appears to expose a problem in libc++(abi): with libstdc++, behaviour is as with GCC, with libc++ it results in

libc++abi: terminating with unexpected exception of type int
Aborted (core dumped)

If it is decided that this program is valid, I'll report the libc++(abi) problem to the bug tracker and possibly see if I can figure out what's going on.

Intel disagrees and causes

terminate called after throwing an instance of 'int'
Aborted (core dumped)

even when not optimising, even though it is using libstdc++.