TSan should handle Effect.Unhandled correctly

[fabbing]

TSan wasn't handling correctly the case where an effect was performed without a matching effect handler, as noted in #12535 (comment).

There were two possible behaviours:

• If an effect was performed from a computation, the last caml_reperform would reconnect the current_stack to its parent stack (the main fiber) too early, causing caml_tsan_entry_on_resume to call __tsan_func_entry on too many functions.
  The proposed fix introduces a struct stack_info const* limit argument to caml_tsan_entry_on_resume, which is set to the previous fiber by caml_perform, preventing the unnecessary calls to __tsan_func_entry.
• If an effect was performed from the main fiber, caml_tsan_entry_on_resume was called unconditionally, although caml_tsan_exit_on_perform was bypassed because the fiber has no parent stack.
  In the proposed fix, the limit set to Caml_state(current_stack) by caml_perform prevents again the unnecessary calls to __tsan_func_entry.

When a continuation is resumed, the limit for caml_tsan_entry_on_resume is set to NULL meaning that we expect to unwind the stack of all the fibers from the continuation.

We have also added a test to the TSan testsuite, to validate that the backtrace given by TSan is correct in the 2 cases described above.

[gasche]

I don't understand the details, but the code does not look very nice: 100% of the additions to the C code are new weird corner cases. Is there a simpler way to do this?

If an effect was performed from the main fiber, caml_tsan_entry_on_resume was called unconditionally, although caml_tsan_exit_on_perform was bypassed because the fiber has no parent stack.

What if we did not bypass caml_tsan_exit_on_perform, by calling it before the parent is NULL? check? Could you implement a simpler fix in this scenario?

[OlivierNicole]

We were dubious initially, but it turns out that yes, we found a smaller fix by doing caml_tsan_exit_on_perform prior to testing for the parent fiber.

Note: for this to work, we modify caml_exit_on_perform to perform one less TSan-exit, which we compensate by an explicit exit where needed from the assembly.

[gasche]

I indeed agree that the new version is nicer. I don't 100% follow the details of how the show stack winding/unwinding work in the new model vs. the older one, but I trust the combination of you and the testsuite to get it correct. I reviewed that this PR does not break anything outside TSan mode. Approved.

[gasche]

This line looks a bit redundant with the comment just above.

[OlivierNicole]

Fixed in latest version (I also squashed the history)