diff options
author | Andy Ayers <andya@microsoft.com> | 2018-09-20 15:02:58 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-09-20 15:02:58 -0700 |
commit | e30f187cda3767e1c50b870864de8d0eb5f8582a (patch) | |
tree | 0f892e999ac2a2bba32b90a95636da4cbab63062 | |
parent | 4ca5cd260a1c04f74961faccfad3200fe865a249 (diff) | |
download | coreclr-e30f187cda3767e1c50b870864de8d0eb5f8582a.tar.gz coreclr-e30f187cda3767e1c50b870864de8d0eb5f8582a.tar.bz2 coreclr-e30f187cda3767e1c50b870864de8d0eb5f8582a.zip |
JIT: Fix operand evaluation order for GT_INDEX_ADDR (#20047)
We need to evaluate the array operand first, and it's op1. So evaluate
in that order, and don't allow reversal.
Closes #20040.
-rw-r--r-- | src/jit/flowgraph.cpp | 6 | ||||
-rw-r--r-- | src/jit/gentree.h | 3 | ||||
-rw-r--r-- | tests/src/JIT/Regression/JitBlue/GitHub_20040/GitHub_20040.cs | 58 | ||||
-rw-r--r-- | tests/src/JIT/Regression/JitBlue/GitHub_20040/GitHub_20040.csproj | 34 |
4 files changed, 96 insertions, 5 deletions
diff --git a/src/jit/flowgraph.cpp b/src/jit/flowgraph.cpp index d03c6e379a..18e663f91b 100644 --- a/src/jit/flowgraph.cpp +++ b/src/jit/flowgraph.cpp @@ -18685,10 +18685,10 @@ void Compiler::fgSetTreeSeqHelper(GenTree* tree, bool isLIR) break; case GT_INDEX_ADDR: - // Evaluate the index first, then the array address - assert((tree->gtFlags & GTF_REVERSE_OPS) != 0); - fgSetTreeSeqHelper(tree->AsIndexAddr()->Index(), isLIR); + // Evaluate the array first, then the index.... + assert((tree->gtFlags & GTF_REVERSE_OPS) == 0); fgSetTreeSeqHelper(tree->AsIndexAddr()->Arr(), isLIR); + fgSetTreeSeqHelper(tree->AsIndexAddr()->Index(), isLIR); break; default: diff --git a/src/jit/gentree.h b/src/jit/gentree.h index dbb5a75913..5660241c8a 100644 --- a/src/jit/gentree.h +++ b/src/jit/gentree.h @@ -4257,8 +4257,7 @@ struct GenTreeIndexAddr : public GenTreeOp gtFlags |= GTF_INX_RNGCHK; } - // REVERSE_OPS is set because we must evaluate the index before the array address. - gtFlags |= GTF_EXCEPT | GTF_GLOB_REF | GTF_REVERSE_OPS; + gtFlags |= GTF_EXCEPT | GTF_GLOB_REF; } #if DEBUGGABLE_GENTREE diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_20040/GitHub_20040.cs b/tests/src/JIT/Regression/JitBlue/GitHub_20040/GitHub_20040.cs new file mode 100644 index 0000000000..4403273cd9 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_20040/GitHub_20040.cs @@ -0,0 +1,58 @@ +// 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; + +// GitHub 20040: operand ordering bug with GT_INDEX_ADDR +// Requires minopts/tier0 to repro + +namespace GitHub_20040 +{ + class Program + { + static int Main(string[] args) + { + var array = new byte[] {0x00, 0x01}; + var reader = new BinaryTokenStreamReader(array); + + var val = reader.ReadByte(); + + if (val == 0x01) + { + Console.WriteLine("Pass"); + return 100; + } + else + { + Console.WriteLine($"Fail: val=0x{val:x2}, expected 0x01"); + return 0; + } + } + } + + public class BinaryTokenStreamReader + { + private readonly byte[] currentBuffer; + + public BinaryTokenStreamReader(byte[] input) + { + this.currentBuffer = input; + } + + byte[] CheckLength(out int offset) + { + // In the original code, this logic is more complicated. + // It's simplified here to demonstrate the bug. + offset = 1; + return currentBuffer; + } + + public byte ReadByte() + { + int offset; + var buff = CheckLength(out offset); + return buff[offset]; + } + } +} diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_20040/GitHub_20040.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_20040/GitHub_20040.csproj new file mode 100644 index 0000000000..e191e01217 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_20040/GitHub_20040.csproj @@ -0,0 +1,34 @@ +<?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>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</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>None</DebugType> + <Optimize>True</Optimize> + </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> +</Project> |