Skip to content

Commit bde9ba9

Browse files
authored
JIT: array allocation fixes (#112676)
Fix some issues with array stack allocation that are currently hard to observe: * use correct method table for shared array types * pad array layout out to a multiple of TARGET_POINTER_SIZE It's currently not easy to observe the type of a stack allocated array as accessing the type causes escape. But this becomes possible when supporting span capture or once we have a version of the covariant store check that can handle stack allocated arrays. Non-padded layouts may mean elements just off the end of an odd-length short or byte array won't be zeroed, which some unsafe uses may expect. Added some test cases.
1 parent 1e38de0 commit bde9ba9

File tree

3 files changed

+47
-7
lines changed

3 files changed

+47
-7
lines changed

src/coreclr/jit/helperexpansion.cpp

+3-7
Original file line numberDiff line numberDiff line change
@@ -2822,6 +2822,7 @@ bool Compiler::fgExpandStackArrayAllocation(BasicBlock* block, Statement* stmt,
28222822

28232823
const CorInfoHelpFunc helper = eeGetHelperNum(call->gtCallMethHnd);
28242824
int lengthArgIndex = -1;
2825+
int typeArgIndex = -1;
28252826

28262827
switch (helper)
28272828
{
@@ -2830,10 +2831,7 @@ bool Compiler::fgExpandStackArrayAllocation(BasicBlock* block, Statement* stmt,
28302831
case CORINFO_HELP_NEWARR_1_OBJ:
28312832
case CORINFO_HELP_NEWARR_1_ALIGN8:
28322833
lengthArgIndex = 1;
2833-
break;
2834-
2835-
case CORINFO_HELP_READYTORUN_NEWARR_1:
2836-
lengthArgIndex = 0;
2834+
typeArgIndex = 0;
28372835
break;
28382836

28392837
default:
@@ -2871,9 +2869,7 @@ bool Compiler::fgExpandStackArrayAllocation(BasicBlock* block, Statement* stmt,
28712869

28722870
// Initialize the array method table pointer.
28732871
//
2874-
CORINFO_CLASS_HANDLE arrayHnd = (CORINFO_CLASS_HANDLE)call->compileTimeHelperArgumentHandle;
2875-
2876-
GenTree* const mt = gtNewIconEmbClsHndNode(arrayHnd);
2872+
GenTree* const mt = call->gtArgs.GetArgByIndex(typeArgIndex)->GetNode();
28772873
GenTree* const mtStore = gtNewStoreValueNode(TYP_I_IMPL, stackLocalAddress, mt);
28782874
Statement* const mtStmt = fgNewStmtFromTree(mtStore);
28792875

src/coreclr/jit/layout.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -778,6 +778,7 @@ ClassLayoutBuilder ClassLayoutBuilder::BuildArray(Compiler* compiler, CORINFO_CL
778778

779779
ClrSafeInt<unsigned> totalSize(elementSize);
780780
totalSize *= static_cast<unsigned>(length);
781+
totalSize.AlignUp(TARGET_POINTER_SIZE);
781782
totalSize += static_cast<unsigned>(OFFSETOF__CORINFO_Array__data);
782783
assert(!totalSize.IsOverflow());
783784

src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs

+43
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,11 @@ public static int TestEntryPoint()
156156
// Stack allocation of boxed structs is now enabled
157157
CallTestAndVerifyAllocation(BoxSimpleStructAndAddFields, 12, expectedAllocationKind);
158158

159+
// Fixed-sized stack array cases
159160
CallTestAndVerifyAllocation(AllocateArrayWithNonGCElements, 84, expectedAllocationKind);
161+
CallTestAndVerifyAllocation(AllocateArrayWithGCElements, 84, expectedAllocationKind);
162+
CallTestAndVerifyAllocation(AllocateArrayT<int>, 84, expectedAllocationKind);
163+
CallTestAndVerifyAllocation(AllocateArrayT<string>, 84, expectedAllocationKind);
160164

161165
// The remaining tests currently never allocate on the stack
162166
if (expectedAllocationKind == AllocationKind.Stack) {
@@ -170,6 +174,7 @@ public static int TestEntryPoint()
170174
CallTestAndVerifyAllocation(AllocateSimpleClassAndCast, 7, expectedAllocationKind);
171175

172176
CallTestAndVerifyAllocation(AllocateArrayWithNonGCElementsEscape, 42, expectedAllocationKind);
177+
CallTestAndVerifyAllocation(AllocateArrayWithGCElementsEscape, 42, expectedAllocationKind);
173178

174179
// This test calls CORINFO_HELP_OVERFLOW
175180
CallTestAndVerifyAllocation(AllocateArrayWithNonGCElementsOutOfRangeLeft, 0, expectedAllocationKind, true);
@@ -375,6 +380,29 @@ static int AllocateArrayWithNonGCElements()
375380
return array[24] + array.Length;
376381
}
377382

383+
[MethodImpl(MethodImplOptions.NoInlining)]
384+
static int AllocateArrayWithGCElements()
385+
{
386+
string[] array = new string[42];
387+
array[24] = "42";
388+
GC.Collect();
389+
return array[24].Length * 21 + array.Length;
390+
}
391+
392+
[MethodImpl(MethodImplOptions.NoInlining)]
393+
static int AllocateArrayT<T>()
394+
{
395+
T[] array = new T[42];
396+
T t = array[24];
397+
GC.Collect();
398+
399+
// Todo -- validate array type (currently causes escape for shared)
400+
// Todo -- store to array (currently causes escape for shared)
401+
402+
Consume(t);
403+
return array.Length + 42;
404+
}
405+
378406
[MethodImpl(MethodImplOptions.NoInlining)]
379407
static int AllocateArrayWithNonGCElementsEscape()
380408
{
@@ -384,6 +412,15 @@ static int AllocateArrayWithNonGCElementsEscape()
384412
return array[24];
385413
}
386414

415+
[MethodImpl(MethodImplOptions.NoInlining)]
416+
static int AllocateArrayWithGCElementsEscape()
417+
{
418+
string[] array = new string[42];
419+
Use(ref array[24]);
420+
GC.Collect();
421+
return array[24].Length * 21;
422+
}
423+
387424
[MethodImpl(MethodImplOptions.NoInlining)]
388425
static int AllocateArrayWithNonGCElementsOutOfRangeRight()
389426
{
@@ -424,6 +461,12 @@ static void Use(ref int v)
424461
v = 42;
425462
}
426463

464+
[MethodImpl(MethodImplOptions.NoInlining)]
465+
static void Use(ref string s)
466+
{
467+
s = "42";
468+
}
469+
427470
[MethodImpl(MethodImplOptions.NoInlining)]
428471
private static void ZeroAllocTest()
429472
{

0 commit comments

Comments
 (0)