[runtime] Fix handler block
authorVlad Brezae <brezaevlad@gmail.com>
Mon, 24 Jul 2017 18:08:52 +0000 (21:08 +0300)
committerVlad Brezae <brezaevlad@gmail.com>
Tue, 25 Jul 2017 09:23:34 +0000 (12:23 +0300)
Instead of hijacking the return address when suspending a thread in a finally block for an abort, do an explicit check in managed code when we are returning from the handler in order to see if we need to abort. Hijacking the return address is arch specific and it doesn't work if we are suspended before it is initialized in handler start.

Fixes #5798

mono/metadata/threads-types.h
mono/metadata/threads.c
mono/mini/method-to-ir.c
mono/mini/mini-exceptions.c
mono/mini/mini-runtime.c
mono/mini/mini.c
mono/mini/mini.h

index cf40b5a09d17a412279223672c5e59cdc547aecf..ced0ac516efbf9a8e91c47df9fbef6ceec645bc6 100644 (file)
@@ -214,6 +214,7 @@ void mono_threads_set_shutting_down (void);
 gunichar2* mono_thread_get_name (MonoInternalThread *this_obj, guint32 *name_len);
 
 MONO_API MonoException* mono_thread_get_undeniable_exception (void);
+void mono_thread_self_abort (void);
 
 void mono_thread_set_name_internal (MonoInternalThread *this_obj, MonoString *name, gboolean permanent, gboolean reset, MonoError *error);
 
index 21c6e53c317c9d0be21d916b524ff4c9eb33e76b..97efd32c1cdac04e48296a738ee74cf9a8098346 100644 (file)
@@ -3947,6 +3947,14 @@ mono_threads_abort_appdomain_threads (MonoDomain *domain, int timeout)
        return TRUE;
 }
 
+void
+mono_thread_self_abort (void)
+{
+       MonoError error;
+       self_abort_internal (&error);
+       mono_error_set_pending_exception (&error);
+}
+
 /*
  * mono_thread_get_undeniable_exception:
  *
index c20c6b2bbbf6102bd76c963a00cba4af9ba82c56..a74291148e889e440358f663f257094636ad6807 100644 (file)
@@ -7345,6 +7345,8 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
                        tblock->real_offset = clause->handler_offset;
                        tblock->flags |= BB_EXCEPTION_HANDLER;
 
+                       if (clause->flags == MONO_EXCEPTION_CLAUSE_FINALLY)
+                               mono_create_exvar_for_offset (cfg, clause->handler_offset);
                        /*
                         * Linking the try block with the EH block hinders inlining as we won't be able to 
                         * merge the bblocks from inlining and produce an artificial hole for no good reason.
@@ -11408,18 +11410,35 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
 
                        if ((handlers = mono_find_final_block (cfg, ip, target, MONO_EXCEPTION_CLAUSE_FINALLY))) {
                                GList *tmp;
-                               MonoExceptionClause *clause;
 
                                for (tmp = handlers; tmp; tmp = tmp->next) {
-                                       clause = (MonoExceptionClause *)tmp->data;
+                                       MonoExceptionClause *clause = (MonoExceptionClause *)tmp->data;
+                                       MonoInst *abort_exc = (MonoInst *)mono_find_exvar_for_offset (cfg, clause->handler_offset);
+                                       MonoBasicBlock *dont_throw;
+
                                        tblock = cfg->cil_offset_to_bb [clause->handler_offset];
                                        g_assert (tblock);
                                        link_bblock (cfg, cfg->cbb, tblock);
+
+                                       MONO_EMIT_NEW_PCONST (cfg, abort_exc->dreg, 0);
+
                                        MONO_INST_NEW (cfg, ins, OP_CALL_HANDLER);
                                        ins->inst_target_bb = tblock;
                                        ins->inst_eh_block = clause;
                                        MONO_ADD_INS (cfg->cbb, ins);
                                        cfg->cbb->has_call_handler = 1;
+
+                                       /* Throw exception if exvar is set */
+                                       /* FIXME Do we need this for calls from catch/filter ? */
+                                       NEW_BBLOCK (cfg, dont_throw);
+                                       MONO_EMIT_NEW_BIALU_IMM (cfg, OP_COMPARE_IMM, -1, abort_exc->dreg, 0);
+                                       MONO_EMIT_NEW_BRANCH_BLOCK (cfg, OP_PBEQ, dont_throw);
+                                       mono_emit_jit_icall (cfg, mono_thread_self_abort, NULL);
+                                       cfg->cbb->clause_hole = clause;
+
+                                       MONO_START_BB (cfg, dont_throw);
+                                       cfg->cbb->clause_hole = clause;
+
                                        if (COMPILE_LLVM (cfg)) {
                                                MonoBasicBlock *target_bb;
 
index 47350c4377bef77cf31a49cbc827ae02439db84f..d73c4c8e462dd16dfa9dca540fe95066f2823734 100644 (file)
@@ -120,6 +120,7 @@ static void mono_walk_stack_full (MonoJitStackWalk func, MonoContext *start_ctx,
 static void mono_raise_exception_with_ctx (MonoException *exc, MonoContext *ctx);
 static void mono_runtime_walk_stack_with_ctx (MonoJitStackWalk func, MonoContext *start_ctx, MonoUnwindOptions unwind_options, void *user_data);
 static gboolean mono_current_thread_has_handle_block_guard (void);
+static gboolean mono_install_handler_block_guard (MonoThreadUnwindState *ctx);
 
 static gboolean
 first_managed (MonoStackFrameInfo *frame, MonoContext *ctx, gpointer addr)
@@ -2106,11 +2107,12 @@ mono_handle_exception_internal (MonoContext *ctx, MonoObject *obj, gboolean resu
                                         * that was called by the EH machinery. It won't have a guard trampoline installed, so we must
                                         * check for this situation here and resume interruption if we are below the guarded block.
                                         */
-                                       if (G_UNLIKELY (jit_tls->handler_block_return_address)) {
+                                       if (G_UNLIKELY (jit_tls->handler_block)) {
                                                gboolean is_outside = FALSE;
                                                gpointer prot_bp = MONO_CONTEXT_GET_BP (&jit_tls->handler_block_context);
                                                gpointer catch_bp = MONO_CONTEXT_GET_BP (ctx);
                                                //FIXME make this stack direction aware
+
                                                if (catch_bp > prot_bp) {
                                                        is_outside = TRUE;
                                                } else if (catch_bp == prot_bp) {
@@ -2130,7 +2132,6 @@ mono_handle_exception_internal (MonoContext *ctx, MonoObject *obj, gboolean resu
                                                        }
                                                }
                                                if (is_outside) {
-                                                       jit_tls->handler_block_return_address = NULL;
                                                        jit_tls->handler_block = NULL;
                                                        mono_thread_resume_interruption (TRUE); /*We ignore the exception here, it will be raised later*/
                                                }
@@ -2890,8 +2891,6 @@ mono_resume_unwind (MonoContext *ctx)
        mono_restore_context (&new_ctx);
 }
 
-#ifdef MONO_ARCH_HAVE_HANDLER_BLOCK_GUARD
-
 typedef struct {
        MonoJitInfo *ji;
        MonoContext ctx;
@@ -2928,12 +2927,13 @@ find_last_handler_block (StackFrameInfo *frame, MonoContext *ctx, gpointer data)
 }
 
 
-static gpointer
+static void
 install_handler_block_guard (MonoJitInfo *ji, MonoContext *ctx)
 {
        int i;
        MonoJitExceptionInfo *clause = NULL;
        gpointer ip;
+       guint8 *bp;
 
        ip = MONO_CONTEXT_GET_IP (ctx);
 
@@ -2947,20 +2947,21 @@ install_handler_block_guard (MonoJitInfo *ji, MonoContext *ctx)
 
        /*no matching finally */
        if (i == ji->num_clauses)
-               return NULL;
+               return;
 
-       return mono_arch_install_handler_block_guard (ji, clause, ctx, mono_create_handler_block_trampoline ());
+        /*Load the spvar*/
+        bp = (guint8*)MONO_CONTEXT_GET_BP (ctx);
+        *(bp + clause->exvar_offset) = 1;
 }
 
 /*
  * Finds the bottom handler block running and install a block guard if needed.
  */
-gboolean
+static gboolean
 mono_install_handler_block_guard (MonoThreadUnwindState *ctx)
 {
        FindHandlerBlockData data = { 0 };
        MonoJitTlsData *jit_tls = (MonoJitTlsData *)ctx->unwind_data [MONO_UNWIND_DATA_JIT_TLS];
-       gpointer resume_ip;
 
 #ifndef MONO_ARCH_HAVE_HANDLER_BLOCK_GUARD_AOT
        if (mono_aot_only)
@@ -2970,7 +2971,7 @@ mono_install_handler_block_guard (MonoThreadUnwindState *ctx)
        /* Guard against a null MonoJitTlsData. This can happens if the thread receives the
          * interrupt signal before the JIT has time to initialize its TLS data for the given thread.
         */
-       if (!jit_tls || jit_tls->handler_block_return_address)
+       if (!jit_tls || jit_tls->handler_block)
                return FALSE;
 
        /* Do an async safe stack walk */
@@ -2983,11 +2984,8 @@ mono_install_handler_block_guard (MonoThreadUnwindState *ctx)
 
        memcpy (&jit_tls->handler_block_context, &data.ctx, sizeof (MonoContext));
 
-       resume_ip = install_handler_block_guard (data.ji, &data.ctx);
-       if (resume_ip == NULL)
-               return FALSE;
+       install_handler_block_guard (data.ji, &data.ctx);
 
-       jit_tls->handler_block_return_address = resume_ip;
        jit_tls->handler_block = data.ei;
 
        return TRUE;
@@ -2997,24 +2995,9 @@ static gboolean
 mono_current_thread_has_handle_block_guard (void)
 {
        MonoJitTlsData *jit_tls = (MonoJitTlsData *)mono_tls_get_jit_tls ();
-       return jit_tls && jit_tls->handler_block_return_address != NULL;
-}
-
-#else
-gboolean
-mono_install_handler_block_guard (MonoThreadUnwindState *ctx)
-{
-       return FALSE;
-}
-
-static gboolean
-mono_current_thread_has_handle_block_guard (void)
-{
-       return FALSE;
+       return jit_tls && jit_tls->handler_block != NULL;
 }
 
-#endif
-
 void
 mono_set_cast_details (MonoClass *from, MonoClass *to)
 {
index f3995858181cd1595754e3988592d353dcc9044d..7c22cc941575da3682a32aa27975058995aefc6f 100644 (file)
@@ -4084,6 +4084,7 @@ register_icalls (void)
        register_dyn_icall (mono_get_rethrow_exception (), "mono_arch_rethrow_exception", "void object", TRUE);
        register_dyn_icall (mono_get_throw_corlib_exception (), "mono_arch_throw_corlib_exception", "void ptr", TRUE);
        register_icall (mono_thread_get_undeniable_exception, "mono_thread_get_undeniable_exception", "object", FALSE);
+       register_icall (mono_thread_self_abort, "mono_thread_self_abort", "void", FALSE);
        register_icall (mono_thread_interruption_checkpoint, "mono_thread_interruption_checkpoint", "object", FALSE);
        register_icall (mono_thread_force_interruption_checkpoint_noraise, "mono_thread_force_interruption_checkpoint_noraise", "object", FALSE);
 
index 99f0755dd05679cea2fc4c2f3d52d5b67fd33deb..5374f9a3653704f39cb982c8ea524013044de012 100644 (file)
@@ -2254,6 +2254,9 @@ mono_codegen (MonoCompile *cfg)
                        mono_arch_emit_epilog (cfg);
                        cfg->epilog_end = cfg->code_len;
                }
+
+               if (bb->clause_hole)
+                       mono_cfg_add_try_hole (cfg, bb->clause_hole, cfg->native_code + bb->native_offset, bb);
        }
 
        mono_arch_emit_exceptions (cfg);
@@ -2652,27 +2655,15 @@ create_jit_info (MonoCompile *cfg, MonoMethod *method_to_compile)
                        MonoExceptionClause *ec = &header->clauses [i];
                        MonoJitExceptionInfo *ei = &jinfo->clauses [i];
                        MonoBasicBlock *tblock;
-                       MonoInst *exvar, *spvar;
+                       MonoInst *exvar;
 
                        ei->flags = ec->flags;
 
                        if (G_UNLIKELY (cfg->verbose_level >= 4))
                                printf ("IL clause: try 0x%x-0x%x handler 0x%x-0x%x filter 0x%x\n", ec->try_offset, ec->try_offset + ec->try_len, ec->handler_offset, ec->handler_offset + ec->handler_len, ec->flags == MONO_EXCEPTION_CLAUSE_FILTER ? ec->data.filter_offset : 0);
 
-                       /*
-                        * The spvars are needed by mono_arch_install_handler_block_guard ().
-                        */
-                       if (ei->flags == MONO_EXCEPTION_CLAUSE_FINALLY) {
-                               int region;
-
-                               region = ((i + 1) << 8) | MONO_REGION_FINALLY | ec->flags;
-                               spvar = mono_find_spvar_for_region (cfg, region);
-                               g_assert (spvar);
-                               ei->exvar_offset = spvar->inst_offset;
-                       } else {
-                               exvar = mono_find_exvar_for_offset (cfg, ec->handler_offset);
-                               ei->exvar_offset = exvar ? exvar->inst_offset : 0;
-                       }
+                       exvar = mono_find_exvar_for_offset (cfg, ec->handler_offset);
+                       ei->exvar_offset = exvar ? exvar->inst_offset : 0;
 
                        if (ei->flags == MONO_EXCEPTION_CLAUSE_FILTER) {
                                tblock = cfg->cil_offset_to_bb [ec->data.filter_offset];
index 455929e5ea943115d1eb9a6f5984fbe50b260f48..348c4a5e9583f6cc227a40f2879ebc1cde523780 100644 (file)
@@ -755,6 +755,9 @@ struct MonoBasicBlock {
        /* List of call sites in this bblock sorted by pc_offset */
        GSList *gc_callsites;
 
+       /* If this is not null, the basic block is a try hole for this clause */
+       MonoExceptionClause *clause_hole;
+
        /*
         * The region encodes whether the basic block is inside
         * a finally, catch, filter or none of these.
@@ -1635,10 +1638,18 @@ typedef struct {
 
        /* A hashtable of region ID-> SP var mappings */
        /* An SP var is a place to store the stack pointer (used by handlers)*/
+       /*
+        * FIXME We can potentially get rid of this, since it was mainly used
+        * for hijacking return address for handler.
+        */
        GHashTable      *spvars;
 
-       /* A hashtable of region ID -> EX var mappings */
-       /* An EX var stores the exception object passed to catch/filter blocks */
+       /*
+        * A hashtable of region ID -> EX var mappings
+        * An EX var stores the exception object passed to catch/filter blocks
+        * For finally blocks, it is set to TRUE if we should throw an abort
+        * once the execution of the finally block is over.
+        */
        GHashTable      *exvars;
 
        GList           *ldstr_list; /* used by AOT */
@@ -2857,7 +2868,6 @@ gpointer mono_arch_get_enter_icall_trampoline   (MonoTrampInfo **info);
 gpointer mono_arch_install_handler_block_guard (MonoJitInfo *ji, MonoJitExceptionInfo *clause, MonoContext *ctx, gpointer new_value);
 gpointer mono_arch_create_handler_block_trampoline (MonoTrampInfo **info, gboolean aot);
 gpointer mono_create_handler_block_trampoline (void);
-gboolean mono_install_handler_block_guard (MonoThreadUnwindState *ctx);
 
 /*New interruption machinery */
 void