From 755056dd25052d1105aa64707775d60ca82ac62f Mon Sep 17 00:00:00 2001 From: =?utf8?q?Alex=20R=C3=B8nne=20Petersen?= Date: Thu, 14 Apr 2016 04:12:57 +0200 Subject: [PATCH] [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 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. 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. --- mono/cil/cil-opcodes.xml | 1 + mono/cil/opcode.def | 1 + mono/metadata/boehm-gc.c | 11 ++++-- mono/metadata/gc-internals.h | 9 ++++- mono/metadata/null-gc.c | 2 +- mono/metadata/object-offsets.h | 5 ++- mono/metadata/sgen-client-mono.h | 11 ++++++ mono/metadata/sgen-mono.c | 63 ++++++++++++++++++++++++++------ mono/mini/aot-compiler.c | 8 +--- mono/mini/aot-runtime.c | 5 ++- mono/mini/method-to-ir.c | 16 ++++++++ mono/sgen/sgen-alloc.c | 10 ++++- mono/utils/mono-tls.h | 3 +- 13 files changed, 118 insertions(+), 27 deletions(-) diff --git a/mono/cil/cil-opcodes.xml b/mono/cil/cil-opcodes.xml index 6cf7a6ee484..087e3f30087 100644 --- a/mono/cil/cil-opcodes.xml +++ b/mono/cil/cil-opcodes.xml @@ -318,4 +318,5 @@ + diff --git a/mono/cil/opcode.def b/mono/cil/opcode.def index 620289b0595..ee69b699ba6 100644 --- a/mono/cil/opcode.def +++ b/mono/cil/opcode.def @@ -318,6 +318,7 @@ OPDEF(CEE_MONO_LDPTR_NURSERY_START, "mono_ldptr_nursery_start", Pop0, PushI, Inl OPDEF(CEE_MONO_LDPTR_NURSERY_BITS, "mono_ldptr_nursery_bits", Pop0, PushI, InlineNone, X, 2, 0xF0, 0x17, NEXT) OPDEF(CEE_MONO_CALLI_EXTRA_ARG, "mono_calli_extra_arg", VarPop, VarPush, InlineSig, X, 2, 0xF0, 0x18, CALL) OPDEF(CEE_MONO_LDDOMAIN, "mono_lddomain", Pop0, PushI, InlineNone, X, 2, 0xF0, 0x19, NEXT) +OPDEF(CEE_MONO_ATOMIC_STORE_I4, "mono_atomic_store_i4", PopI+PopI, Push0, InlineI, X, 2, 0xF0, 0x1A, NEXT) #ifndef OPALIAS #define _MONO_CIL_OPALIAS_DEFINED_ #define OPALIAS(a,s,r) diff --git a/mono/metadata/boehm-gc.c b/mono/metadata/boehm-gc.c index 43844ad9830..e76a3a3bfb7 100644 --- a/mono/metadata/boehm-gc.c +++ b/mono/metadata/boehm-gc.c @@ -1125,7 +1125,9 @@ mono_gc_get_managed_allocator (MonoClass *klass, gboolean for_box, gboolean know return NULL; if (!SMALL_ENOUGH (klass->instance_size)) return NULL; - if (mono_class_has_finalizer (klass) || mono_class_is_marshalbyref (klass) || (mono_profiler_get_events () & MONO_PROFILE_ALLOCATIONS)) + if (mono_class_has_finalizer (klass) || mono_class_is_marshalbyref (klass)) + return NULL; + if (mono_profiler_get_events () & (MONO_PROFILE_ALLOCATIONS | MONO_PROFILE_STATISTICAL)) return NULL; if (klass->rank) return NULL; @@ -1151,7 +1153,7 @@ mono_gc_get_managed_allocator (MonoClass *klass, gboolean for_box, gboolean know atype = ATYPE_NORMAL; */ } - return mono_gc_get_managed_allocator_by_type (atype, FALSE); + return mono_gc_get_managed_allocator_by_type (atype, MANAGED_ALLOCATOR_REGULAR); } MonoMethod* @@ -1166,10 +1168,11 @@ mono_gc_get_managed_array_allocator (MonoClass *klass) * Return a managed allocator method corresponding to allocator type ATYPE. */ MonoMethod* -mono_gc_get_managed_allocator_by_type (int atype, gboolean slowpath) +mono_gc_get_managed_allocator_by_type (int atype, ManagedAllocatorVariant variant) { int offset = -1; MonoMethod *res; + gboolean slowpath = variant != MANAGED_ALLOCATOR_REGULAR; MonoMethod **cache = slowpath ? slowpath_alloc_method_cache : alloc_method_cache; MONO_THREAD_VAR_OFFSET (GC_thread_tls, offset); @@ -1226,7 +1229,7 @@ mono_gc_get_managed_array_allocator (MonoClass *klass) } MonoMethod* -mono_gc_get_managed_allocator_by_type (int atype, gboolean slowpath) +mono_gc_get_managed_allocator_by_type (int atype, ManagedAllocatorVariant variant) { return NULL; } diff --git a/mono/metadata/gc-internals.h b/mono/metadata/gc-internals.h index ff44969a81d..82e013b09cd 100644 --- a/mono/metadata/gc-internals.h +++ b/mono/metadata/gc-internals.h @@ -189,10 +189,17 @@ void mono_gc_finalize_threadpool_threads (void); /* fast allocation support */ +typedef enum { + // Regular fast path allocator. + MANAGED_ALLOCATOR_REGULAR, + // Managed allocator that just calls into the runtime. Used when allocation profiling w/ AOT. + MANAGED_ALLOCATOR_SLOW_PATH, +} ManagedAllocatorVariant; + int mono_gc_get_aligned_size_for_allocator (int size); MonoMethod* mono_gc_get_managed_allocator (MonoClass *klass, gboolean for_box, gboolean known_instance_size); MonoMethod* mono_gc_get_managed_array_allocator (MonoClass *klass); -MonoMethod *mono_gc_get_managed_allocator_by_type (int atype, gboolean slowpath); +MonoMethod *mono_gc_get_managed_allocator_by_type (int atype, ManagedAllocatorVariant variant); guint32 mono_gc_get_managed_allocator_types (void); diff --git a/mono/metadata/null-gc.c b/mono/metadata/null-gc.c index 7f904f3dec7..f987e6c3483 100644 --- a/mono/metadata/null-gc.c +++ b/mono/metadata/null-gc.c @@ -321,7 +321,7 @@ mono_gc_get_managed_array_allocator (MonoClass *klass) } MonoMethod* -mono_gc_get_managed_allocator_by_type (int atype, gboolean slowpath) +mono_gc_get_managed_allocator_by_type (int atype, ManagedAllocatorVariant variant) { return NULL; } diff --git a/mono/metadata/object-offsets.h b/mono/metadata/object-offsets.h index c032bc0ff58..34d9ffc1df1 100644 --- a/mono/metadata/object-offsets.h +++ b/mono/metadata/object-offsets.h @@ -129,10 +129,13 @@ DECL_OFFSET(MonoTypedRef, value) DECL_OFFSET(MonoThreadsSync, status) DECL_OFFSET(MonoThreadsSync, nest) -#if defined (HAVE_SGEN_GC) && !defined (HAVE_KW_THREAD) +#ifdef HAVE_SGEN_GC +DECL_OFFSET(SgenClientThreadInfo, in_critical_region) +#ifndef HAVE_KW_THREAD DECL_OFFSET(SgenThreadInfo, tlab_next_addr) DECL_OFFSET(SgenThreadInfo, tlab_temp_end) #endif +#endif #endif //DISABLE METADATA OFFSETS diff --git a/mono/metadata/sgen-client-mono.h b/mono/metadata/sgen-client-mono.h index 8641db0542d..332ed1df8f6 100644 --- a/mono/metadata/sgen-client-mono.h +++ b/mono/metadata/sgen-client-mono.h @@ -728,6 +728,17 @@ extern MonoNativeTlsKey thread_info_key; */ #define EXIT_CRITICAL_REGION do { mono_atomic_store_release (&IN_CRITICAL_REGION, 0); } while (0) +#ifndef DISABLE_CRITICAL_REGION +/* + * We can only use a critical region in the managed allocator if the JIT supports OP_ATOMIC_STORE_I4. + * + * TODO: Query the JIT instead of this ifdef hack. + */ +#if defined (TARGET_X86) || defined (TARGET_AMD64) || (defined (TARGET_ARM) && defined (HAVE_ARMV7)) || defined (TARGET_ARM64) +#define MANAGED_ALLOCATOR_CAN_USE_CRITICAL_REGION +#endif +#endif + #define SGEN_TV_DECLARE(name) gint64 name #define SGEN_TV_GETTIME(tv) tv = mono_100ns_ticks () #define SGEN_TV_ELAPSED(start,end) ((gint64)(end-start)) diff --git a/mono/metadata/sgen-mono.c b/mono/metadata/sgen-mono.c index aaece7e29ca..08e41cb1db4 100644 --- a/mono/metadata/sgen-mono.c +++ b/mono/metadata/sgen-mono.c @@ -993,6 +993,13 @@ static gboolean use_managed_allocator = TRUE; #ifdef HAVE_KW_THREAD +#define EMIT_TLS_ACCESS_IN_CRITICAL_REGION_ADDR(mb) \ + do { \ + mono_mb_emit_byte ((mb), MONO_CUSTOM_PREFIX); \ + mono_mb_emit_byte ((mb), CEE_MONO_TLS); \ + mono_mb_emit_i4 ((mb), TLS_KEY_SGEN_IN_CRITICAL_REGION_ADDR); \ + } while (0) + #define EMIT_TLS_ACCESS_NEXT_ADDR(mb) do { \ mono_mb_emit_byte ((mb), MONO_CUSTOM_PREFIX); \ mono_mb_emit_byte ((mb), CEE_MONO_TLS); \ @@ -1008,6 +1015,15 @@ static gboolean use_managed_allocator = TRUE; #else #if defined(TARGET_OSX) || defined(TARGET_WIN32) || defined(TARGET_ANDROID) || defined(TARGET_IOS) +#define EMIT_TLS_ACCESS_IN_CRITICAL_REGION_ADDR(mb) \ + do { \ + mono_mb_emit_byte ((mb), MONO_CUSTOM_PREFIX); \ + mono_mb_emit_byte ((mb), CEE_MONO_TLS); \ + mono_mb_emit_i4 ((mb), TLS_KEY_SGEN_THREAD_INFO); \ + mono_mb_emit_icon ((mb), MONO_STRUCT_OFFSET (SgenClientThreadInfo, in_critical_region)); \ + mono_mb_emit_byte ((mb), CEE_ADD); \ + } while (0) + #define EMIT_TLS_ACCESS_NEXT_ADDR(mb) do { \ mono_mb_emit_byte ((mb), MONO_CUSTOM_PREFIX); \ mono_mb_emit_byte ((mb), CEE_MONO_TLS); \ @@ -1029,6 +1045,7 @@ static gboolean use_managed_allocator = TRUE; #else #define EMIT_TLS_ACCESS_NEXT_ADDR(mb) do { g_error ("sgen is not supported when using --with-tls=pthread.\n"); } while (0) #define EMIT_TLS_ACCESS_TEMP_END(mb) do { g_error ("sgen is not supported when using --with-tls=pthread.\n"); } while (0) +#define EMIT_TLS_ACCESS_IN_CRITICAL_REGION_ADDR(mb) do { g_error ("sgen is not supported when using --with-tls=pthread.\n"); } while (0) #endif #endif @@ -1042,9 +1059,10 @@ static gboolean use_managed_allocator = TRUE; * that they are executed atomically via the restart mechanism. */ static MonoMethod* -create_allocator (int atype, gboolean slowpath) +create_allocator (int atype, ManagedAllocatorVariant variant) { int p_var, size_var; + gboolean slowpath = variant == MANAGED_ALLOCATOR_SLOW_PATH; guint32 slowpath_branch, max_size_branch; MonoMethodBuilder *mb; MonoMethod *res; @@ -1116,6 +1134,14 @@ create_allocator (int atype, gboolean slowpath) goto done; } +#ifdef MANAGED_ALLOCATOR_CAN_USE_CRITICAL_REGION + EMIT_TLS_ACCESS_IN_CRITICAL_REGION_ADDR (mb); + mono_mb_emit_byte (mb, CEE_LDC_I4_1); + mono_mb_emit_byte (mb, MONO_CUSTOM_PREFIX); + mono_mb_emit_byte (mb, CEE_MONO_ATOMIC_STORE_I4); + mono_mb_emit_i4 (mb, MONO_MEMORY_BARRIER_NONE); +#endif + size_var = mono_mb_add_local (mb, &mono_defaults.int_class->byval_arg); if (atype == ATYPE_SMALL) { /* size_var = size_arg */ @@ -1350,11 +1376,18 @@ create_allocator (int atype, gboolean slowpath) mono_mb_emit_byte (mb, MONO_CEE_STIND_I4); } +#ifdef MANAGED_ALLOCATOR_CAN_USE_CRITICAL_REGION + EMIT_TLS_ACCESS_IN_CRITICAL_REGION_ADDR (mb); + mono_mb_emit_byte (mb, CEE_LDC_I4_0); + mono_mb_emit_byte (mb, MONO_CUSTOM_PREFIX); + mono_mb_emit_byte (mb, CEE_MONO_ATOMIC_STORE_I4); +#else + mono_mb_emit_byte (mb, MONO_CUSTOM_PREFIX); + mono_mb_emit_byte (mb, CEE_MONO_MEMORY_BARRIER); +#endif /* We must make sure both vtable and max_length are globaly visible before returning to managed land. */ - mono_mb_emit_byte (mb, MONO_CUSTOM_PREFIX); - mono_mb_emit_byte (mb, CEE_MONO_MEMORY_BARRIER); mono_mb_emit_i4 (mb, MONO_MEMORY_BARRIER_REL); /* return p */ @@ -1403,17 +1436,19 @@ mono_gc_get_managed_allocator (MonoClass *klass, gboolean for_box, gboolean know return NULL; if (known_instance_size && ALIGN_TO (klass->instance_size, SGEN_ALLOC_ALIGN) >= SGEN_MAX_SMALL_OBJ_SIZE) return NULL; - if (mono_class_has_finalizer (klass) || mono_class_is_marshalbyref (klass) || (mono_profiler_get_events () & MONO_PROFILE_ALLOCATIONS)) + if (mono_class_has_finalizer (klass) || mono_class_is_marshalbyref (klass)) return NULL; if (klass->rank) return NULL; + if (mono_profiler_get_events () & MONO_PROFILE_ALLOCATIONS) + return NULL; if (klass->byval_arg.type == MONO_TYPE_STRING) - return mono_gc_get_managed_allocator_by_type (ATYPE_STRING, FALSE); + return mono_gc_get_managed_allocator_by_type (ATYPE_STRING, MANAGED_ALLOCATOR_REGULAR); /* Generic classes have dynamic field and can go above MAX_SMALL_OBJ_SIZE. */ if (known_instance_size) - return mono_gc_get_managed_allocator_by_type (ATYPE_SMALL, FALSE); + return mono_gc_get_managed_allocator_by_type (ATYPE_SMALL, MANAGED_ALLOCATOR_REGULAR); else - return mono_gc_get_managed_allocator_by_type (ATYPE_NORMAL, FALSE); + return mono_gc_get_managed_allocator_by_type (ATYPE_NORMAL, MANAGED_ALLOCATOR_REGULAR); #else return NULL; #endif @@ -1433,7 +1468,7 @@ mono_gc_get_managed_array_allocator (MonoClass *klass) return NULL; g_assert (!mono_class_has_finalizer (klass) && !mono_class_is_marshalbyref (klass)); - return mono_gc_get_managed_allocator_by_type (ATYPE_VECTOR, FALSE); + return mono_gc_get_managed_allocator_by_type (ATYPE_VECTOR, MANAGED_ALLOCATOR_REGULAR); #else return NULL; #endif @@ -1446,11 +1481,11 @@ sgen_set_use_managed_allocator (gboolean flag) } MonoMethod* -mono_gc_get_managed_allocator_by_type (int atype, gboolean slowpath) +mono_gc_get_managed_allocator_by_type (int atype, ManagedAllocatorVariant variant) { #ifdef MANAGED_ALLOCATION MonoMethod *res; - MonoMethod **cache = slowpath ? slowpath_alloc_method_cache : alloc_method_cache; + MonoMethod **cache; if (!use_managed_allocator) return NULL; @@ -1458,11 +1493,17 @@ mono_gc_get_managed_allocator_by_type (int atype, gboolean slowpath) if (!mono_runtime_has_tls_get ()) return NULL; + switch (variant) { + case MANAGED_ALLOCATOR_REGULAR: cache = alloc_method_cache; break; + case MANAGED_ALLOCATOR_SLOW_PATH: cache = slowpath_alloc_method_cache; break; + default: g_assert_not_reached (); break; + } + res = cache [atype]; if (res) return res; - res = create_allocator (atype, slowpath); + res = create_allocator (atype, variant); LOCK_GC; if (cache [atype]) { mono_free_method (res); diff --git a/mono/mini/aot-compiler.c b/mono/mini/aot-compiler.c index 4ae5c894de5..28d254d0ac6 100644 --- a/mono/mini/aot-compiler.c +++ b/mono/mini/aot-compiler.c @@ -3875,13 +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); } } diff --git a/mono/mini/aot-runtime.c b/mono/mini/aot-runtime.c index 7407fecdf57..b5d7ab9d36b 100644 --- a/mono/mini/aot-runtime.c +++ b/mono/mini/aot-runtime.c @@ -930,8 +930,11 @@ decode_method_ref_with_target (MonoAotModule *module, MethodRef *ref, MonoMethod #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; diff --git a/mono/mini/method-to-ir.c b/mono/mini/method-to-ir.c index ec4e6f429e4..fe467254e95 100644 --- a/mono/mini/method-to-ir.c +++ b/mono/mini/method-to-ir.c @@ -12768,6 +12768,22 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b ip += 6; break; } + case CEE_MONO_ATOMIC_STORE_I4: { + g_assert (mono_arch_opcode_supported (OP_ATOMIC_STORE_I4)); + + CHECK_OPSIZE (6); + CHECK_STACK (2); + sp -= 2; + + MONO_INST_NEW (cfg, ins, OP_ATOMIC_STORE_I4); + ins->dreg = sp [0]->dreg; + ins->sreg1 = sp [1]->dreg; + ins->backend.memory_barrier_kind = (int) read32 (ip + 2); + MONO_ADD_INS (cfg->cbb, ins); + + ip += 6; + break; + } case CEE_MONO_JIT_ATTACH: { MonoInst *args [16], *domain_ins; MonoInst *ad_ins, *jit_tls_ins; diff --git a/mono/sgen/sgen-alloc.c b/mono/sgen/sgen-alloc.c index 2bc3214f143..0c824caa9eb 100644 --- a/mono/sgen/sgen-alloc.c +++ b/mono/sgen/sgen-alloc.c @@ -68,6 +68,9 @@ static __thread char *tlab_temp_end; static __thread char *tlab_real_end; /* Used by the managed allocator/wbarrier */ static __thread char **tlab_next_addr MONO_ATTR_USED; +#ifndef SGEN_WITHOUT_MONO +static __thread volatile int *in_critical_region_addr MONO_ATTR_USED; +#endif #endif #ifdef HAVE_KW_THREAD @@ -506,6 +509,9 @@ sgen_init_tlab_info (SgenThreadInfo* info) #ifdef HAVE_KW_THREAD tlab_next_addr = &tlab_next; +#ifndef SGEN_WITHOUT_MONO + in_critical_region_addr = &info->client_info.in_critical_region; +#endif #endif } @@ -530,13 +536,15 @@ sgen_init_allocator (void) #if defined(HAVE_KW_THREAD) && !defined(SGEN_WITHOUT_MONO) int tlab_next_addr_offset = -1; int tlab_temp_end_offset = -1; - + int in_critical_region_addr_offset = -1; MONO_THREAD_VAR_OFFSET (tlab_next_addr, tlab_next_addr_offset); MONO_THREAD_VAR_OFFSET (tlab_temp_end, tlab_temp_end_offset); + MONO_THREAD_VAR_OFFSET (in_critical_region_addr, in_critical_region_addr_offset); mono_tls_key_set_offset (TLS_KEY_SGEN_TLAB_NEXT_ADDR, tlab_next_addr_offset); mono_tls_key_set_offset (TLS_KEY_SGEN_TLAB_TEMP_END, tlab_temp_end_offset); + mono_tls_key_set_offset (TLS_KEY_SGEN_IN_CRITICAL_REGION_ADDR, in_critical_region_addr_offset); #endif #ifdef HEAVY_STATISTICS diff --git a/mono/utils/mono-tls.h b/mono/utils/mono-tls.h index 565b3fe593b..2c3ed31df45 100644 --- a/mono/utils/mono-tls.h +++ b/mono/utils/mono-tls.h @@ -27,7 +27,8 @@ typedef enum { TLS_KEY_SGEN_TLAB_TEMP_END = 6, TLS_KEY_BOEHM_GC_THREAD = 7, TLS_KEY_LMF_ADDR = 8, - TLS_KEY_NUM = 9 + TLS_KEY_SGEN_IN_CRITICAL_REGION_ADDR = 9, + TLS_KEY_NUM = 10 } MonoTlsKey; #ifdef HOST_WIN32 -- 2.25.1