[interp] Remove assertions from release and add more to debug builds (#37160)
authorVlad Brezae <brezaevlad@gmail.com>
Fri, 29 May 2020 22:09:11 +0000 (01:09 +0300)
committerGitHub <noreply@github.com>
Fri, 29 May 2020 22:09:11 +0000 (01:09 +0300)
* [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.

src/mono/mono/mini/interp/interp.c

index 4ac1b20..ed8559c 100644 (file)
@@ -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