Fix GC reproting for by-ref-like structs (#16231)
authorKoundinya Veluri <kouvel@users.noreply.github.com>
Tue, 6 Feb 2018 23:59:42 +0000 (15:59 -0800)
committerGitHub <noreply@github.com>
Tue, 6 Feb 2018 23:59:42 +0000 (15:59 -0800)
Fix GC reproting for by-ref-like structs

Fixes https://github.com/dotnet/coreclr/issues/16044:
- Divide by pointer size was removed to produce the proper offset
- Fixed some assertion failures seen in https://github.com/dotnet/coreclr/issues/16229

src/vm/i386/stublinkerx86.cpp
src/vm/object.inl
tests/src/CoreMangLib/system/span/RefStructWithSpan.cs [new file with mode: 0644]
tests/src/CoreMangLib/system/span/RefStructWithSpan.csproj [new file with mode: 0644]

index a11c6a321291a1beca6fe6210dde3ee58375b539..7674464cfd3155abd77596cee718f56df5335a06 100644 (file)
@@ -5925,13 +5925,15 @@ static void AppendGCLayout(ULONGARRAY &gcLayout, size_t baseOffset, BOOL fIsType
         _ASSERTE(pMT);
         _ASSERTE(pMT->IsValueType());
 
-        if (pMT->IsByRefLike())
+        BOOL isByRefLike = pMT->IsByRefLike();
+        if (isByRefLike)
         {
             FindByRefPointerOffsetsInByRefLikeObject(
                 pMT,
                 baseOffset,
                 [&](size_t pointerOffset)
                 {
+                    _ASSERTE(gcLayout.Count() == 0 || pointerOffset > (gcLayout[gcLayout.Count() - 1] & ~(ULONG)1));
                     *gcLayout.AppendThrowing() = (ULONG)(pointerOffset | 1); // "| 1" to mark it as an interior pointer
                 });
         }
@@ -5962,9 +5964,24 @@ static void AppendGCLayout(ULONGARRAY &gcLayout, size_t baseOffset, BOOL fIsType
                 size_t stop = start - (cur->GetSeriesSize() + size);
                 for (size_t off = stop + sizeof(void*); off <= start; off += sizeof(void*))
                 {
-                    _ASSERTE(gcLayout.Count() == 0 || off > gcLayout[gcLayout.Count() - 1]);
                     _ASSERTE(FitsInU4(off));
-                    *gcLayout.AppendThrowing() = (ULONG)off;
+
+                    int gcLayoutInsertIndex = gcLayout.Count();
+                    _ASSERTE(gcLayoutInsertIndex >= 0);
+                    if (isByRefLike)
+                    {
+                        // Offsets in 'gcLayout' are expected to be in increasing order and for by-ref-like types the by-refs would
+                        // have already been inserted into 'gcLayout' above. Find the appropriate index at which to insert this
+                        // offset.
+                        while (gcLayoutInsertIndex != 0 && off < gcLayout[gcLayoutInsertIndex - 1])
+                        {
+                            --gcLayoutInsertIndex;
+                            _ASSERTE(off != (gcLayout[gcLayoutInsertIndex] & ~(ULONG)1));
+                        }
+                    }
+
+                    _ASSERTE(gcLayoutInsertIndex == 0 || off > (gcLayout[gcLayoutInsertIndex - 1] & ~(ULONG)1));
+                    *gcLayout.InsertThrowing(gcLayoutInsertIndex) = (ULONG)off;
                 }
                 cur++;
 
index 7d79554cc9bb2315531f0e925808223ec6227f76..82e55f7844902229134b1ba43694721a1f51a041 100644 (file)
@@ -389,8 +389,7 @@ inline void FindByRefPointerOffsetsInByRefLikeObject(PTR_MethodTable pMT, SIZE_T
             continue;
         }
 
-        SIZE_T fieldStartIndex = pFD->GetOffset() / sizeof(void *);
-        FindByRefPointerOffsetsInByRefLikeObject(pFieldMT, baseOffset + fieldStartIndex, processPointerOffset);
+        FindByRefPointerOffsetsInByRefLikeObject(pFieldMT, baseOffset + pFD->GetOffset(), processPointerOffset);
     }
 }
 
diff --git a/tests/src/CoreMangLib/system/span/RefStructWithSpan.cs b/tests/src/CoreMangLib/system/span/RefStructWithSpan.cs
new file mode 100644 (file)
index 0000000..2af14d6
--- /dev/null
@@ -0,0 +1,96 @@
+// 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.Collections.Generic;
+using System.Threading;
+
+ref struct MyStruct<A, B>
+{
+    static Random r = new Random();
+
+    Span<int> s1, s2, s3, s4, s5, s6, s7, s8, s9, s10;
+
+    public static void Test(int depth)
+    {
+        MyStruct<A, B> u;
+        u.s1 = My.g[r.Next(My.g.Length)];
+        u.s2 = My.g[r.Next(My.g.Length)];
+        u.s3 = My.g[r.Next(My.g.Length)];
+        u.s4 = My.g[r.Next(My.g.Length)];
+        u.s5 = My.g[r.Next(My.g.Length)];
+        u.s6 = My.g[r.Next(My.g.Length)];
+        u.s6 = My.g[r.Next(My.g.Length)];
+        u.s7 = My.g[r.Next(My.g.Length)];
+        u.s8 = My.g[r.Next(My.g.Length)];
+        u.s9 = My.g[r.Next(My.g.Length)];
+        u.s10 = My.g[r.Next(My.g.Length)];
+        Test(depth, u);
+    }
+
+    public static void Test(int depth, MyStruct<A, B> u)
+    {
+        int x1 = u.s1.Length + u.s2.Length + u.s3.Length + u.s4.Length + u.s5.Length +
+                 u.s6.Length + u.s7.Length + u.s8.Length + u.s9.Length + u.s10.Length;
+        int x2 = u.s1[0] + u.s2[0] + u.s3[0] + u.s4[0] + u.s5[0] +
+                 u.s6[0] + u.s7[0] + u.s8[0] + u.s9[0] + u.s10[0];
+        if (x1 != x2)
+            throw new InvalidOperationException();
+
+        if (depth-- == 0)
+            return;
+        MyStruct<KeyValuePair<A, B>, B>.Test(depth);
+        MyStruct<A, KeyValuePair<B, A>>.Test(depth);
+    }
+}
+
+class My
+{
+    static void Stress()
+    {
+        for (; ; )
+        {
+            GC.Collect();
+            Thread.Sleep(1);
+        }
+    }
+
+    static void Churn()
+    {
+        Random r = new Random();
+        for (; ; )
+        {
+            var a = new int[1 + r.Next(100)];
+            a[0] = a.Length;
+            g[r.Next(g.Length)] = a;
+        }
+    }
+
+    public static int[][] g = new int[10000][];
+
+    static int Main()
+    {
+        int[] empty = new int[] { 1 };
+        for (int i = 0; i < g.Length; i++)
+            g[i] = empty;
+
+        var t = new Thread(Stress);
+        t.IsBackground = true;
+        t.Start();
+        t = new Thread(Churn);
+        t.IsBackground = true;
+        t.Start();
+
+        int result = 100; // pass
+        try
+        {
+            MyStruct<int, uint>.Test(3);
+        }
+        catch (InvalidOperationException)
+        {
+            result = 1; // fail
+        }
+        return result;
+    }
+}
diff --git a/tests/src/CoreMangLib/system/span/RefStructWithSpan.csproj b/tests/src/CoreMangLib/system/span/RefStructWithSpan.csproj
new file mode 100644 (file)
index 0000000..fd5a3bf
--- /dev/null
@@ -0,0 +1,31 @@
+<?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>{3801DEB6-DDA1-4E94-B96F-9C04CC083DCC}</ProjectGuid>
+    <OutputType>Exe</OutputType>
+    <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+    <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
+    <LangVersion>latest</LangVersion>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <CLRTestKind>BuildAndRun</CLRTestKind>
+    <CLRTestPriority>1</CLRTestPriority>
+    <GCStressIncompatible>true</GCStressIncompatible>
+  </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="RefStructWithSpan.cs" />
+  </ItemGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+</Project>