Skip to content

Commit

Permalink
[mono] Fix multidimensional array construction when using programmer-…
Browse files Browse the repository at this point in the history 
…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 #34377.
Fixes #34378.
Fixes #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: imhameed <imhameed@users.noreply.github.com>
  • Loading branch information
@monojenkins @imhameed
monojenkins and imhameed committed Apr 19, 2020
1 parent 01a08fe commit 86940e1
Show file tree
Hide file tree
Showing 3 changed files with 212 additions and 20 deletions.
172 changes: 172 additions & 0 deletions src/mono/mono/mini/iltests.il
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

}
31 changes: 17 additions & 14 deletions src/mono/mono/mini/interp/interp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
29 changes: 23 additions & 6 deletions src/mono/mono/mini/method-to-ir.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8728,9 +8728,10 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
*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;
Expand All @@ -8745,8 +8746,8 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
}
}

/* 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) {
Expand All @@ -8759,9 +8760,25 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
}
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;
Expand Down

0 comments on commit 86940e1

Please sign in to comment.