[coop][interp] Fix GC transitions for thunk invoke wrappers (#81773)
authorAleksey Kliger (λgeek) <alklig@microsoft.com>
Mon, 13 Feb 2023 14:36:24 +0000 (09:36 -0500)
committerGitHub <noreply@github.com>
Mon, 13 Feb 2023 14:36:24 +0000 (09:36 -0500)
commit9f8c00963f977b3b39039aed4b164fcd04f7d5bd
tree1d7c8b59bb3f6b8f9387d10d18d89d67734d6850
parentae6666d24b0749b73bdf937eb3223eb50d062361
[coop][interp] Fix GC transitions for thunk invoke wrappers (#81773)

The code that recognizes GC transition icalls in the interpreter was only assuming that it will see `mono_threads_enter_gc_safe_region_unbalanced` / `mono_threads_exit_gc_safe_region_unbalanced` which are used by managed-to-native wrappers.

In most cases for native-to-managed wrappers the marshaller emits `mono_threads_attach_coop` / `mono_threads_detach_coop` and those are handled elsewhere by setting the `needs_thread_attach` flag on the `InterpMethod`.

However when the `mono_marshal_get_thunk_invoke_wrapper` API is used, we emit a thunk invoke wrapper which uses
`mono_threads_enter_gc_unsafe_region_unbalanced` / `mono_threads_exit_gc_unsafe_region_unbalanced`

Change the thunk invoke wrapper to also use attach_coop/detach_coop.  This makes the thunks a bit more flexible (they can now be called from unattached threads), at the cost of a TLS lookup.  Also fixes interpreter support since they use the existing attach/detach handling.

As an implementation detail, I added a GC Unsafe Transition Builder to the marshaling code.

Fixes a failure seen in https://github.com/dotnet/runtime/pull/81380 in the MonoAPI/MonoMono/Thunks test (exposed by calling a cctor later than previously)

* Assert that CEE_MONO_GET_SP is followed by gc safe enter/exit icalls

   Only assert in debug builds

* [marshal] Add GCUnsafeTransitionBuilder; use for thunk_invoke_wrapper

Make a "GC Unsafe Transition Builder" - that always calls
mono_threads_attach_coop / mono_threads_detach_coop.

Use it in the native to managed wrappers:
emit_thunk_invoke_wrapper and emit_managed_wrapper

This is a change in behavior for emit_thunk_invoke_wrapper -
previously it directly called mono_threads_enter_gc_unsafe_region_unbalanced.

That means that compared to the previous behavior, the thunk invoke
wrappers are now a bit more lax: they will be able to be called on
threads that aren't attached to the runtime and they will attach
automatically.

On the other hand existing code will continue to work, with the extra
cost of a check of a thread local var.

Using mono_thread_attach_coop also makes invoke wrappers work
correctly in the interpreter - it special cases
mono_thread_attach_coop but not enter_gc_unsafe.
src/mono/mono/metadata/marshal-lightweight.c
src/mono/mono/mini/interp/transform.c