[runtime] Add write barriers for FindFirstFile/FindNextFile icalls (#4114)

We were seeing crashes like [1] or [2] after https://github.com/mono/mono/pull/4042 was merged.
While debugging the issue I found out that when instead of using `out data.cFileName` for passing
the string through the the icall I use a temporary field and assign the result to `data` later on
the crash went away. It also worked if I made the `WIN32_FIND_DATA` class a struct instead.

I talked to @ludovic-henry and he was able to figure out what was going on (many thanks!):

> It seems to be a missing wbarrier
> and that would also explains why it would work with a struct, but not with a class
> so it would work with a struct, because `out data.filename` would be a `MonoString**` to the thread stack,
> and that would work because we would always scan the thread stack
> and it wouldn’t work with a class, because `out data.filename` would be a `MonoString**` to the heap
>
> and because we have a generational GC, if `data` is in the “old” heap (and because we are missing this write barrier),
> we would never scan `data`, thus freeing the `data.filename` string object, and as soon as you are trying to access it,
> well you access garbage
>
> with the write barrier, we will make sure to scan the `data` object, and not loose the reference to `data.filename`

[1] https://jenkins.mono-project.com/job/test-mono-mainline/label=osx-amd64/5422/parsed_console/log_content.html#WARNING1
[2] https://jenkins.mono-project.com/job/test-mono-mainline/label=osx-amd64/5419/parsed_console/log_content.html#WARNING1
[3] https://github.com/mono/mono/blob/dd88ff52e2f130ee188661a34794aa7c7b564fa8/mcs/class/referencesource/mscorlib/system/io/filesystemenumerable.cs#L280

diff --git a/mono/metadata/file-io.c b/mono/metadata/file-io.c
index 5f1ce7efe4738f0e4a5c01878c1a845e09bc0b36..44fde40538cec2ac87acaa2d8a4f9468bc492daa 100644 (file)
--- a/mono/metadata/file-io.c
+++ b/mono/metadata/file-io.c
@@ -475,7 +475,7 @@ ves_icall_System_IO_MonoIO_FindFirstFile (MonoString *path_with_pattern, MonoStr
                return hnd;
        }
 
-       *file_name = mono_string_from_utf16_checked (data.cFileName, &error);
+       mono_gc_wbarrier_generic_store (file_name, (MonoObject*) mono_string_from_utf16_checked (data.cFileName, &error));
        mono_error_set_pending_exception (&error);
 
        *file_attr = data.dwFileAttributes;
@@ -500,7 +500,7 @@ ves_icall_System_IO_MonoIO_FindNextFile (HANDLE hnd, MonoString **file_name, gin
                return res;
        }
 
-       *file_name = mono_string_from_utf16_checked (data.cFileName, &error);
+       mono_gc_wbarrier_generic_store (file_name, (MonoObject*) mono_string_from_utf16_checked (data.cFileName, &error));
        mono_error_set_pending_exception (&error);
 
        *file_attr = data.dwFileAttributes;