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

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

In D137381#3924698 <https://reviews.llvm.org/D137381#3924698>, @lebedev.ri wrote:

> 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.

@MaskRay Seems find to me. Why do you think so?

>> 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?

Would be nice to see some measuraments? E.g. CTMark.

>> 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.

Handling this case would be usefull.

> 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.





================
Comment at: clang/docs/ReleaseNotes.rst:806
+- A new Undefined Behavior Sanitizer check has been implemented:
+  ``-fsanitize-exception-escape`` (part of ``-fsanitize=undefined``),
+  which catches cases of C++ exceptions trying to unwind
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137381