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

Commit 2f99ca8

Browse files
AndyAyersMSAnipik
authored andcommitted
[release/3.1] Port fix for JIT silent bad code (#27972)
* [release/3.1] Port fix for JIT silent bad code Release/3.1 port of dotnet/runtime#797. Fixes dotnet/runtime#764 The jit might incorrectly order a read from a struct field with an operation that modifies the field, so that the read returns the wrong value. Silent bad code; program behaves incorrectly. Yes, introduced in the 3.0 cycle. Verified the user's test case now passes; no diffs seen in any existing framework or test code. **Low**: the jit is now spilling the eval stack entries to temps in cases where it did not before; this should be conservatively safe. cc: @Brucefo ____ If we're appending an assignment whose LHS is is a location within a local struct, we need to spill all references to that struct from the eval stack. Update the existing logic for this to handle the case where the LHS is a field of a local struct, and the field is updated by unusual means (here, `initobj`). Fixes dotnet/runtime#764. * Fix test
1 parent 1e0eabe commit 2f99ca8

File tree

3 files changed

+101
-5
lines changed

3 files changed

+101
-5
lines changed

src/jit/importer.cpp

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10837,11 +10837,22 @@ void Compiler::impImportBlockCode(BasicBlock* block)
1083710837
}
1083810838
else if (lhs->OperIsBlk())
1083910839
{
10840-
// Check for ADDR(LCL_VAR), or ADD(ADDR(LCL_VAR),CNS_INT))
10841-
// (the latter may appear explicitly in the IL).
10842-
// Local field stores will cause the stack to be spilled when
10843-
// they are encountered.
10844-
lclVar = lhs->AsBlk()->Addr()->IsLocalAddrExpr();
10840+
// Check if LHS address is within some struct local, to catch
10841+
// cases where we're updating the struct by something other than a stfld
10842+
GenTree* addr = lhs->AsBlk()->Addr();
10843+
10844+
// Catches ADDR(LCL_VAR), or ADD(ADDR(LCL_VAR),CNS_INT))
10845+
lclVar = addr->IsLocalAddrExpr();
10846+
10847+
// Catches ADDR(FIELD(... ADDR(LCL_VAR)))
10848+
if (lclVar == nullptr)
10849+
{
10850+
GenTree* lclTree = nullptr;
10851+
if (impIsAddressInLocal(addr, &lclTree))
10852+
{
10853+
lclVar = lclTree->AsLclVarCommon();
10854+
}
10855+
}
1084510856
}
1084610857
if (lclVar != nullptr)
1084710858
{
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
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+
using System.Runtime.CompilerServices;
7+
8+
// Regression test case for importer bug.
9+
// If Release is inlined into Main, the importer may unsafely re-order trees.
10+
11+
public struct Ptr<T> where T: class
12+
{
13+
public Ptr(T value)
14+
{
15+
_value = value;
16+
}
17+
18+
public T Release()
19+
{
20+
T tmp = _value;
21+
_value = null;
22+
return tmp;
23+
}
24+
25+
T _value;
26+
}
27+
28+
class Runtime_764
29+
{
30+
private static int Main(string[] args)
31+
{
32+
Ptr<string> ptr = new Ptr<string>("Hello, world");
33+
34+
bool res = false;
35+
while (res)
36+
{
37+
}
38+
39+
string summary = ptr.Release();
40+
41+
if (summary == null)
42+
{
43+
Console.WriteLine("FAILED");
44+
return -1;
45+
}
46+
else
47+
{
48+
Console.WriteLine("PASSED");
49+
return 100;
50+
}
51+
}
52+
}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
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+
<SchemaVersion>2.0</SchemaVersion>
8+
<ProjectGuid>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</ProjectGuid>
9+
<OutputType>Exe</OutputType>
10+
<ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
11+
<SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
12+
</PropertyGroup>
13+
<!-- Default configurations to help VS understand the configurations -->
14+
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "></PropertyGroup>
15+
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' " />
16+
<ItemGroup>
17+
<CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
18+
<Visible>False</Visible>
19+
</CodeAnalysisDependentAssemblyPaths>
20+
</ItemGroup>
21+
<PropertyGroup>
22+
<DebugType>None</DebugType>
23+
<Optimize>True</Optimize>
24+
</PropertyGroup>
25+
<ItemGroup>
26+
<Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
27+
</ItemGroup>
28+
<ItemGroup>
29+
<Compile Include="$(MSBuildProjectName).cs" />
30+
</ItemGroup>
31+
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
32+
<PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "></PropertyGroup>
33+
</Project>

0 commit comments

Comments
 (0)