diff options
author | Brian Sullivan <briansul@microsoft.com> | 2018-03-13 17:16:44 -0700 |
---|---|---|
committer | Brian Sullivan <briansul@microsoft.com> | 2018-03-14 15:21:49 -0700 |
commit | b46f8e8358f9cee049eda7eda3707d4ccdc960f9 (patch) | |
tree | 09a24cef2948c45787927d9dbd7870345cd0bd72 | |
parent | 76be5e3928cbfa01e96086e532a6476f1acac293 (diff) | |
download | coreclr-b46f8e8358f9cee049eda7eda3707d4ccdc960f9.tar.gz coreclr-b46f8e8358f9cee049eda7eda3707d4ccdc960f9.tar.bz2 coreclr-b46f8e8358f9cee049eda7eda3707d4ccdc960f9.zip |
Fix GH Issue 16892 - GC hole due to GT_INDEX_ADDR
Added test case
-rw-r--r-- | src/jit/compiler.cpp | 10 | ||||
-rw-r--r-- | src/jit/flowgraph.cpp | 2 | ||||
-rw-r--r-- | src/jit/gentree.h | 2 | ||||
-rw-r--r-- | src/jit/morph.cpp | 2 | ||||
-rw-r--r-- | tests/src/JIT/Regression/JitBlue/GitHub_16892/GitHub_16892.cs | 104 | ||||
-rw-r--r-- | tests/src/JIT/Regression/JitBlue/GitHub_16892/GitHub_16892.csproj | 41 |
6 files changed, 156 insertions, 5 deletions
diff --git a/src/jit/compiler.cpp b/src/jit/compiler.cpp index dc68ce0af5..bc0159ca2e 100644 --- a/src/jit/compiler.cpp +++ b/src/jit/compiler.cpp @@ -9546,10 +9546,6 @@ int cTreeFlagsIR(Compiler* comp, GenTree* tree) case GT_INDEX: - if (tree->gtFlags & GTF_INX_RNGCHK) - { - chars += printf("[INX_RNGCHK]"); - } if (tree->gtFlags & GTF_INX_REFARR_LAYOUT) { chars += printf("[INX_REFARR_LAYOUT]"); @@ -9558,6 +9554,12 @@ int cTreeFlagsIR(Compiler* comp, GenTree* tree) { chars += printf("[INX_STRING_LAYOUT]"); } + __fallthrough; + case GT_INDEX_ADDR: + if (tree->gtFlags & GTF_INX_RNGCHK) + { + chars += printf("[INX_RNGCHK]"); + } break; case GT_IND: diff --git a/src/jit/flowgraph.cpp b/src/jit/flowgraph.cpp index ccf70dcddf..b1a4fef8e2 100644 --- a/src/jit/flowgraph.cpp +++ b/src/jit/flowgraph.cpp @@ -23615,6 +23615,8 @@ Compiler::fgWalkResult Compiler::fgChkThrowCB(GenTree** pTree, fgWalkData* data) break; case GT_INDEX: + case GT_INDEX_ADDR: + // These two call CORINFO_HELP_RNGCHKFAIL for Debug code if (tree->gtFlags & GTF_INX_RNGCHK) { return Compiler::WALK_ABORT; diff --git a/src/jit/gentree.h b/src/jit/gentree.h index 35c215cf21..c33cb51d58 100644 --- a/src/jit/gentree.h +++ b/src/jit/gentree.h @@ -893,7 +893,7 @@ public: #define GTF_FLD_VOLATILE 0x40000000 // GT_FIELD/GT_CLS_VAR -- same as GTF_IND_VOLATILE #define GTF_FLD_INITCLASS 0x20000000 // GT_FIELD/GT_CLS_VAR -- field access requires preceding class/static init helper -#define GTF_INX_RNGCHK 0x80000000 // GT_INDEX -- the array reference should be range-checked. +#define GTF_INX_RNGCHK 0x80000000 // GT_INDEX/GT_INDEX_ADDR -- the array reference should be range-checked. #define GTF_INX_REFARR_LAYOUT 0x20000000 // GT_INDEX #define GTF_INX_STRING_LAYOUT 0x40000000 // GT_INDEX -- this uses the special string array layout diff --git a/src/jit/morph.cpp b/src/jit/morph.cpp index aeca2feb80..1e2b1cbc52 100644 --- a/src/jit/morph.cpp +++ b/src/jit/morph.cpp @@ -1590,6 +1590,7 @@ void fgArgInfo::ArgsComplete() // This means unnesting, sorting, etc. Technically this is overly // conservative, but I want to avoid as much special-case debug-only code // as possible, so leveraging the GTF_CALL flag is the easiest. + // if (!(argx->gtFlags & GTF_CALL) && (argx->gtFlags & GTF_EXCEPT) && (argCount > 1) && compiler->opts.compDbgCode && (compiler->fgWalkTreePre(&argx, Compiler::fgChkThrowCB) == Compiler::WALK_ABORT)) @@ -6063,6 +6064,7 @@ GenTree* Compiler::fgMorphArrayIndex(GenTree* tree) indexAddr->gtFlags |= (array->gtFlags | index->gtFlags) & GTF_ALL_EFFECT; // Mark the indirection node as needing a range check if necessary. + // Note this will always be true unless JitSkipArrayBoundCheck() is used if ((indexAddr->gtFlags & GTF_INX_RNGCHK) != 0) { fgSetRngChkTarget(indexAddr); diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_16892/GitHub_16892.cs b/tests/src/JIT/Regression/JitBlue/GitHub_16892/GitHub_16892.cs new file mode 100644 index 0000000000..3621f6f927 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_16892/GitHub_16892.cs @@ -0,0 +1,104 @@ +using System; +using System.Runtime.CompilerServices; +// +// Test case for a GC Stress 4 failure +// +// This test was failing during a GC Stress 4 run in the method Test(...) +// +// The failure requires that this test be built with Debug codegen +// +// The GC Stress failure will occur if the JIT +// 1. has evaluated and stored the two outgoing stack based arguments: a5, a6 +// 2. and then performs a call to the helper CORINFO_HELP_RNGCHKFAIL +// +// With the fix the JIT will evaluate the arr[3] with the rangecheck +// into a new compiler temp, before storing any outgoing arguments. +// + +class Item +{ + int _value; + + [MethodImpl(MethodImplOptions.NoInlining)] + public Item(int value) { _value = value; } + + [MethodImpl(MethodImplOptions.NoInlining)] + public int GetValue() { return _value; } +} + +class Program +{ + public Item[] itemArray; + + [MethodImpl(MethodImplOptions.NoInlining)] + void Init() + { + itemArray = new Item[11]; + for (int i=0; i<11; i++) + { + itemArray[i] = new Item(i); + } + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int Compute(Item r1, Item r2, Item r3, Item r4, Item s5, Item s6) + { + int result = r1.GetValue(); + result += r2.GetValue(); + result += r3.GetValue(); + result += r4.GetValue(); + result += s5.GetValue(); + result += s6.GetValue(); + return result; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + int Test(Item a1, Item a2, Item a4, Item a5, Item a6) + { + Item[] arr = itemArray; + int result = 0; + + // Insure that we have to generate fully interruptible GC information + // Form a possible infinte loop that the JIT believes could execute + // without encountering a GC safe point. + // + do { + if (result < 5) + { + result = Compute(a1, a2, arr[3], a4, a5, a6); + } + } while (result < 0); + + return result; + + } + + static int Main(string[] args) + { + Program prog = new Program(); + + prog.Init(); + + Item[] arr = prog.itemArray; + + Item obj1 = arr[1]; + Item obj2 = arr[2]; + Item obj3 = arr[3]; + Item obj4 = arr[4]; + Item obj5 = arr[5]; + Item obj6 = arr[6]; + + int result = prog.Test(obj1, obj2, obj4, obj5, obj6); + + if (result == 21) + { + Console.WriteLine("Passed"); + return 100; + } + else + { + Console.WriteLine("Failed"); + return -1; + } + } +} diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_16892/GitHub_16892.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_16892/GitHub_16892.csproj new file mode 100644 index 0000000000..0ee4f4aaa2 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_16892/GitHub_16892.csproj @@ -0,0 +1,41 @@ +<?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> + <AssemblyName>$(MSBuildProjectName)</AssemblyName> + <SchemaVersion>2.0</SchemaVersion> + <ProjectGuid>{76E69AA0-8C5A-4F76-8561-B8089FFA8D79}</ProjectGuid> + <OutputType>Exe</OutputType> + <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids> + <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir> + + </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> + <PropertyGroup> + <DebugType>Full</DebugType> + <Optimize>False</Optimize> + <AllowUnsafeBlocks>True</AllowUnsafeBlocks> + </PropertyGroup> + <ItemGroup> + <Compile Include="$(MSBuildProjectName).cs" /> + </ItemGroup> + <ItemGroup> + <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" /> + </ItemGroup> + <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" /> + <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "></PropertyGroup> + <PropertyGroup> + <ProjectAssetsFile>$(JitPackagesConfigFileDirectory)benchmark\obj\project.assets.json</ProjectAssetsFile> + </PropertyGroup> +</Project> |