Skip to content

Commit

Permalink
Don't assume objects don't escape in pure helpers.
Browse files Browse the repository at this point in the history
We can't assume objects don't escape in helpers marked as pure for the following reasons:

1. The helpers may call user code that may make objects escape, e.g.,
https://github.com/dotnet/coreclr/blob/dotnet/coreclr@c94d8e68222d931d4bb1c4eb9a52b4b056e85f12/src/vm/jithelpers.cpp#L2371

2. The helpers usually protect gc pointers with GCPROTECT_BEGIN() so the pointers are reported as normal pointers to the gc.
Pointers to stack-allocated objects need to be reported as interior so they have to be protected with
GCPROTECT_BEGININTERIOR().

3. The helpers may cause these asserts in ValidateInner on stack-allocated objects:
https://github.com/dotnet/coreclr/blob/dotnet/coreclr@c94d8e68222d931d4bb1c4eb9a52b4b056e85f12/src/vm/object.cpp#L723
https://github.com/dotnet/coreclr/blob/dotnet/coreclr@c94d8e68222d931d4bb1c4eb9a52b4b056e85f12/src/vm/object.cpp#L730


Commit migrated from dotnet/coreclr@65f8867
  • Loading branch information
erozenfeld committed Nov 24, 2018
1 parent 0cd617e commit a0bfc30
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 11 deletions.
12 changes: 5 additions & 7 deletions src/coreclr/src/jit/objectalloc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -579,14 +579,12 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack<GenTree*>* parent

if (asCall->gtCallType == CT_HELPER)
{
const CorInfoHelpFunc helperNum = comp->eeGetHelperNum(asCall->gtCallMethHnd);
// TODO-ObjectStackAllocation: Special-case helpers here that
// 1. Don't make objects escape.
// 2. Protect objects as interior (GCPROTECT_BEGININTERIOR() instead of GCPROTECT_BEGIN()).
// 3. Don't check that the object is in the heap in ValidateInner.

if (Compiler::s_helperCallProperties.IsPure(helperNum))
{
// Pure helpers don't modify the heap.
// TODO-ObjectStackAllocation: We may be able to special-case more helpers here.
canLclVarEscapeViaParentStack = false;
}
canLclVarEscapeViaParentStack = true;
}
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,6 @@ public static int Main()

CallTestAndVerifyAllocation(AllocateSimpleClassesAndNECompareThem, 1, expectedAllocationKind);

CallTestAndVerifyAllocation(AllocateSimpleClassAndCheckType, 1, expectedAllocationKind);

CallTestAndVerifyAllocation(AllocateSimpleClassAndCast, 7, expectedAllocationKind);

CallTestAndVerifyAllocation(AllocateSimpleClassAndGetField, 7, expectedAllocationKind);

CallTestAndVerifyAllocation(AllocateClassWithNestedStructAndGetField, 5, expectedAllocationKind);
Expand All @@ -116,6 +112,12 @@ public static int Main()
expectedAllocationKind = AllocationKind.Heap;
}

// This test calls CORINFO_HELP_ISINSTANCEOFCLASS
CallTestAndVerifyAllocation(AllocateSimpleClassAndCheckType, 1, expectedAllocationKind);

// This test calls CORINFO_HELP_CHKCASTCLASS_SPECIAL
CallTestAndVerifyAllocation(AllocateSimpleClassAndCast, 7, expectedAllocationKind);

// Stack allocation of classes with GC fields is currently disabled
CallTestAndVerifyAllocation(AllocateSimpleClassWithGCFieldAndAddFields, 12, expectedAllocationKind);

Expand Down

0 comments on commit a0bfc30

Please sign in to comment.