From a0bfc3087a3774e05172d656c423851425035c8b Mon Sep 17 00:00:00 2001 From: Eugene Rozenfeld Date: Fri, 23 Nov 2018 14:55:02 -0800 Subject: [PATCH] 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/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 | 12 +++++------- .../ObjectStackAllocationTests.cs | 10 ++++++---- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/coreclr/src/jit/objectalloc.cpp b/src/coreclr/src/jit/objectalloc.cpp index 00732ec750f44..a5e240509be76 100644 --- a/src/coreclr/src/jit/objectalloc.cpp +++ b/src/coreclr/src/jit/objectalloc.cpp @@ -579,14 +579,12 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* 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/src/coreclr/tests/src/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs b/src/coreclr/tests/src/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs index fc4fc15eea7d8..9e665244678e5 100644 --- a/src/coreclr/tests/src/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs +++ b/src/coreclr/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);