llvm dev - Aug 2021 - [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(...) {}
}
```

   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: https://reviews.llvm.org/D108277.
   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
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20210830/4db8b2ed/attachment.html>

On 2021-08-30, chuanqi.xcq wrote:
>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: https://reviews.llvm.org/D108277.
>   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 https://reviews.llvm.org/D108905