Skip to content

Commit

Permalink
Fix GC reporting for slow tail call arguments of type Span<T>
Browse files Browse the repository at this point in the history
Fixes https://github.com/dotnet/coreclr/issues/9032:
- Refactored by-ref-like method table walking to find offsets of by-ref pointers in siginfo.hpp/cpp
- Reused that for appending GC layout when creating the copy-args helper for a slow tail call
  • Loading branch information
kouvel committed Nov 2, 2017
1 parent 0bbce0c commit 4b78842
Show file tree
Hide file tree
Showing 5 changed files with 138 additions and 39 deletions.
11 changes: 11 additions & 0 deletions src/vm/i386/stublinkerx86.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5925,6 +5925,17 @@ static void AppendGCLayout(ULONGARRAY &gcLayout, size_t baseOffset, BOOL fIsType
_ASSERTE(pMT);
_ASSERTE(pMT->IsValueType());

if (pMT->IsByRefLike())
{
FindByRefPointerOffsetsInByRefLikeObject(
pMT,
baseOffset,
[&](size_t pointerOffset)
{
*gcLayout.AppendThrowing() = (ULONG)(pointerOffset | 1); // "| 1" to mark it as an interior pointer
});
}

// walk the GC descriptors, reporting the correct offsets
if (pMT->ContainsPointers())
{
Expand Down
46 changes: 8 additions & 38 deletions src/vm/siginfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4946,50 +4946,20 @@ void PromoteCarefully(promote_func fn,
(*fn) (ppObj, sc, flags);
}

void ReportByRefPointersFromByRefLikeObject(promote_func *fn, ScanContext *sc, PTR_MethodTable pMT, PTR_VOID pSrc)
{
WRAPPER_NO_CONTRACT;

_ASSERTE(pMT->IsByRefLike());

// TODO: TypedReference should ideally be implemented as a by-ref-like struct containing a ByReference<T> field,
// in which case the check for g_TypedReferenceMT below would not be necessary
if (pMT == g_TypedReferenceMT || pMT->HasSameTypeDefAs(g_pByReferenceClass))
{
(*fn)(dac_cast<PTR_PTR_Object>(pSrc), sc, GC_CALL_INTERIOR);
return;
}

ApproxFieldDescIterator fieldIterator(pMT, ApproxFieldDescIterator::INSTANCE_FIELDS);
for (FieldDesc *pFD = fieldIterator.Next(); pFD != NULL; pFD = fieldIterator.Next())
{
if (pFD->GetFieldType() != ELEMENT_TYPE_VALUETYPE)
{
continue;
}

// TODO: GetApproxFieldTypeHandleThrowing may throw. This is a potential stress problem for fragile NGen of non-CoreLib
// assemblies. It won’t ever throw for CoreCLR with R2R. Figure out if anything needs to be done to deal with the
// exception.
PTR_MethodTable pFieldMT = pFD->GetApproxFieldTypeHandleThrowing().AsMethodTable();
if (!pFieldMT->IsByRefLike())
{
continue;
}

int fieldStartIndex = pFD->GetOffset() / sizeof(void *);
PTR_PTR_Object fieldRef = dac_cast<PTR_PTR_Object>(PTR_BYTE(pSrc) + fieldStartIndex);
ReportByRefPointersFromByRefLikeObject(fn, sc, pFieldMT, fieldRef);
}
}

void ReportPointersFromValueType(promote_func *fn, ScanContext *sc, PTR_MethodTable pMT, PTR_VOID pSrc)
{
WRAPPER_NO_CONTRACT;

if (pMT->IsByRefLike())
{
ReportByRefPointersFromByRefLikeObject(fn, sc, pMT, pSrc);
FindByRefPointerOffsetsInByRefLikeObject(
pMT,
0 /* baseOffset */,
[&](SIZE_T pointerOffset)
{
PTR_PTR_Object fieldRef = dac_cast<PTR_PTR_Object>(PTR_BYTE(pSrc) + pointerOffset);
(*fn)(fieldRef, sc, GC_CALL_INTERIOR);
});
}

if (!pMT->ContainsPointers())
Expand Down
38 changes: 37 additions & 1 deletion src/vm/siginfo.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1188,7 +1188,43 @@ BOOL CompareTypeDefsForEquivalence(mdToken tk1, mdToken tk2, Module *pModule1, M
BOOL IsTypeDefEquivalent(mdToken tk, Module *pModule);
BOOL IsTypeDefExternallyVisible(mdToken tk, Module *pModule, DWORD dwAttrs);

template<class F>
inline void FindByRefPointerOffsetsInByRefLikeObject(PTR_MethodTable pMT, SIZE_T baseOffset, const F processPointerOffset)
{
WRAPPER_NO_CONTRACT;
_ASSERTE(pMT != nullptr);
_ASSERTE(pMT->IsByRefLike());

// TODO: TypedReference should ideally be implemented as a by-ref-like struct containing a ByReference<T> field,
// in which case the check for g_TypedReferenceMT below would not be necessary
if (pMT == g_TypedReferenceMT || pMT->HasSameTypeDefAs(g_pByReferenceClass))
{
processPointerOffset(baseOffset);
return;
}

ApproxFieldDescIterator fieldIterator(pMT, ApproxFieldDescIterator::INSTANCE_FIELDS);
for (FieldDesc *pFD = fieldIterator.Next(); pFD != NULL; pFD = fieldIterator.Next())
{
if (pFD->GetFieldType() != ELEMENT_TYPE_VALUETYPE)
{
continue;
}

// TODO: GetApproxFieldTypeHandleThrowing may throw. This is a potential stress problem for fragile NGen of non-CoreLib
// assemblies. It won’t ever throw for CoreCLR with R2R. Figure out if anything needs to be done to deal with the
// exception.
PTR_MethodTable pFieldMT = pFD->GetApproxFieldTypeHandleThrowing().AsMethodTable();
if (!pFieldMT->IsByRefLike())
{
continue;
}

SIZE_T fieldStartIndex = pFD->GetOffset() / sizeof(void *);
FindByRefPointerOffsetsInByRefLikeObject(pFieldMT, baseOffset + fieldStartIndex, processPointerOffset);
}
}

void ReportPointersFromValueType(promote_func *fn, ScanContext *sc, PTR_MethodTable pMT, PTR_VOID pSrc);

#endif /* _H_SIGINFO */

53 changes: 53 additions & 0 deletions tests/src/CoreMangLib/system/span/SlowTailCallArgs.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Reflection;
using System.Reflection.Emit;

internal static class Program
{
private static int Main()
{
DynamicMethod dm = new DynamicMethod("TailCaller", typeof(void), new Type[] { typeof(Span<int>) });
ILGenerator il = dm.GetILGenerator();
il.Emit(OpCodes.Ldarg_0);
il.Emit(OpCodes.Ldarg_0);
il.Emit(OpCodes.Ldarg_0);
il.Emit(OpCodes.Ldarg_0);
il.Emit(OpCodes.Ldarg_0);
il.Emit(OpCodes.Tailcall);
il.Emit(OpCodes.Call, typeof(Program).GetMethod("TailCallee", BindingFlags.Static | BindingFlags.NonPublic));
il.Emit(OpCodes.Ret);

var tailCaller = (ActionOfSpanOfInt)dm.CreateDelegate(typeof(ActionOfSpanOfInt));

try
{
for (int i = 0; i < 1000; ++i)
{
GC.KeepAlive(new object());
tailCaller(new Span<int>(new int[] { 42 }));
GC.KeepAlive(new object());
}
}
catch (ArgumentException)
{
return 1; // fail
}

return 100; // pass
}

private static void TailCallee(Span<int> a, Span<int> b, Span<int> c, Span<int> d, Span<int> e)
{
GC.Collect();
for (int i = 0; i < 10000; i++)
GC.KeepAlive(new object());
if (a[0] != 42 || b[0] != 42 || c[0] != 42 || d[0] != 42 || e[0] != 42)
throw new ArgumentException();
}

private delegate void ActionOfSpanOfInt(Span<int> x);
}
29 changes: 29 additions & 0 deletions tests/src/CoreMangLib/system/span/SlowTailCallArgs.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
<PropertyGroup>
<Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
<Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
<SchemaVersion>2.0</SchemaVersion>
<ProjectGuid>{9C964267-06FB-4A77-BC33-2E3A6F3A1648}</ProjectGuid>
<OutputType>Exe</OutputType>
<ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
<SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<CLRTestKind>BuildAndRun</CLRTestKind>
<CLRTestPriority>1</CLRTestPriority>
</PropertyGroup>
<!-- Default configurations to help VS understand the configurations -->
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "></PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' "></PropertyGroup>
<ItemGroup>
<CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
<Visible>False</Visible>
</CodeAnalysisDependentAssemblyPaths>
</ItemGroup>
<ItemGroup>
<!-- Add Compile Object Here -->
<Compile Include="SlowTailCallArgs.cs" />
</ItemGroup>
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
</Project>

0 comments on commit 4b78842

Please sign in to comment.