Skip to content

Commit

Permalink
Fix infrequent/random crashes on Windows x64 due to use of GC forward…
Browse files Browse the repository at this point in the history 
…ed objects. (#34694)

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 <[email protected]>
  • Loading branch information
@monojenkins @lateralusX
monojenkins and lateralusX authored Apr 14, 2020
1 parent 747df47 commit ad8e751
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 10 deletions.
5 changes: 1 addition & 4 deletions src/mono/mono/mini/mini-windows.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
32 changes: 32 additions & 0 deletions src/mono/mono/utils/mono-context.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
9 changes: 3 additions & 6 deletions src/mono/mono/utils/mono-threads-windows.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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 ());
Expand Down
17 changes: 17 additions & 0 deletions src/mono/mono/utils/win64.asm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit ad8e751

Please sign in to comment.