[mono] Fix multidimensional array construction when using programmer-specified...

[mono] Fix multidimensional array construction when using programmer-specified lower bounds. (#35091)

* [mono] Fix multidimensional array construction when using programmer-specified lower bounds.

mono_array_full_new_checked and mono_array_full_new (which is marked as
a MONO_API function) both take two pointers to buffers containing
lengths and lower bounds. mono_array_new_n_icall can split the incoming
parameter list in two before forwarding the results to
mono_array_full_new_checked, so mono_array_full_new_checked was
receiving two buffers containing interleaved lower bounds and lengths.

ECMA-335 states that array constructors that specify both lower bounds
and lengths interleave these values. Deinterleave these
in method_to_ir.

Add some tests to iltests.il that verify that multidimensional arrays
with custom lower bounds work.

Fixes https://github.com/dotnet/runtime/issues/34377.
Fixes https://github.com/dotnet/runtime/issues/34378.
Fixes https://github.com/dotnet/runtime/issues/34381.

* Deinterleave lower bounds and array lengths in the interpreter.

Also make the condition in which the deinterleaving code path is taken
more precise. Perhaps this branch should be marked as unlikely.

* Fix a typo.
Co-authored-by: Nimhameed <imhameed@users.noreply.github.com>

src/mono/mono/mini/iltests.il

@@ -3673,4 +3673,176 @@ L3:
 		ret
 	}
 
+	.method private hidebysig static int32 test_0_two_dimensional_array_bounds() cil managed
+	{
+		.maxstack 5
+		.locals init (int32[,] arr)
+		ldc.i4 100
+		ldc.i4 27
+		ldc.i4 200
+		ldc.i4 31
+		newobj instance void int32[,]::.ctor(int32, int32, int32, int32)
+		stloc arr
+
+		ldc.i4 1
+		ldloc arr
+		ldc.i4 0
+		callvirt instance int32 [mscorlib]System.Array::GetLowerBound(int32)
+		ldc.i4 100
+		bne.un exit
+		pop
+
+		ldc.i4 2
+		ldloc arr
+		ldc.i4 0
+		callvirt instance int32 [mscorlib]System.Array::GetUpperBound(int32)
+		ldc.i4 126
+		bne.un exit
+		pop
+
+		ldc.i4 3
+		ldloc arr
+		ldc.i4 1
+		callvirt instance int32 [mscorlib]System.Array::GetLowerBound(int32)
+		ldc.i4 200
+		bne.un exit
+		pop
+
+		ldc.i4 4
+		ldloc arr
+		ldc.i4 1
+		callvirt instance int32 [mscorlib]System.Array::GetUpperBound(int32)
+		ldc.i4 230
+		bne.un exit
+		pop
+
+		ldc.i4 5
+		ldloc arr
+		ldc.i4 100
+		ldc.i4 200
+		ldc.i4 1234
+		call instance void int32[,]::Set(int32, int32, int32)
+		ldloc arr
+		ldc.i4 100
+		ldc.i4 200
+		call instance int32 int32[,]::Get(int32, int32)
+		ldc.i4 1234
+		bne.un exit
+		pop
+
+		ldc.i4 6
+		ldloc arr
+		ldc.i4 126
+		ldc.i4 230
+		ldc.i4 5678
+		call instance void int32[,]::Set(int32, int32, int32)
+		ldloc arr
+		ldc.i4 126
+		ldc.i4 230
+		call instance int32 int32[,]::Get(int32, int32)
+		ldc.i4 5678
+		bne.un exit
+		pop
+
+		ldc.i4 0
+	exit:
+		ret
+	}
+
+	.method private hidebysig static int32 test_0_five_dimensional_array() cil managed
+	{
+		.maxstack 12
+		.locals init (int32[,,,,] arr)
+		ldc.i4 100
+		ldc.i4 2
+		ldc.i4 200
+		ldc.i4 3
+		ldc.i4 300
+		ldc.i4 4
+		ldc.i4 400
+		ldc.i4 5
+		ldc.i4 500
+		ldc.i4 6
+		newobj instance void int32[,,,,]::.ctor(
+			int32, int32, int32, int32, int32, int32,
+			int32, int32, int32, int32)
+		stloc arr
+
+		ldc.i4 1
+		ldloc arr
+		ldc.i4 0
+		callvirt instance int32 [mscorlib]System.Array::GetLowerBound(int32)
+		ldc.i4 100
+		bne.un exit
+		pop
+
+		ldc.i4 2
+		ldloc arr
+		ldc.i4 0
+		callvirt instance int32 [mscorlib]System.Array::GetUpperBound(int32)
+		ldc.i4 101
+		bne.un exit
+		pop
+
+		ldc.i4 3
+		ldloc arr
+		ldc.i4 4
+		callvirt instance int32 [mscorlib]System.Array::GetLowerBound(int32)
+		ldc.i4 500
+		bne.un exit
+		pop
+
+		ldc.i4 4
+		ldloc arr
+		ldc.i4 4
+		callvirt instance int32 [mscorlib]System.Array::GetUpperBound(int32)
+		ldc.i4 505
+		bne.un exit
+		pop
+
+		ldc.i4 5
+		ldloc arr
+		ldc.i4 100
+		ldc.i4 200
+		ldc.i4 300
+		ldc.i4 400
+		ldc.i4 500
+		ldc.i4 1234
+		call instance void int32[,,,,]::Set(int32, int32, int32, int32, int32, int32)
+		ldloc arr
+		ldc.i4 100
+		ldc.i4 200
+		ldc.i4 300
+		ldc.i4 400
+		ldc.i4 500
+		call instance int32 int32[,,,,]::Get(int32, int32, int32, int32, int32)
+		ldc.i4 1234
+		bne.un exit
+		pop
+
+		ldc.i4 6
+		ldloc arr
+		ldc.i4 101
+		ldc.i4 202
+		ldc.i4 303
+		ldc.i4 404
+		ldc.i4 505
+		ldc.i4 5678
+		call instance void int32[,,,,]::Set(int32, int32, int32, int32, int32, int32)
+		ldloc arr
+		ldc.i4 101
+		ldc.i4 202
+		ldc.i4 303
+		ldc.i4 404
+		ldc.i4 505
+		call instance int32 int32[,,,,]::Get(int32, int32, int32, int32, int32)
+		ldc.i4 5678
+		bne.un exit
+		pop
+
+		ldc.i4 0
+	exit:
+		ret
+	}
+
 }

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

@@ -1146,22 +1146,25 @@ interp_throw (ThreadContext *context, MonoException *ex, InterpFrame *frame, con
 static MonoObject*
 ves_array_create (MonoDomain *domain, MonoClass *klass, int param_count, stackval *values, MonoError *error)
 {
-	uintptr_t *lengths;
-	intptr_t *lower_bounds;
-	int i;
-
-	lengths = g_newa (uintptr_t, m_class_get_rank (klass) * 2);
-	for (i = 0; i < param_count; ++i) {
-		lengths [i] = values->data.i;
-		values ++;
-	}
-	if (m_class_get_rank (klass) == param_count) {
-		/* Only lengths provided. */
-		lower_bounds = NULL;
-	} else {
+	int rank = m_class_get_rank (klass);
+	uintptr_t *lengths = g_newa (uintptr_t, rank * 2);
+	intptr_t *lower_bounds = NULL;
+	if (2 * rank == param_count) {
+		for (int l = 0; l < 2; ++l) {
+			int src = l;
+			int dst = l * rank;
+			for (int r = 0; r < rank; ++r, src += 2, ++dst) {
+				lengths [dst] = values [src].data.i;
+			}
+		}
 		/* lower bounds are first. */
 		lower_bounds = (intptr_t *) lengths;
-		lengths += m_class_get_rank (klass);
+		lengths += rank;
+	} else {
+		/* Only lengths provided. */
+		for (int i = 0; i < param_count; ++i) {
+			lengths [i] = values [i].data.i;
+		}
 	}
 	return (MonoObject*) mono_array_new_full_checked (domain, klass, lengths, lower_bounds, error);
 }

src/mono/mono/mini/method-to-ir.c

@@ -8728,9 +8728,10 @@ calli_end:
 				*sp = emit_get_rgctx_method (cfg, context_used,
 											 cmethod, MONO_RGCTX_INFO_METHOD);
 				MonoJitICallId function = MONO_JIT_ICALL_ZeroIsReserved;
+				int rank = m_class_get_rank (cmethod->klass);
 				int n = fsig->param_count;
 				/* Optimize the common cases, use ctor using length for each rank (no lbound). */
-				if (n == m_class_get_rank (cmethod->klass)) {
+				if (n == rank) {
 					switch (n) {
 					case 1: function = MONO_JIT_ICALL_mono_array_new_1;
 						break;
@@ -8745,8 +8746,8 @@ calli_end:
 					}
 				}
 
-				/* Instancing jagged arrays should not end up here since ctor (int32, int32) for an array with rank 1 represent lenght and lbound. */
-				g_assert (!(m_class_get_rank (cmethod->klass) == 1 && fsig->param_count == 2 && m_class_get_rank (m_class_get_element_class (cmethod->klass))));
+				/* Instancing jagged arrays should not end up here since ctor (int32, int32) for an array with rank 1 represents length and lbound. */
+				g_assert (!(rank == 1 && fsig->param_count == 2 && m_class_get_rank (m_class_get_element_class (cmethod->klass))));
 
 				/* Regular case, rank > 4 or legnth, lbound specified per rank. */
 				if (function == MONO_JIT_ICALL_ZeroIsReserved) {
@@ -8759,9 +8760,25 @@ calli_end:
 					}
 					array_new_localalloc_ins->inst_imm = MAX (array_new_localalloc_ins->inst_imm, n * sizeof (target_mgreg_t));
 					int dreg = array_new_localalloc_ins->dreg;
-					for (int i = 0; i < n; ++i) {
-						NEW_STORE_MEMBASE (cfg, ins, OP_STORE_MEMBASE_REG, dreg, i * sizeof (target_mgreg_t), sp [i + 1]->dreg);
-						MONO_ADD_INS (cfg->cbb, ins);
+					if (2 * rank == n) {
+						/* [lbound, length, lbound, length, ...]
+						 * mono_array_new_n_icall expects a non-interleaved list of
+						 * lbounds and lengths, so deinterleave here.
+						 */
+						for (int l = 0; l < 2; ++l) {
+							int src = l;
+							int dst = l * rank;
+							for (int r = 0; r < rank; ++r, src += 2, ++dst) {
+								NEW_STORE_MEMBASE (cfg, ins, OP_STORE_MEMBASE_REG, dreg, dst * sizeof (target_mgreg_t), sp [src + 1]->dreg);
+								MONO_ADD_INS (cfg->cbb, ins);
+							}
+						}
+					} else {
+						/* [length, length, length, ...] */
+						for (int i = 0; i < n; ++i) {
+							NEW_STORE_MEMBASE (cfg, ins, OP_STORE_MEMBASE_REG, dreg, i * sizeof (target_mgreg_t), sp [i + 1]->dreg);
+							MONO_ADD_INS (cfg->cbb, ins);
+						}
 					}
 					EMIT_NEW_ICONST (cfg, ins, n);
 					sp [1] = ins;