[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 updated this revision to Diff 473529.
lebedev.ri edited the summary of this revision.
lebedev.ri added a comment.
Herald added a subscriber: mstorsjo.

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

> In D137381#3907104 <https://reviews.llvm.org/D137381#3907104>, @MaskRay wrote:
>
>> In your example, `clang++ a.cc; ./a.out` gives a libstdc++ error:
>>
>>   terminate called after throwing an instance of 'int'
>>
>> libc++'s is similar.
>
> That's great, but just a symptom of misreduced testcase.
> The whole problem is that in the original bug *no* abort happened at runtime,
> the program terminated successfully, with a mysterious leak.
>
>> footgun is nounwind (due to the GNU pure attribute), so Clang uses `call` instead of `invoke` and the function call is described by a call site entry with a zero action_record_offset (i.e. not a handler) in `.gcc_except_table`.
>> In `_Unwind_RaiseException` called by `__cxa_throw`, the missing exception handler causes `__terminate`.
>>
>> `g++ a.cc; ./a.out` succeeds, because its `footgun` call is caught by `catch (...)`. (Perhaps GCC doesn't have Clang's nounwind optimization.)

FWIW, i now have a standalone reproducer (~3MiB filesize),
and without sanitizers there's no abort regardless of optimization level,
with sanitizers there's only the leak report, and no mention of the exception.

I've already reduced it, but something gone wrong, and for some inexplicable reason,
creduce succeeded with a repro that does not pass the interestingness test
(this is not an error in said interestingness test.) So i will need to re-reduce.

But as as i have expected, the problem will likely end up being more fundamental,
because, not unexpectedly, this new diagnostic does not show up either.

TLDR:

thisisfine <https://reviews.llvm.org/file/data/fbgalfleh3huuxlldqul/PHID-FILE-5xsihirzrmdkq7wnmjfh/meme-on-fire.jpeg>

>> The patch doesn't implement the runtime correctly (I get a linker error) so I cannot try it out.

I believe this should now be fixed.
I did not update `getRecoverableKind()`, which, conveniently, duplicates `Unrecoverable` from `SanitizerArgs.cpp`...

>> How expensive is your instrumentation?
>> Does it work with `-fno-exceptions` intermediate functions? (Unwinding through such functions should fail as well). Note that this check can be done today,without any instrumentation: just use `-fno-asynchronous-unwind-tables` (for some targets which default to async unwind tables (aarch64,x86,etc)).
>> If the process installs a SIGABRT signal handler, the stack trace can be printed when `__terminate` calls `abort`.

As you can see, at least right now, this quite literally relies on
the existing `CodeGenFunction::getTerminateLandingPad()` handling infra,
so it is as cheap as it will be. But has the same bugs as it already has.

The obvious alternative implementation could be to, when emitting `nounwing` function body `F`,
emit it as a thunk with an an invoke of an actual function, but without `nounwind`,
but that is undesirable because we lose all source debug information.

This is UB, and it should be handled by UBSan, not somewhere else,
especially because that will result in best error messages.

> I find this comment non-welcoming and discouraging.
> I just wanted to get something posted when i had something to post already. All of this needs a bit more time.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137381

Files:
  clang/docs/ReleaseNotes.rst
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/include/clang/Basic/Sanitizers.def
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGException.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/CodeGenCXX/catch-exception-escape.cpp
  clang/test/Driver/fsanitize.c
  compiler-rt/lib/ubsan/ubsan_checks.inc
  compiler-rt/lib/ubsan/ubsan_handlers.cpp
  compiler-rt/lib/ubsan/ubsan_handlers.h
  compiler-rt/lib/ubsan_minimal/ubsan_minimal_handlers.cpp
  compiler-rt/test/ubsan/TestCases/Misc/exception-escape.cpp
  compiler-rt/test/ubsan_minimal/TestCases/exception-escape.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D137381.473529.patch
Type: text/x-patch
Size: 30144 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20221107/60d45f22/attachment-0001.bin>