Fix infrequent/random crashes on Windows x64 due to use of GC forwarded objects....
authormonojenkins <jo.shields+jenkins@xamarin.com>
Tue, 14 Apr 2020 11:46:55 +0000 (07:46 -0400)
committerGitHub <noreply@github.com>
Tue, 14 Apr 2020 11:46:55 +0000 (13:46 +0200)
Hard to repro and very infrequent crash. Have been analyzing a couple of crash dumps from retail devices getting different crashes related to vtable "corruption" on Windows x64. After some deeper analysis it turns out the object instance has been forwarded by GC (object vtable pointers lowest bit set to 1), but object still holds tagged vtable. This will then cause misaligned reads, getting back random values and pointers from vtable on next object access.

After some further analyzing it turns out that LLVM codegen and some specific generic vt arrays lowering can cause optimized mem copies using XMM registers. I have also identified scenarios where vt copies gets lowered into a c-runtime memcpy that in turn uses XMM registers as an optimization. Since Windows x64 currently don't include XMM registers in context, any references in XMM registers will not be visible and pinned by GC, meaning that they will point to potentially
forwarded objects after completing GC, restarting threads, leading to these infrequent random crashes.

Fix includes xmm0-xmm15 into MonoContext on Windows x64, making sure GC will see all references that could be held in those registers, regardless if getting into those registers due to LLVM optimization or other native code, like memcpy.

Co-authored-by: lateralusX <lateralusX@users.noreply.github.com>
src/mono/mono/mini/mini-windows.c
src/mono/mono/utils/mono-context.c
src/mono/mono/utils/mono-threads-windows.c
src/mono/mono/utils/win64.asm

index 673aeef..eca6011 100644 (file)
@@ -383,16 +383,13 @@ mono_setup_thread_context(DWORD thread_id, MonoContext *mono_context)
        handle = OpenThread (THREAD_ALL_ACCESS, FALSE, thread_id);
        g_assert (handle);
 
-       context.ContextFlags = CONTEXT_INTEGER | CONTEXT_CONTROL;
+       context.ContextFlags = CONTEXT_INTEGER | CONTEXT_FLOATING_POINT | CONTEXT_CONTROL;
 
        if (!GetThreadContext (handle, &context)) {
                CloseHandle (handle);
                return FALSE;
        }
 
-       g_assert (context.ContextFlags & CONTEXT_INTEGER);
-       g_assert (context.ContextFlags & CONTEXT_CONTROL);
-
        memset (mono_context, 0, sizeof (MonoContext));
        mono_sigctx_to_monoctx (&context, mono_context);
 
index 59af925..62b1feb 100644 (file)
@@ -233,6 +233,22 @@ mono_sigctx_to_monoctx (void *sigctx, MonoContext *mctx)
        mctx->gregs [AMD64_R13] = context->R13;
        mctx->gregs [AMD64_R14] = context->R14;
        mctx->gregs [AMD64_R15] = context->R15;
+       memcpy (&(mctx->fregs [AMD64_XMM0]), &(context->Xmm0), sizeof (MonoContextSimdReg));
+       memcpy (&(mctx->fregs [AMD64_XMM1]), &(context->Xmm1), sizeof (MonoContextSimdReg));
+       memcpy (&(mctx->fregs [AMD64_XMM2]), &(context->Xmm2), sizeof (MonoContextSimdReg));
+       memcpy (&(mctx->fregs [AMD64_XMM3]), &(context->Xmm3), sizeof (MonoContextSimdReg));
+       memcpy (&(mctx->fregs [AMD64_XMM4]), &(context->Xmm4), sizeof (MonoContextSimdReg));
+       memcpy (&(mctx->fregs [AMD64_XMM5]), &(context->Xmm5), sizeof (MonoContextSimdReg));
+       memcpy (&(mctx->fregs [AMD64_XMM6]), &(context->Xmm6), sizeof (MonoContextSimdReg));
+       memcpy (&(mctx->fregs [AMD64_XMM7]), &(context->Xmm7), sizeof (MonoContextSimdReg));
+       memcpy (&(mctx->fregs [AMD64_XMM8]), &(context->Xmm8), sizeof (MonoContextSimdReg));
+       memcpy (&(mctx->fregs [AMD64_XMM9]), &(context->Xmm9), sizeof (MonoContextSimdReg));
+       memcpy (&(mctx->fregs [AMD64_XMM10]), &(context->Xmm10), sizeof (MonoContextSimdReg));
+       memcpy (&(mctx->fregs [AMD64_XMM11]), &(context->Xmm11), sizeof (MonoContextSimdReg));
+       memcpy (&(mctx->fregs [AMD64_XMM12]), &(context->Xmm12), sizeof (MonoContextSimdReg));
+       memcpy (&(mctx->fregs [AMD64_XMM13]), &(context->Xmm13), sizeof (MonoContextSimdReg));
+       memcpy (&(mctx->fregs [AMD64_XMM14]), &(context->Xmm14), sizeof (MonoContextSimdReg));
+       memcpy (&(mctx->fregs [AMD64_XMM15]), &(context->Xmm15), sizeof (MonoContextSimdReg));
 #elif defined(__HAIKU__)
        // Haiku uses sigcontext because there's no ucontext
        struct sigcontext *ctx = (struct sigcontext *)sigctx;
@@ -326,6 +342,22 @@ mono_monoctx_to_sigctx (MonoContext *mctx, void *sigctx)
        context->R13 = mctx->gregs [AMD64_R13];
        context->R14 = mctx->gregs [AMD64_R14];
        context->R15 = mctx->gregs [AMD64_R15];
+       memcpy (&(context->Xmm0), &(mctx->fregs [AMD64_XMM0]), sizeof (MonoContextSimdReg));
+       memcpy (&(context->Xmm1), &(mctx->fregs [AMD64_XMM1]), sizeof (MonoContextSimdReg));
+       memcpy (&(context->Xmm2), &(mctx->fregs [AMD64_XMM2]), sizeof (MonoContextSimdReg));
+       memcpy (&(context->Xmm3), &(mctx->fregs [AMD64_XMM3]), sizeof (MonoContextSimdReg));
+       memcpy (&(context->Xmm4), &(mctx->fregs [AMD64_XMM4]), sizeof (MonoContextSimdReg));
+       memcpy (&(context->Xmm5), &(mctx->fregs [AMD64_XMM5]), sizeof (MonoContextSimdReg));
+       memcpy (&(context->Xmm6), &(mctx->fregs [AMD64_XMM6]), sizeof (MonoContextSimdReg));
+       memcpy (&(context->Xmm7), &(mctx->fregs [AMD64_XMM7]), sizeof (MonoContextSimdReg));
+       memcpy (&(context->Xmm8), &(mctx->fregs [AMD64_XMM8]), sizeof (MonoContextSimdReg));
+       memcpy (&(context->Xmm9), &(mctx->fregs [AMD64_XMM9]), sizeof (MonoContextSimdReg));
+       memcpy (&(context->Xmm10), &(mctx->fregs [AMD64_XMM10]), sizeof (MonoContextSimdReg));
+       memcpy (&(context->Xmm11), &(mctx->fregs [AMD64_XMM11]), sizeof (MonoContextSimdReg));
+       memcpy (&(context->Xmm12), &(mctx->fregs [AMD64_XMM12]), sizeof (MonoContextSimdReg));
+       memcpy (&(context->Xmm13), &(mctx->fregs [AMD64_XMM13]), sizeof (MonoContextSimdReg));
+       memcpy (&(context->Xmm14), &(mctx->fregs [AMD64_XMM14]), sizeof (MonoContextSimdReg));
+       memcpy (&(context->Xmm15), &(mctx->fregs [AMD64_XMM15]), sizeof (MonoContextSimdReg));
 #elif defined(__HAIKU__)
        // Haiku uses sigcontext because there's no ucontext
        struct sigcontext *ctx = (struct sigcontext *)sigctx;
index 378c2a9..8e94bf2 100644 (file)
@@ -200,7 +200,7 @@ mono_threads_suspend_begin_async_suspend (MonoThreadInfo *info, gboolean interru
        /* suspended request, this will wait until thread is suspended and thread context has been collected */
        /* and returned. */
        CONTEXT context;
-       context.ContextFlags = CONTEXT_INTEGER | CONTEXT_CONTROL;
+       context.ContextFlags = CONTEXT_INTEGER | CONTEXT_FLOATING_POINT | CONTEXT_CONTROL;
        if (!GetThreadContext (handle, &context)) {
                result = ResumeThread (handle);
                g_assert (result == 1);
@@ -289,19 +289,16 @@ mono_threads_suspend_begin_async_resume (MonoThreadInfo *info)
                info->async_target = NULL;
                info->user_data = NULL;
 
-               context.ContextFlags = CONTEXT_INTEGER | CONTEXT_CONTROL;
+               context.ContextFlags = CONTEXT_INTEGER | CONTEXT_FLOATING_POINT | CONTEXT_CONTROL;
 
                if (!GetThreadContext (handle, &context)) {
                        THREADS_SUSPEND_DEBUG ("RESUME FAILED (GetThreadContext), id=%p, err=%u\n", GUINT_TO_POINTER (mono_thread_info_get_tid (info)), GetLastError ());
                        return FALSE;
                }
 
-               g_assert (context.ContextFlags & CONTEXT_INTEGER);
-               g_assert (context.ContextFlags & CONTEXT_CONTROL);
-
                mono_monoctx_to_sigctx (&ctx, &context);
 
-               context.ContextFlags = CONTEXT_INTEGER | CONTEXT_CONTROL;
+               context.ContextFlags = CONTEXT_INTEGER | CONTEXT_FLOATING_POINT | CONTEXT_CONTROL;
                res = SetThreadContext (handle, &context);
                if (!res) {
                        THREADS_SUSPEND_DEBUG ("RESUME FAILED (SetThreadContext), id=%p, err=%u\n", GUINT_TO_POINTER (mono_thread_info_get_tid (info)), GetLastError ());
index 7f23a04..669e96a 100644 (file)
@@ -36,6 +36,23 @@ mono_context_get_current PROC
        mov rax, qword ptr [rsp]
        mov [rcx + 80h], rax
 
+       movaps xmmword ptr [rcx + 90h], xmm0
+       movaps xmmword ptr [rcx + 0A0h], xmm1
+       movaps xmmword ptr [rcx + 0B0h], xmm2
+       movaps xmmword ptr [rcx + 0C0h], xmm3
+       movaps xmmword ptr [rcx + 0D0h], xmm4
+       movaps xmmword ptr [rcx + 0E0h], xmm5
+       movaps xmmword ptr [rcx + 0F0h], xmm6
+       movaps xmmword ptr [rcx + 100h], xmm7
+       movaps xmmword ptr [rcx + 110h], xmm8
+       movaps xmmword ptr [rcx + 120h], xmm9
+       movaps xmmword ptr [rcx + 130h], xmm10
+       movaps xmmword ptr [rcx + 140h], xmm11
+       movaps xmmword ptr [rcx + 150h], xmm12
+       movaps xmmword ptr [rcx + 160h], xmm13
+       movaps xmmword ptr [rcx + 170h], xmm14
+       movaps xmmword ptr [rcx + 180h], xmm15
+
        ret
 
 mono_context_get_current endP