Don't assume objects don't escape in pure helpers.
authorEugene Rozenfeld <erozen@microsoft.com>
Fri, 23 Nov 2018 22:55:02 +0000 (14:55 -0800)
committerEugene Rozenfeld <erozen@microsoft.com>
Sat, 24 Nov 2018 06:33:32 +0000 (22:33 -0800)
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 https://github.com/dotnet/coreclr/commit/65f88672f888e893a44f21b59ecfd87f4d17e499

src/coreclr/src/jit/objectalloc.cpp
src/coreclr/tests/src/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs

index 00732ec..a5e2405 100644 (file)
@@ -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;
             }
index fc4fc15..9e66524 100644 (file)
@@ -101,10 +101,6 @@ namespace ObjectStackAllocation
 
             CallTestAndVerifyAllocation(AllocateSimpleClassesAndNECompareThem, 1, expectedAllocationKind);
 
-            CallTestAndVerifyAllocation(AllocateSimpleClassAndCheckType, 1, expectedAllocationKind);
-
-            CallTestAndVerifyAllocation(AllocateSimpleClassAndCast, 7, expectedAllocationKind);
-
             CallTestAndVerifyAllocation(AllocateSimpleClassAndGetField, 7, expectedAllocationKind);
 
             CallTestAndVerifyAllocation(AllocateClassWithNestedStructAndGetField, 5, expectedAllocationKind);
@@ -116,6 +112,12 @@ namespace ObjectStackAllocation
                 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);