[mono] Possible GC hole in runtime invoke

[lambdageek]

Introduced by #58215 and the 6.0 backport #58364

The issue was: if a method is returning a large valuetype that will need to be boxed into a MonoObject that is bigger than the fixed-size stack allocated buffer that we prepare, use malloc to allocate a bigger buffer on the unmanaged heap and store the result there and then copy it over into a MonoObject.

The problem is that while the value is only in the unmanaged heap buffer, if it has any reference types pointing into the managed heap, those pointers are not visible to the GC. So if there's a collection between when the invoked method returns and the point where we copy the result to the MonoObject, we will have GC holes.

---

Probably what we need to do is use alloca (upto some bigger bound - originally it was 256 bytes, but maybe it needs to be larger).

Alternately, if we always expect to box the result anyway, we could just write it directly into the MonoObject (I think the managed code would always have write barriers there).

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[ghost]

Tagging subscribers to this area: @BrzVlad
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Introduced by #58215 and the 6.0 backport #58364

The issue was: if a method is returning a large valuetype that will need to be boxed into a MonoObject that is bigger than the fixed-size stack allocated buffer that we prepare, use malloc to allocate a bigger buffer on the unmanaged heap and store the result there and then copy it over into a MonoObject.

The problem is that while the value is only in the unmanaged heap buffer, if it has any reference types pointing into the managed heap, those pointers are not visible to the GC. So if there's a collection between when the invoked method returns and the point where we copy the result to the MonoObject, we will have GC holes.

---

Probably what we need to do is use alloca (upto some bigger bound - originally it was 256 bytes, but maybe it needs to be larger).

Alternately, if we always expect to box the result anyway, we could just write it directly into the MonoObject (I think the managed code would always have write barriers there).

Author:	lambdageek
Assignees:	-
Labels:	untriaged, area-GC-mono
Milestone:	-

[lambdageek]

@marek-safar FYI - I think we should probably revert #58364 from 6.0, but also we shoudl throw an ExecutionEngineException if the valuetype is too big (just reverting will mean we're going to go back to a buffer overflow). That will break a System.Text.Json test that was doing something with a large valuetype.

[BrzVlad]

I think we should just use alloca without thinking about it. There is no added risk here.

[lambdageek]

I think we should just use alloca without thinking about it. There is no added risk here.

if the alloca causes a stack overflow in unmanaged code, are we going to get a managed stack overflow exception on the thread? or will it be some native crash?

[BrzVlad]

On some platforms we have stack guard but surely not on all of them. Anyhow, I don't see why it would matter. Any code can potentially stack overflow and we are not causing unreasonable stack increase here. Whatever the size of the valuetype, instances of it would likely still be stored plenty of times on stack frames from managed code. And it's not like the code before wouldn't also stack overflow. The same stack space would still be written when initializing the valuetype, only that now it overflows into other frames making everything even worse.