Don't assume objects don't escape in pure helpers.

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/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/c94d8e68222d931d4bb1c4eb9a52b4b056e85f12/src/vm/object.cpp#L723 https://github.com/dotnet/coreclr/blob/c94d8e68222d931d4bb1c4eb9a52b4b056e85f12/src/vm/object.cpp#L730
dotnet · Nov 24, 2018 · 65f8867 · 65f8867
1 parent b5c9edd
commit 65f8867
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 11 deletions.
diff --git a/src/jit/objectalloc.cpp b/src/jit/objectalloc.cpp
@@ -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;
             }

diff --git a/tests/src/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs b/tests/src/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs
@@ -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);
@@ -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);