From 48dfe0404994df4dcfdcd321cd58cc8226bc05e0 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Sun, 17 Oct 2021 12:45:25 +0300 Subject: [PATCH] [interp] Fix sufficient stack check when using interpreter The icall was only checking the real stack pointer of the thread. The interpreter uses its own execution stack in addition, so we also need to check for overflow there. --- .../tests/Dataflow/DataflowBlockExtensionTests.cs | 2 -- src/mono/mono/metadata/icall.c | 4 ++++ src/mono/mono/metadata/object-internals.h | 1 + src/mono/mono/mini/ee.h | 1 + src/mono/mono/mini/interp-stubs.c | 6 ++++++ src/mono/mono/mini/interp/interp.c | 8 ++++++++ src/mono/mono/mini/mini-runtime.c | 8 ++++++++ 7 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Threading.Tasks.Dataflow/tests/Dataflow/DataflowBlockExtensionTests.cs b/src/libraries/System.Threading.Tasks.Dataflow/tests/Dataflow/DataflowBlockExtensionTests.cs index d2a46b41ed236..2d59951cad66e 100644 --- a/src/libraries/System.Threading.Tasks.Dataflow/tests/Dataflow/DataflowBlockExtensionTests.cs +++ b/src/libraries/System.Threading.Tasks.Dataflow/tests/Dataflow/DataflowBlockExtensionTests.cs @@ -1122,7 +1122,6 @@ public async Task TestReceiveAsync_ManyInOrder() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/38817", typeof(PlatformDetection), nameof(PlatformDetection.IsMonoInterpreter))] [SkipOnPlatform(TestPlatforms.Browser, "uses a lot of stack")] public async Task TestReceiveAsync_LongChain() { @@ -1924,7 +1923,6 @@ public async Task TestOutputAvailableAsync_DataAfterCompletion() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/38817", typeof(PlatformDetection), nameof(PlatformDetection.IsMonoInterpreter))] [SkipOnPlatform(TestPlatforms.Browser, "uses a lot of stack")] public async Task TestOutputAvailableAsync_LongSequence() { diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index 04479684a6125..3050cd083e5b9 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -1075,6 +1075,10 @@ ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_SufficientExecutionStac if (current < limit) return FALSE; + if (mono_get_runtime_callbacks ()->is_interpreter_enabled () && + !mono_get_runtime_callbacks ()->interp_sufficient_stack (MONO_MIN_EXECUTION_STACK_SIZE)) + return FALSE; + return TRUE; } diff --git a/src/mono/mono/metadata/object-internals.h b/src/mono/mono/metadata/object-internals.h index b50ed3c47d8bd..6dd9d4d4fdee6 100644 --- a/src/mono/mono/metadata/object-internals.h +++ b/src/mono/mono/metadata/object-internals.h @@ -646,6 +646,7 @@ typedef struct { // Same as compile_method, but returns a MonoFtnDesc in llvmonly mode gpointer (*get_ftnptr)(MonoMethod *method, MonoError *error); void (*interp_jit_info_foreach)(InterpJitInfoFunc func, gpointer user_data); + gboolean (*interp_sufficient_stack)(gsize size); } MonoRuntimeCallbacks; typedef gboolean (*MonoInternalStackWalk) (MonoStackFrameInfo *frame, MonoContext *ctx, gpointer data); diff --git a/src/mono/mono/mini/ee.h b/src/mono/mono/mini/ee.h index 6f171c85bd718..54c2ea6dfb342 100644 --- a/src/mono/mono/mini/ee.h +++ b/src/mono/mono/mini/ee.h @@ -60,6 +60,7 @@ typedef gpointer MonoInterpFrameHandle; MONO_EE_CALLBACK (void, cleanup, (void)) \ MONO_EE_CALLBACK (void, mark_stack, (gpointer thread_info, GcScanFunc func, gpointer gc_data, gboolean precise)) \ MONO_EE_CALLBACK (void, jit_info_foreach, (InterpJitInfoFunc func, gpointer user_data)) \ + MONO_EE_CALLBACK (gboolean, sufficient_stack, (gsize size)) \ typedef struct _MonoEECallbacks { diff --git a/src/mono/mono/mini/interp-stubs.c b/src/mono/mono/mini/interp-stubs.c index 81e71c0982793..8772f765829eb 100644 --- a/src/mono/mono/mini/interp-stubs.c +++ b/src/mono/mono/mini/interp-stubs.c @@ -220,6 +220,12 @@ stub_jit_info_foreach (InterpJitInfoFunc func, gpointer user_data) { } +static gboolean +stub_sufficient_stack (gsize size) +{ + g_assert_not_reached (); +} + #undef MONO_EE_CALLBACK #define MONO_EE_CALLBACK(ret, name, sig) stub_ ## name, diff --git a/src/mono/mono/mini/interp/interp.c b/src/mono/mono/mini/interp/interp.c index abd3e25e65a15..4d687a99120ff 100644 --- a/src/mono/mono/mini/interp/interp.c +++ b/src/mono/mono/mini/interp/interp.c @@ -7520,6 +7520,14 @@ interp_jit_info_foreach (InterpJitInfoFunc func, gpointer user_data) } } +static gboolean +interp_sufficient_stack (gsize size) +{ + ThreadContext *context = get_context (); + + return (context->stack_pointer + size) < (context->stack_start + INTERP_STACK_SIZE); +} + static void interp_cleanup (void) { diff --git a/src/mono/mono/mini/mini-runtime.c b/src/mono/mono/mini/mini-runtime.c index 5ad5a916421b2..7eb181c5412ac 100644 --- a/src/mono/mono/mini/mini-runtime.c +++ b/src/mono/mono/mini/mini-runtime.c @@ -160,6 +160,7 @@ static void register_icalls (void); static void runtime_cleanup (MonoDomain *domain, gpointer user_data); static void mini_invalidate_transformed_interp_methods (MonoAssemblyLoadContext *alc, uint32_t generation); static void mini_interp_jit_info_foreach(InterpJitInfoFunc func, gpointer user_data); +static gboolean mini_interp_sufficient_stack (gsize size); gboolean mono_running_on_valgrind (void) @@ -4445,6 +4446,7 @@ mini_init (const char *filename, const char *runtime_version) callbacks.metadata_update_published = mini_invalidate_transformed_interp_methods; callbacks.interp_jit_info_foreach = mini_interp_jit_info_foreach; + callbacks.interp_sufficient_stack = mini_interp_sufficient_stack; callbacks.init_mem_manager = init_jit_mem_manager; callbacks.free_mem_manager = free_jit_mem_manager; @@ -5215,6 +5217,12 @@ mini_interp_jit_info_foreach(InterpJitInfoFunc func, gpointer user_data) mini_get_interp_callbacks ()->jit_info_foreach (func, user_data); } +static gboolean +mini_interp_sufficient_stack (gsize size) +{ + return mini_get_interp_callbacks ()->sufficient_stack (size); +} + /* * mini_get_default_mem_manager: *