From: Vlad Brezae <brezaevlad@gmail.com>
Date: Fri, 29 May 2020 22:09:11 +0000 (+0300)
Subject: [interp] Remove assertions from release and add more to debug builds (#37160)
X-Git-Tag: submit/tizen/20210909.063632~7701
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=7bd5ec866b1ef39199989018216f24648498f9c3;p=platform%2Fupstream%2Fdotnet%2Fruntime.git

[interp] Remove assertions from release and add more to debug builds (#37160)

* [interp] Remove some assertions from hot path

We can keep them only for debug builds, where in the future we can add more verbose assertions, like ones for stack overflows.

* [interp] Add more assertions on debug build

Most of the weird interp bugs are due to overflows in the interp stacks. Since we now test interp on CI with debug build, try to catch them early.
---

diff --git a/src/mono/mono/mini/interp/interp.c b/src/mono/mono/mini/interp/interp.c
index 4ac1b20..ed8559c 100644
--- a/src/mono/mono/mini/interp/interp.c
+++ b/src/mono/mono/mini/interp/interp.c
@@ -199,7 +199,7 @@ frame_stack_alloc (FrameStack *stack, int size, StackFragment **out_frag)
 static MONO_ALWAYS_INLINE void
 frame_stack_pop (FrameStack *stack, StackFragment *frag, gpointer pos)
 {
-	g_assert ((guint8*)pos >= (guint8*)&frag->data && (guint8*)pos <= (guint8*)frag->end);
+	g_assert_checked ((guint8*)pos >= (guint8*)&frag->data && (guint8*)pos <= (guint8*)frag->end);
 	stack->current = frag;
 	mono_compiler_barrier ();
 	stack->current->pos = (guint8*)pos;
@@ -365,7 +365,7 @@ int mono_interp_traceopt = 0;
 
 #endif
 
-#if defined(__GNUC__) && !defined(TARGET_WASM) && !COUNT_OPS && !DEBUG_INTERP
+#if defined(__GNUC__) && !defined(TARGET_WASM) && !COUNT_OPS && !DEBUG_INTERP && !ENABLE_CHECKED_BUILD
 #define USE_COMPUTED_GOTO 1
 #endif
 
@@ -3423,8 +3423,11 @@ main_loop:
 	 */
 	while (1) {
 		MintOpcode opcode;
-		/* g_assert (sp >= frame->stack); */
-		/* g_assert(vt_sp - vtalloc <= frame->imethod->vt_stack_size); */
+#ifdef ENABLE_CHECKED_BUILD
+		g_assert (sp >= frame->stack);
+		g_assert (vt_sp >= sp);
+		g_assert (locals >= vt_sp);
+#endif
 		DUMP_INSTR();
 		MINT_IN_SWITCH (*ip) {
 		MINT_IN_CASE(MINT_INITLOCALS)
@@ -4008,12 +4011,18 @@ call:
 				// FIXME This can only happen in a few wrappers. Add separate opcode for it
 				*frame->retval = *sp;
 			}
+#ifdef ENABLE_CHECKED_BUILD
+			/* FIXME Fix these warnings and replace with assertions */
 			if (sp > frame->stack)
 				g_warning_d ("ret: more values on stack: %d", sp - frame->stack);
+#endif
 			goto exit_frame;
 		MINT_IN_CASE(MINT_RET_VOID)
+#ifdef ENABLE_CHECKED_BUILD
+			/* FIXME Fix these warnings and replace with assertions */
 			if (sp > frame->stack)
 				g_warning_ds ("ret.void: more values on stack: %d %s", sp - frame->stack, mono_method_full_name (frame->imethod->method, TRUE));
+#endif
 			goto exit_frame;
 		MINT_IN_CASE(MINT_RET_VT) {
 			gpointer dest_vt;
@@ -4029,8 +4038,11 @@ call:
 				dest_vt = frame->retval->data.p;
 			}
 			memcpy (dest_vt, sp->data.p, i32);
+#ifdef ENABLE_CHECKED_BUILD
+			/* FIXME Fix these warnings and replace with assertions */
 			if (sp > frame->stack)
 				g_warning_d ("ret.vt: more values on stack: %d", sp - frame->stack);
+#endif
 			goto exit_frame;
 		}
 
@@ -7186,7 +7198,7 @@ resume:
 exit_frame:
 	error_init_reuse (error);
 
-	g_assert (frame->imethod);
+	g_assert_checked (frame->imethod);
 
 	if (clause_args && clause_args->base_frame) {
 		// We finished executing a filter. The execution stack of the base frame