Merge pull request #2881 from alexrp/gc-sample-managed-alloc

[gc] Register a critical region when executing managed allocators.

Previously, if we stopped a thread and it had a stack looking like this:

    ...
    profiler_signal_handler
    <signal handler called>
    managed_allocator
    ...

We would fail to identify the fact that we've just interrupted a thread that's
in a managed allocator (uninterruptible code) because we only look at the very
latest instruction pointer of the thread, which will be pointing into
profiler_signal_handler or some other function called by it. So we would
happily continue along with the STW process and proceed to doing the actual GC,
where we would see a broken heap.

We could solve this by unwinding the stack and checking all frames, but that's
complicated and error-prone. Instead, register a critical region while the
managed allocator runs. This way, if we don't identify it by instruction pointer, we
will identify it by the fact that the thread is in a critical region.

diff --combined mono/mini/aot-compiler.c
index 57507007c8da5d59201fe58473555954315e593b,28d254d0ac601e4f0b3e1c18b384b584f4d084ba..eabbf626018d03b50b0922d4a6bedbaa3c7eaa48
--- 1/mono/mini/aot-compiler.c
--- 2/mono/mini/aot-compiler.c
+++ b/mono/mini/aot-compiler.c
@@@ -3875,13 -3875,9 +3875,9 @@@ add_wrappers (MonoAotCompile *acfg
                        /* Managed Allocators */
                        nallocators = mono_gc_get_managed_allocator_types ();
                        for (i = 0; i < nallocators; ++i) {
-                               m = mono_gc_get_managed_allocator_by_type (i, TRUE);
-                               if (m)
+                               if ((m = mono_gc_get_managed_allocator_by_type (i, MANAGED_ALLOCATOR_REGULAR)))
                                        add_method (acfg, m);
-                       }
-                       for (i = 0; i < nallocators; ++i) {
-                               m = mono_gc_get_managed_allocator_by_type (i, FALSE);
-                               if (m)
+                               if ((m = mono_gc_get_managed_allocator_by_type (i, MANAGED_ALLOCATOR_SLOW_PATH)))
                                        add_method (acfg, m);
                        }
                }
@@@ -10065,7 -10061,7 +10061,7 @@@ acfg_free (MonoAotCompile *acfg
        g_hash_table_destroy (acfg->image_hash);
        g_hash_table_destroy (acfg->unwind_info_offsets);
        g_hash_table_destroy (acfg->method_label_hash);
 -      if (!acfg->typespec_classes)
 +      if (acfg->typespec_classes)
                g_hash_table_destroy (acfg->typespec_classes);
        g_hash_table_destroy (acfg->export_names);
        g_hash_table_destroy (acfg->plt_entry_debug_sym_cache);

diff --combined mono/mini/aot-runtime.c
index d35294e2b07bb079dba6e263d92f95dd7451022b,b5d7ab9d36b25dd3a7cc51f8a289a8e0d7aa3cab..21468cc25cd6fdb1adf30df2cfbdad1b2b9e7890
--- 1/mono/mini/aot-runtime.c
--- 2/mono/mini/aot-runtime.c
+++ b/mono/mini/aot-runtime.c
@@@ -930,8 -930,11 +930,11 @@@ decode_method_ref_with_target (MonoAotM
  #endif
                case MONO_WRAPPER_ALLOC: {
                        int atype = decode_value (p, &p);
+                       ManagedAllocatorVariant variant =
+                               mono_profiler_get_events () & MONO_PROFILE_ALLOCATIONS ?
+                               MANAGED_ALLOCATOR_SLOW_PATH : MANAGED_ALLOCATOR_REGULAR;
  
-                       ref->method = mono_gc_get_managed_allocator_by_type (atype, !!(mono_profiler_get_events () & MONO_PROFILE_ALLOCATIONS));
+                       ref->method = mono_gc_get_managed_allocator_by_type (atype, variant);
                        if (!ref->method) {
                                mono_error_set_bad_image_name (error, module->aot_name, "Error: No managed allocator, but we need one for AOT.\nAre you using non-standard GC options?\n");
                                return FALSE;
@@@ -4602,7 -4605,7 +4605,7 @@@ mono_aot_get_method (MonoDomain *domain
  
        gpointer res = mono_aot_get_method_checked (domain, method, &error);
        /* This is external only, so its ok to raise here */
 -      mono_error_raise_exception (&error);
 +      mono_error_raise_exception (&error); /* OK to throw, external only without a good alternative */
        return res;
  }
  

diff --combined mono/mini/mini-arm.c
index e8cfe51c5a56b9107815595df3c94c0015940b1f,8ae5fc9481c05110bcb786ff7834854a64378e35..6c31a8ed53c6899e151b149746df19281bdc0ff8
--- 1/mono/mini/mini-arm.c
--- 2/mono/mini/mini-arm.c
+++ b/mono/mini/mini-arm.c
@@@ -1362,25 -1362,10 +1362,25 @@@ get_call_info (MonoMemPool *mp, MonoMet
                        cinfo->ret.nregs = nfields;
                        cinfo->ret.esize = esize;
                } else {
 -                      if (is_pinvoke && mono_class_native_size (mono_class_from_mono_type (t), &align) <= sizeof (gpointer))
 -                              cinfo->ret.storage = RegTypeStructByVal;
 -                      else
 +                      if (is_pinvoke) {
 +                              int native_size = mono_class_native_size (mono_class_from_mono_type (t), &align);
 +                              int max_size;
 +
 +#ifdef TARGET_WATCHOS
 +                              max_size = 16;
 +#else
 +                              max_size = 4;
 +#endif
 +                              if (native_size <= max_size) {
 +                                      cinfo->ret.storage = RegTypeStructByVal;
 +                                      cinfo->ret.struct_size = native_size;
 +                                      cinfo->ret.nregs = ALIGN_TO (native_size, 4) / 4;
 +                              } else {
 +                                      cinfo->ret.storage = RegTypeStructByAddr;
 +                              }
 +                      } else {
                                cinfo->ret.storage = RegTypeStructByAddr;
 +                      }
                }
                break;
        case MONO_TYPE_VAR:
@@@ -1533,27 -1518,6 +1533,27 @@@
                                        size = mini_type_stack_size_full (t, &align, FALSE);
                        }
                        DEBUG(g_print ("load %d bytes struct\n", size));
 +
 +#ifdef TARGET_WATCHOS
 +                      /* Watchos pass large structures by ref */
 +                      /* We only do this for pinvoke to make gsharedvt/dyncall simpler */
 +                      if (sig->pinvoke && size > 16) {
 +                              add_general (&gr, &stack_size, ainfo, TRUE);
 +                              switch (ainfo->storage) {
 +                              case RegTypeGeneral:
 +                                      ainfo->storage = RegTypeStructByAddr;
 +                                      break;
 +                              case RegTypeBase:
 +                                      ainfo->storage = RegTypeStructByAddrOnStack;
 +                                      break;
 +                              default:
 +                                      g_assert_not_reached ();
 +                                      break;
 +                              }
 +                              break;
 +                      }
 +#endif
 +
                        align_size = size;
                        nwords = 0;
                        align_size += (sizeof (gpointer) - 1);
@@@ -1840,16 -1804,21 +1840,16 @@@ mono_arch_allocate_vars (MonoCompile *c
  
        switch (cinfo->ret.storage) {
        case RegTypeStructByVal:
 -              cfg->ret->opcode = OP_REGOFFSET;
 -              cfg->ret->inst_basereg = cfg->frame_reg;
 -              offset += sizeof (gpointer) - 1;
 -              offset &= ~(sizeof (gpointer) - 1);
 -              cfg->ret->inst_offset = - offset;
 -              offset += sizeof(gpointer);
 -              break;
        case RegTypeHFA:
                /* Allocate a local to hold the result, the epilog will copy it to the correct place */
                offset = ALIGN_TO (offset, 8);
                cfg->ret->opcode = OP_REGOFFSET;
                cfg->ret->inst_basereg = cfg->frame_reg;
                cfg->ret->inst_offset = offset;
 -              // FIXME:
 -              offset += 32;
 +              if (cinfo->ret.storage == RegTypeStructByVal)
 +                      offset += cinfo->ret.nregs * sizeof (gpointer);
 +              else
 +                      offset += 32;
                break;
        case RegTypeStructByAddr:
                ins = cfg->vret_addr;
@@@ -2171,13 -2140,6 +2171,13 @@@ mono_arch_get_llvm_call_info (MonoCompi
                linfo->ret.storage = LLVMArgVtypeRetAddr;
                linfo->vret_arg_index = cinfo->vret_arg_index;
                break;
 +#if TARGET_WATCHOS
 +      case RegTypeStructByVal:
 +              /* LLVM models this by returning an int array */
 +              linfo->ret.storage = LLVMArgAsIArgs;
 +              linfo->ret.nslots = cinfo->ret.nregs;
 +              break;
 +#endif
        default:
                cfg->exception_message = g_strdup_printf ("unknown ret conv (%d)", cinfo->ret.storage);
                cfg->disable_llvm = TRUE;
@@@ -2185,10 -2147,9 +2185,10 @@@
        }
  
        for (i = 0; i < n; ++i) {
 +              LLVMArgInfo *lainfo = &linfo->args [i];
                ainfo = cinfo->args + i;
  
 -              linfo->args [i].storage = LLVMArgNone;
 +              lainfo->storage = LLVMArgNone;
  
                switch (ainfo->storage) {
                case RegTypeGeneral:
@@@ -2196,15 -2157,11 +2196,15 @@@
                case RegTypeBase:
                case RegTypeBaseGen:
                case RegTypeFP:
 -                      linfo->args [i].storage = LLVMArgNormal;
 +                      lainfo->storage = LLVMArgNormal;
                        break;
                case RegTypeStructByVal:
 -                      linfo->args [i].storage = LLVMArgAsIArgs;
 -                      linfo->args [i].nslots = ainfo->struct_size / sizeof (gpointer);
 +                      lainfo->storage = LLVMArgAsIArgs;
 +                      lainfo->nslots = ainfo->struct_size / sizeof (gpointer);
 +                      break;
 +              case RegTypeStructByAddr:
 +              case RegTypeStructByAddrOnStack:
 +                      lainfo->storage = LLVMArgVtypeByRef;
                        break;
                default:
                        cfg->exception_message = g_strdup_printf ("ainfo->storage (%d)", ainfo->storage);
@@@ -2232,14 -2189,10 +2232,14 @@@ mono_arch_emit_call (MonoCompile *cfg, 
  
        switch (cinfo->ret.storage) {
        case RegTypeStructByVal:
 -              /* The JIT will transform this into a normal call */
 -              call->vret_in_reg = TRUE;
 -              break;
        case RegTypeHFA:
 +              if (cinfo->ret.storage == RegTypeStructByVal && cinfo->ret.nregs == 1) {
 +                      /* The JIT will transform this into a normal call */
 +                      call->vret_in_reg = TRUE;
 +                      break;
 +              }
 +              if (call->inst.opcode == OP_TAILCALL)
 +                      break;
                /*
                 * The vtype is returned in registers, save the return area address in a local, and save the vtype into
                 * the location pointed to by it after call in emit_move_return_value ().
@@@ -2353,12 -2306,19 +2353,12 @@@
                                mono_call_inst_add_outarg_reg (cfg, call, ins->dreg, ainfo->reg, FALSE);
                        }
                        break;
 -              case RegTypeStructByAddr:
 -                      NOT_IMPLEMENTED;
 -#if 0
 -                      /* FIXME: where si the data allocated? */
 -                      arg->backend.reg3 = ainfo->reg;
 -                      call->used_iregs |= 1 << ainfo->reg;
 -                      g_assert_not_reached ();
 -#endif
 -                      break;
                case RegTypeStructByVal:
                case RegTypeGSharedVtInReg:
                case RegTypeGSharedVtOnStack:
                case RegTypeHFA:
 +              case RegTypeStructByAddr:
 +              case RegTypeStructByAddrOnStack:
                        MONO_INST_NEW (cfg, ins, OP_OUTARG_VT);
                        ins->opcode = OP_OUTARG_VT;
                        ins->sreg1 = in->dreg;
@@@ -2504,12 -2464,10 +2504,12 @@@ mono_arch_emit_outarg_vt (MonoCompile *
  
        switch (ainfo->storage) {
        case RegTypeGSharedVtInReg:
 +      case RegTypeStructByAddr:
                /* Pass by addr */
                mono_call_inst_add_outarg_reg (cfg, call, src->dreg, ainfo->reg, FALSE);
                break;
        case RegTypeGSharedVtOnStack:
 +      case RegTypeStructByAddrOnStack:
                /* Pass by addr on stack */
                MONO_EMIT_NEW_STORE_MEMBASE (cfg, OP_STORE_MEMBASE_REG, ARMREG_SP, ainfo->offset, src->dreg);
                break;
@@@ -4079,16 -4037,10 +4079,16 @@@ emit_move_return_value (MonoCompile *cf
        cinfo = call->call_info;
  
        switch (cinfo->ret.storage) {
 +      case RegTypeStructByVal:
        case RegTypeHFA: {
                MonoInst *loc = cfg->arch.vret_addr_loc;
                int i;
  
 +              if (cinfo->ret.storage == RegTypeStructByVal && cinfo->ret.nregs == 1) {
 +                      /* The JIT treats this as a normal call */
 +                      break;
 +              }
 +
                /* Load the destination address */
                g_assert (loc && loc->opcode == OP_REGOFFSET);
  
@@@ -4098,34 -4050,11 +4098,34 @@@
                        code = mono_arm_emit_load_imm (code, ARMREG_LR, loc->inst_offset);
                        ARM_LDR_REG_REG (code, ARMREG_LR, loc->inst_basereg, ARMREG_LR);
                }
 -              for (i = 0; i < cinfo->ret.nregs; ++i) {
 -                      if (cinfo->ret.esize == 4)
 -                              ARM_FSTS (code, cinfo->ret.reg + i, ARMREG_LR, i * 4);
 -                      else
 -                              ARM_FSTD (code, cinfo->ret.reg + (i * 2), ARMREG_LR, i * 8);
 +
 +              if (cinfo->ret.storage == RegTypeStructByVal) {
 +                      int rsize = cinfo->ret.struct_size;
 +
 +                      for (i = 0; i < cinfo->ret.nregs; ++i) {
 +                              g_assert (rsize >= 0);
 +                              switch (rsize) {
 +                              case 0:
 +                                      break;
 +                              case 1:
 +                                      ARM_STRB_IMM (code, i, ARMREG_LR, i * 4);
 +                                      break;
 +                              case 2:
 +                                      ARM_STRH_IMM (code, i, ARMREG_LR, i * 4);
 +                                      break;
 +                              default:
 +                                      ARM_STR_IMM (code, i, ARMREG_LR, i * 4);
 +                                      break;
 +                              }
 +                              rsize -= 4;
 +                      }
 +              } else {
 +                      for (i = 0; i < cinfo->ret.nregs; ++i) {
 +                              if (cinfo->ret.esize == 4)
 +                                      ARM_FSTS (code, cinfo->ret.reg + i, ARMREG_LR, i * 4);
 +                              else
 +                                      ARM_FSTD (code, cinfo->ret.reg + (i * 2), ARMREG_LR, i * 8);
 +                      }
                }
                return code;
        }
@@@ -4362,7 -4291,8 +4362,8 @@@ mono_arch_output_basic_block (MonoCompi
                                break;
                        }
  
-                       ARM_DMB (code, ARM_DMB_SY);
+                       if (ins->backend.memory_barrier_kind != MONO_MEMORY_BARRIER_NONE)
+                               ARM_DMB (code, ARM_DMB_SY);
                        break;
                }
                case OP_ATOMIC_STORE_I1:
@@@ -4373,7 -4303,8 +4374,8 @@@
                case OP_ATOMIC_STORE_U4:
                case OP_ATOMIC_STORE_R4:
                case OP_ATOMIC_STORE_R8: {
-                       ARM_DMB (code, ARM_DMB_SY);
+                       if (ins->backend.memory_barrier_kind != MONO_MEMORY_BARRIER_NONE)
+                               ARM_DMB (code, ARM_DMB_SY);
  
                        code = mono_arm_emit_load_imm (code, ARMREG_LR, ins->inst_offset);
  
@@@ -6299,7 -6230,6 +6301,7 @@@ mono_arch_emit_prolog (MonoCompile *cfg
                        case RegTypeGeneral:
                        case RegTypeIRegPair:
                        case RegTypeGSharedVtInReg:
 +                      case RegTypeStructByAddr:
                                switch (ainfo->size) {
                                case 1:
                                        if (arm_is_imm12 (inst->inst_offset))
@@@ -6360,7 -6290,6 +6362,7 @@@
                                break;
                        case RegTypeBase:
                        case RegTypeGSharedVtOnStack:
 +                      case RegTypeStructByAddrOnStack:
                                if (arm_is_imm12 (prev_sp_offset + ainfo->offset)) {
                                        ARM_LDR_IMM (code, ARMREG_LR, ARMREG_SP, (prev_sp_offset + ainfo->offset));
                                } else {
@@@ -6453,6 -6382,10 +6455,6 @@@
                                }
                                break;
                        }
 -                      case RegTypeStructByAddr:
 -                              g_assert_not_reached ();
 -                              /* FIXME: handle overrun! with struct sizes not multiple of 4 */
 -                              code = emit_memcpy (code, ainfo->vtsize * sizeof (gpointer), inst->inst_basereg, inst->inst_offset, ainfo->reg, 0);
                        default:
                                g_assert_not_reached ();
                                break;
@@@ -6576,23 -6509,11 +6578,23 @@@ mono_arch_emit_epilog (MonoCompile *cfg
        case RegTypeStructByVal: {
                MonoInst *ins = cfg->ret;
  
 -              if (arm_is_imm12 (ins->inst_offset)) {
 -                      ARM_LDR_IMM (code, ARMREG_R0, ins->inst_basereg, ins->inst_offset);
 +              if (cinfo->ret.nregs == 1) {
 +                      if (arm_is_imm12 (ins->inst_offset)) {
 +                              ARM_LDR_IMM (code, ARMREG_R0, ins->inst_basereg, ins->inst_offset);
 +                      } else {
 +                              code = mono_arm_emit_load_imm (code, ARMREG_LR, ins->inst_offset);
 +                              ARM_LDR_REG_REG (code, ARMREG_R0, ins->inst_basereg, ARMREG_LR);
 +                      }
                } else {
 -                      code = mono_arm_emit_load_imm (code, ARMREG_LR, ins->inst_offset);
 -                      ARM_LDR_REG_REG (code, ARMREG_R0, ins->inst_basereg, ARMREG_LR);
 +                      for (i = 0; i < cinfo->ret.nregs; ++i) {
 +                              int offset = ins->inst_offset + (i * 4);
 +                              if (arm_is_imm12 (offset)) {
 +                                      ARM_LDR_IMM (code, i, ins->inst_basereg, offset);
 +                              } else {
 +                                      code = mono_arm_emit_load_imm (code, ARMREG_LR, offset);
 +                                      ARM_LDR_REG_REG (code, i, ins->inst_basereg, ARMREG_LR);
 +                              }
 +                      }
                }
                break;
        }