[interp] Always init locals (#32193)
authormonojenkins <jo.shields+jenkins@xamarin.com>
Thu, 13 Feb 2020 12:00:51 +0000 (07:00 -0500)
committerGitHub <noreply@github.com>
Thu, 13 Feb 2020 12:00:51 +0000 (13:00 +0100)
Storing uninitialized locals in a managed object shouldn't crash the runtime, so they should be zeroed. This doesn't seem to regress performance on wasm since the initlocals flag is always set, currently. We should be able to remove it most of the time, once we add some multi basic block support to the cprop phase. This will enable us to further reduce call overhead.

Fixes https://github.com/mono/mono/issues/18527

Co-authored-by: Vlad Brezae <brezaevlad@gmail.com>
src/mono/mono/mini/interp/transform.c

index a5d370a..5f2075d 100644 (file)
@@ -3298,7 +3298,18 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header,
                        g_free (name);
                }
 
-               if (header->num_locals && header->init_locals)
+               /*
+                * We initialize the locals regardless of the presence of the init_locals
+                * flag. Locals holding references need to be zeroed so we don't risk
+                * crashing the GC if they end up being stored in an object.
+                *
+                * FIXME
+                * Track values of locals over multiple basic blocks. This would enable
+                * us to kill the MINT_INITLOCALS instruction if all locals are initialized
+                * before use. We also don't need this instruction if the init locals flag
+                * is not set and there are no locals holding references.
+                */
+               if (header->num_locals)
                        interp_add_ins (td, MINT_INITLOCALS);
 
                guint16 enter_profiling = 0;