From: Andi McClure <andi.mcclure@xamarin.com>
Date: Fri, 18 Mar 2016 19:18:38 +0000 (-0400)
Subject: Add mono_threads_reset_blocking nesting checks to gc checked mode
X-Git-Url: http://wien.tomnetworks.com/gitweb/?a=commitdiff_plain;h=173e2d4ce65ca671a69b39011b71ff3de945c4f4;p=mono.git

Add mono_threads_reset_blocking nesting checks to gc checked mode
---

diff --git a/mono/utils/mono-threads-coop.c b/mono/utils/mono-threads-coop.c
index 3b11148aafe..2a31e93e6f1 100644
--- a/mono/utils/mono-threads-coop.c
+++ b/mono/utils/mono-threads-coop.c
@@ -25,6 +25,7 @@
 #include <mono/utils/mono-counters.h>
 #include <mono/utils/mono-threads-coop.h>
 #include <mono/utils/mono-threads-api.h>
+#include <mono/utils/checked-build.h>
 
 #ifdef TARGET_OSX
 #include <mono/utils/mach-support.h>
@@ -39,6 +40,35 @@
 
 volatile size_t mono_polling_required;
 
+// FIXME: This would be more efficient if instead of instantiating the stack it just pushed a simple depth counter up and down,
+// perhaps with a per-thread cookie in the high bits.
+#ifdef ENABLE_CHECKED_BUILD_GC
+// Maintains a single per-thread stack of ints, used to ensure nesting is not violated
+MonoNativeTlsKey coop_reset_count_stack_key;
+static int coop_tls_push (int v) {
+	GArray *stack = mono_native_tls_get_value (coop_reset_count_stack_key);
+	if (!stack) {
+		stack = g_array_new (FALSE,FALSE,sizeof(int));
+		mono_native_tls_set_value (coop_reset_count_stack_key, stack);
+	}
+	g_array_append_val (stack, v);
+	return stack->len;
+}
+static int coop_tls_pop (int *v) {
+	GArray *stack = mono_native_tls_get_value (coop_reset_count_stack_key);
+	if (!stack || 0 == stack->len)
+		return -1;
+	stack->len--;
+	*v = g_array_index (stack, int, stack->len);
+	int len = stack->len;
+	if (0 == len) {
+		g_array_free (stack,TRUE);
+		mono_native_tls_set_value (coop_reset_count_stack_key, NULL);
+	}
+	return len;
+}
+#endif
+
 static int coop_reset_blocking_count;
 static int coop_try_blocking_count;
 static int coop_do_blocking_count;
@@ -193,9 +223,18 @@ mono_threads_reset_blocking_start (void* stackdata)
 	if (!mono_threads_is_coop_enabled ())
 		return NULL;
 
+	info = mono_thread_info_current_unchecked ();
+
+#ifdef ENABLE_CHECKED_BUILD_GC
+	int reset_blocking_count = InterlockedIncrement (&coop_reset_blocking_count);
+	// In this mode, the blocking count is used as the reset cookie. We would prefer
+	// (but do not require) this to be unique across invocations and threads.
+	if (reset_blocking_count == 0) // We *do* require it be nonzero
+		reset_blocking_count = coop_reset_blocking_count = 1;
+#else
 	++coop_reset_blocking_count;
+#endif
 
-	info = mono_thread_info_current_unchecked ();
 	/* If the thread is not attached, it doesn't make sense prepare for suspend. */
 	if (!info || !mono_thread_info_is_live (info))
 		return NULL;
@@ -211,28 +250,50 @@ mono_threads_reset_blocking_start (void* stackdata)
 		return NULL;
 	case AbortBlockingOk:
 		info->thread_saved_state [SELF_SUSPEND_STATE_INDEX].valid = FALSE;
-		return info;
+		break;
 	case AbortBlockingOkAndPool:
 		mono_threads_state_poll ();
-		return info;
+		break;
 	default:
 		g_error ("Unknown thread state");
 	}
+
+#ifdef ENABLE_CHECKED_BUILD_GC
+	if (mono_check_mode_enabled (MONO_CHECK_MODE_GC)) {
+		int level = coop_tls_push (reset_blocking_count);
+		//g_warning("Entering reset nest; level %d; cookie %d\n", level, reset_blocking_count);
+		return (void *)(intptr_t)reset_blocking_count;
+	}
+#endif
+
+	return info;
 }
 
 void
 mono_threads_reset_blocking_end (void *cookie, void* stackdata)
 {
-	MonoThreadInfo *info;
-
 	if (!mono_threads_is_coop_enabled ())
 		return;
 
-	info = (MonoThreadInfo *)cookie;
-	if (!info)
+	if (!cookie)
 		return;
 
-	g_assert (info == mono_thread_info_current_unchecked ());
+#ifdef ENABLE_CHECKED_BUILD_GC
+	if (mono_check_mode_enabled (MONO_CHECK_MODE_GC)) {
+		int received_cookie = (int)(intptr_t)cookie;
+		int desired_cookie;
+		int level = coop_tls_pop (&desired_cookie);
+		//g_warning("Leaving reset nest; back to level %d; desired cookie %d; received cookie %d\n", level, desired_cookie, received_cookie);
+		if (level < 0)
+			g_error_with_history ("Expected cookie %d but found no stack at all\n", desired_cookie);
+		if (desired_cookie != received_cookie)
+			g_error_with_history ("Expected cookie %d but received %d\n", desired_cookie, received_cookie);
+	} else // Notice this matches the line after the endif
+#endif
+	{
+		g_assert (((MonoThreadInfo *)cookie) == mono_thread_info_current_unchecked ());
+	}
+
 	mono_threads_prepare_blocking (stackdata);
 }
 
@@ -248,6 +309,10 @@ mono_threads_init_coop (void)
 	mono_counters_register ("Coop Do Polling", MONO_COUNTER_GC | MONO_COUNTER_INT, &coop_do_polling_count);
 	mono_counters_register ("Coop Save Count", MONO_COUNTER_GC | MONO_COUNTER_INT, &coop_save_count);
 	//See the above for what's wrong here.
+
+#ifdef ENABLE_CHECKED_BUILD_GC
+	mono_native_tls_alloc (&coop_reset_count_stack_key, NULL);
+#endif
 }
 
 void