Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit ad096c8

Browse files
authored
JIT: Fix operand evaluation order for GT_INDEX_ADDR (#20047) (#20127)
We need to evaluate the array operand first, and it's op1. So evaluate in that order, and don't allow reversal. Closes #20040.
1 parent 7a18d04 commit ad096c8

File tree

4 files changed

+96
-5
lines changed

4 files changed

+96
-5
lines changed

src/jit/flowgraph.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18825,10 +18825,10 @@ void Compiler::fgSetTreeSeqHelper(GenTree* tree, bool isLIR)
1882518825
break;
1882618826

1882718827
case GT_INDEX_ADDR:
18828-
// Evaluate the index first, then the array address
18829-
assert((tree->gtFlags & GTF_REVERSE_OPS) != 0);
18830-
fgSetTreeSeqHelper(tree->AsIndexAddr()->Index(), isLIR);
18828+
// Evaluate the array first, then the index....
18829+
assert((tree->gtFlags & GTF_REVERSE_OPS) == 0);
1883118830
fgSetTreeSeqHelper(tree->AsIndexAddr()->Arr(), isLIR);
18831+
fgSetTreeSeqHelper(tree->AsIndexAddr()->Index(), isLIR);
1883218832
break;
1883318833

1883418834
default:

src/jit/gentree.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4387,8 +4387,7 @@ struct GenTreeIndexAddr : public GenTreeOp
43874387
gtFlags |= GTF_INX_RNGCHK;
43884388
}
43894389

4390-
// REVERSE_OPS is set because we must evaluate the index before the array address.
4391-
gtFlags |= GTF_EXCEPT | GTF_GLOB_REF | GTF_REVERSE_OPS;
4390+
gtFlags |= GTF_EXCEPT | GTF_GLOB_REF;
43924391
}
43934392

43944393
#if DEBUGGABLE_GENTREE
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System;
6+
7+
// GitHub 20040: operand ordering bug with GT_INDEX_ADDR
8+
// Requires minopts/tier0 to repro
9+
10+
namespace GitHub_20040
11+
{
12+
class Program
13+
{
14+
static int Main(string[] args)
15+
{
16+
var array = new byte[] {0x00, 0x01};
17+
var reader = new BinaryTokenStreamReader(array);
18+
19+
var val = reader.ReadByte();
20+
21+
if (val == 0x01)
22+
{
23+
Console.WriteLine("Pass");
24+
return 100;
25+
}
26+
else
27+
{
28+
Console.WriteLine($"Fail: val=0x{val:x2}, expected 0x01");
29+
return 0;
30+
}
31+
}
32+
}
33+
34+
public class BinaryTokenStreamReader
35+
{
36+
private readonly byte[] currentBuffer;
37+
38+
public BinaryTokenStreamReader(byte[] input)
39+
{
40+
this.currentBuffer = input;
41+
}
42+
43+
byte[] CheckLength(out int offset)
44+
{
45+
// In the original code, this logic is more complicated.
46+
// It's simplified here to demonstrate the bug.
47+
offset = 1;
48+
return currentBuffer;
49+
}
50+
51+
public byte ReadByte()
52+
{
53+
int offset;
54+
var buff = CheckLength(out offset);
55+
return buff[offset];
56+
}
57+
}
58+
}
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
3+
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
4+
<PropertyGroup>
5+
<Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
6+
<Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
7+
<AssemblyName>$(MSBuildProjectName)</AssemblyName>
8+
<SchemaVersion>2.0</SchemaVersion>
9+
<ProjectGuid>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</ProjectGuid>
10+
<OutputType>Exe</OutputType>
11+
<ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
12+
<SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
13+
</PropertyGroup>
14+
<!-- Default configurations to help VS understand the configurations -->
15+
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "></PropertyGroup>
16+
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' "></PropertyGroup>
17+
<ItemGroup>
18+
<CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
19+
<Visible>False</Visible>
20+
</CodeAnalysisDependentAssemblyPaths>
21+
</ItemGroup>
22+
<PropertyGroup>
23+
<DebugType>None</DebugType>
24+
<Optimize>True</Optimize>
25+
</PropertyGroup>
26+
<ItemGroup>
27+
<Compile Include="$(MSBuildProjectName).cs" />
28+
</ItemGroup>
29+
<ItemGroup>
30+
<Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
31+
</ItemGroup>
32+
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
33+
<PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "></PropertyGroup>
34+
</Project>

0 commit comments

Comments
 (0)