[PATCH] D137381: [clang][compiler-rt] Exception escape out of an non-unwinding function is an undefined behaviour

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added subscribers: vitalybuka, glider, rsmith.
lebedev.ri added a comment.

Can someone from @Sanitizers please comment? :) @vitalybuka ? @glider ? @rsmith  ?

In D137381#3923911 <https://reviews.llvm.org/D137381#3923911>, @MaskRay wrote:

> I think improving diagnostic is useful but `-fsanitize=` is probably not a good place.



> Instrumenting call sites with `callq   __ubsan_handle_exception_escape at PLT` wastes code size.

Can you please explain why replacing one call with another somehow makes things worse?

> The functionality is better handled somewhere in libc++abi personality related code

That would not be helpful, i'm not using libc++.
It seems obviously wrong to me to require every used C++ ABI library
to implement "some improvements", instead, you know,
the clang irgen + clang ubsan lib.

If we do this, we have native source location info for the location of the call site
out of which the exception has escaped, that causes the "abort".
I do not know if that will be possible with less native approach.

The reason i'm doing this, is because recently i had to deal with a few of these bugs,
and i never got any kind of backtrace, so the situation can't not be made better.

> with possible improvement to exception handling related LLVM IR.



>   void bar(); void foo() { bar(); }
>
> `clang++ -fno-exceptions -fsanitize=exception-escape -c b.cc` does not instrument the potentially-throwing-and-propagating `bar()` so an error cannot be caught.

i'm only looking at `-fexceptions` side of things, i'm not sure how
mixing `-fno-exceptions` and `-fexceptions` code is supposed to work.
It is not oblivious to me why not dealing with some other situation
means we can't deal with the situation we can deal with.

> However, for `-fno-exceptions` code, instrumenting every call site is going to greatly increase code size and suppress optimizations.

Citation needed/define "greatly"?
We have a single "termination" landing pad per function,
and we seem to end up with just a single extra `jmp` per call.
https://godbolt.org/z/Td3jWM1oh

But yes, that's the known and expected nature of sanitization.

> I wish that I capture the compiler and runtime behavior well in https://maskray.me/blog/2020-12-12-c++-exception-handling-abi#compiler-behavior. https://discourse.llvm.org/t/catching-exceptions-while-unwinding-through-fno-exceptions-code/57151 is a proposal to improve the diagnostics.

Thanks. That explains current behavior, but does not tell why it must be enshrined and never changed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137381/new/

https://reviews.llvm.org/D137381