[libcxx-commits] [PATCH] D124255: [libc++] Make check_assertion.h use setjmp/longjmp instead of fork

[John Brawn via Phabricator via libcxx-commits]

john.brawn planned changes to this revision.
john.brawn added a comment.

In D124255#3470538 <https://reviews.llvm.org/D124255#3470538>, @ldionne wrote:

> This looks really interesting! However, doesn't that mean that we can't assert that the process terminates if a certain condition is met?

I'll also adjust this to use atexit to catch that kind of thing as well (and not doing this is why test_check_assertion.pass.cpp fails).

The failure in debug.erase.iter_iter.pass.cpp is a bit more complicated. At the point the assertion happens the list is in an inconsistent state, so when it's later destroyed we get a segmentation fault (this wasn't a fault previously as exiting the child process through __libcpp_assertion_handler meant we bypassed the destruction of the list). I think this can be fixed by copying the closure f after the setjmp, so the longjmp will bypass its destruction, but getting it right is a bit fiddly.



================
Comment at: libcxx/test/support/check_assertion.h:148
+    if (IsValidResultKind(n))
+      return (ResultKind)n;
     return RK_Unknown;
----------------
ldionne wrote:
> 
Will do.


================
Comment at: libcxx/test/support/check_assertion.h:162
   if (GlobalMatcher().Matches(file, line, message)) {
-    std::exit(DeathTest::RK_MatchFound);
+    longjmp(GetJmpBuf(), DeathTest::RK_MatchFound);
   }
----------------
ldionne wrote:
> Here and elsewhere.
Will do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124255