Welcome to mirror list, hosted at ThFree Co, Russian Federation.

github.com/mono/mono.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBernhard Urban-Forster <lewurm@gmail.com>2019-11-27 14:20:47 +0300
committerGitHub <noreply@github.com>2019-11-27 14:20:47 +0300
commit690af29ad904c2005f3d9387d7f875571fcf0fcd (patch)
tree4fd9ea33126f0b6cf50822bb3dde0c5885fbb2a3
parentc4136ebd94567bb29557c94e7480e24fe72ffdac (diff)
[amd64] do not stack allocate on the application stack for the transtion from alstack handling (#17922)
### Backstory This test is failing in the interpreter (ONLY on Debian9/amd64. Not Ubuntu/amd64. Not macOS/amd64): https://github.com/mono/mono/blob/0fed03ed63ed4ea742c4511d8edc3bc1c6f4044f/mcs/class/Mono.Debugger.Soft/Test/dtest.cs#L2484-L2499 Where the debuggee is just some unsafe code that tries to read from unmapped memory, thus causing a segfault. So the test expects the runtime to send a CRASH event via the managed debugger interface. This happens very late in the crash handling machinery: https://github.com/mono/mono/blob/0fed03ed63ed4ea742c4511d8edc3bc1c6f4044f/mono/mini/mini-posix.c#L1105 However, the runtime would never reach that, because it silently crashes when calling `backtrace`: https://github.com/mono/mono/blob/0fed03ed63ed4ea742c4511d8edc3bc1c6f4044f/mono/mini/mini-posix.c#L939 At the point where the SIGSEGV happens, the native stack trace looks like this: ``` Thread 1 "mono-sgen" received signal SIGSEGV, Segmentation fault. interp_exec_method_full (frame=0x555555c51f18, context=0x555555c38350, clause_args=clause_args@entry=0x0, error=0x555555c53f18, error@entry=0x7fffffffd8b0) at interp/interp.c:4463 4463 sp[-1].data.i = *(gint32*)sp[-1].data.p; (gdb) bt %0 interp_exec_method_full (frame=0x555555c51f18, context=0x555555c38350, clause_args=clause_args@entry=0x0, error=0x555555c53f18, error@entry=0x7fffffffd8b0) at interp/interp.c:4463 %1 0x000055555569237a in interp_runtime_invoke (method=<optimized out>, obj=<optimized out>, params=<optimized out>, exc=0x0, error=0x7fffffffd8b0) at interp/interp.c:1900 %2 0x0000555555590745 in mono_jit_runtime_invoke (method=0x555555bfdc38, obj=<optimized out>, params=0x7fffffffd858, exc=0x0, error=0x7fffffffd8b0) at mini-runtime.c:3001 %3 0x0000555555778c4c in do_runtime_invoke (method=0x555555bfdc38, obj=<optimized out>, params=<optimized out>, exc=<optimized out>, error=0x7fffffffd8b0) at object.c:3052 %4 0x000055555577b7d0 in do_exec_main_checked (method=0x555555bfdc38, args=<optimized out>, error=0x7fffffffd8b0) at object.c:5184 %5 0x000055555559a2aa in mono_jit_exec_internal (argv=0x7fffffffdcb8, argc=1, assembly=0x0, domain=0x555555bf9d40) at driver.c:1320 %6 mono_jit_exec (domain=domain@entry=0x555555bf9d40, assembly=assembly@entry=0x555555c66ae0, argc=argc@entry=1, argv=argv@entry=0x7fffffffdcb8) at driver.c:1265 %7 0x000055555559b8e2 in main_thread_handler (user_data=<synthetic pointer>) at driver.c:1402 %8 mono_main (argc=<optimized out>, argv=<optimized out>) at driver.c:2622 %9 0x000055555558b757 in mono_main_with_options (argv=<optimized out>, argc=<optimized out>) at main.c:52 %10 main (argc=<optimized out>, argv=<optimized out>) at main.c:434 ``` Stepping into the crash handling machinery, the picture looks a bit different: ``` Thread 1 "mono-sgen" hit Breakpoint 1, altstack_handle_and_restore (ctx=0x7fffffffd2d0, obj=0x0, flags=0) at exceptions-amd64.c:871 871 { (gdb) bt %0 altstack_handle_and_restore (ctx=0x7fffffffd2d0, obj=0x0, flags=0) at exceptions-amd64.c:871 %1 0x00005555556957de in interp_exec_method_full (frame=0x555555c53f08, context=0x555555c38350, clause_args=0x0, error=0x555555c53f18) at interp/interp.c:4461 %2 0x0000000000000000 in ?? () ``` So it looks like we messed up in the transition from altstack back to the program stack. ### The fix The problem is that `backtrace` consults the unwind information emitted at the point where SIGSEGV happens in `interp_exec_method_full ()`. However at that given program code, there is no function call that allocates space to pass arguments on the stack. Still, we massage the stack to do that in the signal handler, which does not add up with the emitted unwind information by the C compiler. Note that without the fix, crashing in `backtrace` is the worst case. In other cases we might (1) get still a correct stack trace, or (2) we get a wrong stack trace, and `backtrace` will not crash. So this PR might improve the crash experience overall. This also reverts commit cff400f3fae9775ac734f8a691339e59a272872c.
-rw-r--r--mcs/class/Mono.Debugger.Soft/Test/dtest.cs1
-rw-r--r--mono/mini/exceptions-amd64.c39
2 files changed, 22 insertions, 18 deletions
diff --git a/mcs/class/Mono.Debugger.Soft/Test/dtest.cs b/mcs/class/Mono.Debugger.Soft/Test/dtest.cs
index 599d3952e63..52da50a45db 100644
--- a/mcs/class/Mono.Debugger.Soft/Test/dtest.cs
+++ b/mcs/class/Mono.Debugger.Soft/Test/dtest.cs
@@ -2476,7 +2476,6 @@ public class DebuggerTests
[Test]
[Category("NotOnWindows")]
[Category ("AndroidSdksNotWorking")]
- [Category ("NotWorkingRuntimeInterpreter")] /* See https://github.com/mono/mono/commit/0a90cf4303f8bea334bf826155b86a7269c61373 */
public void Crash () {
string [] existingCrashFileEntries = Directory.GetFiles (".", "mono_crash*.json");
diff --git a/mono/mini/exceptions-amd64.c b/mono/mini/exceptions-amd64.c
index afa1586527d..61c0f516e83 100644
--- a/mono/mini/exceptions-amd64.c
+++ b/mono/mini/exceptions-amd64.c
@@ -898,34 +898,39 @@ mono_arch_handle_altstack_exception (void *sigctx, MONO_SIG_HANDLER_INFO_TYPE *s
#if defined(MONO_ARCH_USE_SIGACTION)
MonoException *exc = NULL;
gpointer *sp;
- int frame_size;
- MonoContext *copied_ctx;
+ MonoJitTlsData *jit_tls = NULL;
+ MonoContext *copied_ctx = NULL;
gboolean nullref = TRUE;
+ jit_tls = mono_tls_get_jit_tls ();
+ g_assert (jit_tls);
+
+ /* use TLS as temporary storage as we want to avoid
+ * (1) stack allocation on the application stack
+ * (2) calling malloc, because it is not async-safe
+ * (3) using a global storage, because this function is not reentrant
+ *
+ * tls->orig_ex_ctx is used by the stack walker, which shouldn't be running at this point.
+ */
+ copied_ctx = &jit_tls->orig_ex_ctx;
+
if (!mono_is_addr_implicit_null_check (fault_addr))
nullref = FALSE;
if (stack_ovf)
exc = mono_domain_get ()->stack_overflow_ex;
- /* setup a call frame on the real stack so that control is returned there
- * and exception handling can continue.
- * The frame looks like:
- * ucontext struct
- * ...
- * return ip
- * 128 is the size of the red zone
+ /* setup the call frame on the application stack so that control is
+ * returned there and exception handling can continue. we want the call
+ * frame to be minimal as possible, for example no argument passing that
+ * requires allocation on the stack, as this wouldn't be encoded in unwind
+ * information for the caller frame.
*/
- frame_size = sizeof (MonoContext) + sizeof (gpointer) * 4 + 128;
- frame_size += 15;
- frame_size &= ~15;
- sp = (gpointer *)(UCONTEXT_REG_RSP (sigctx) & ~15);
- sp = (gpointer *)((char*)sp - frame_size);
- copied_ctx = (MonoContext*)(sp + 4);
- /* the arguments must be aligned */
+ sp = (gpointer *)UCONTEXT_REG_RSP (sigctx);
+ g_assertf (((unsigned long) sp & 15) == 0, "sp: %p\n", sp);
sp [-1] = (gpointer)UCONTEXT_REG_RIP (sigctx);
mono_sigctx_to_monoctx (sigctx, copied_ctx);
- /* at the return form the signal handler execution starts in altstack_handle_and_restore() */
+ /* at the return from the signal handler execution starts in altstack_handle_and_restore() */
UCONTEXT_REG_RIP (sigctx) = (unsigned long)altstack_handle_and_restore;
UCONTEXT_REG_RSP (sigctx) = (unsigned long)(sp - 1);
UCONTEXT_REG_RDI (sigctx) = (unsigned long)(copied_ctx);