Skip to content

Commit 236643c

Browse files
authored
JIT: fix retyping of BLK ops when parent local is retyped (#116027)
Properly handle cases where we have updated the layout of a struct local (because its fields may now refer to stack allocated objects) and then store or load from just part of that local. Fixes #115979.
1 parent fa05a08 commit 236643c

File tree

3 files changed

+147
-15
lines changed

3 files changed

+147
-15
lines changed

src/coreclr/jit/objectalloc.cpp

Lines changed: 70 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2142,7 +2142,7 @@ void ObjectAllocator::UpdateAncestorTypes(
21422142
assert(parentType == TYP_BYREF);
21432143
parent->ChangeType(newType);
21442144

2145-
// Propgate that upwards.
2145+
// Propagate that upwards.
21462146
//
21472147
++parentIndex;
21482148
keepChecking = true;
@@ -2195,20 +2195,40 @@ void ObjectAllocator::UpdateAncestorTypes(
21952195
else
21962196
{
21972197
assert(tree == parent->AsIndir()->Data());
2198-
GenTree* const addr = parent->AsIndir()->Addr();
21992198

22002199
// If we are storing to a GC struct field, we may need to retype the store
22012200
//
2202-
if (parent->OperIs(GT_STOREIND) && varTypeIsGC(parent->TypeGet()))
2201+
if (varTypeIsGC(parent->TypeGet()))
22032202
{
22042203
parent->ChangeType(newType);
22052204
}
2206-
2207-
// If we are storing a struct, we may need to change the layout
2208-
//
2209-
if (retypeFields && parent->OperIs(GT_STORE_BLK))
2205+
else if (retypeFields && parent->OperIs(GT_STORE_BLK))
22102206
{
2211-
parent->AsBlk()->SetLayout(newLayout);
2207+
GenTreeBlk* const block = parent->AsBlk();
2208+
ClassLayout* const oldLayout = block->GetLayout();
2209+
2210+
if (oldLayout->HasGCPtr())
2211+
{
2212+
if (newLayout->GetSize() == oldLayout->GetSize())
2213+
{
2214+
block->SetLayout(newLayout);
2215+
}
2216+
else
2217+
{
2218+
// We must be storing just a portion of the original local
2219+
//
2220+
assert(newLayout->GetSize() > oldLayout->GetSize());
2221+
2222+
if (newLayout->HasGCPtr())
2223+
{
2224+
block->SetLayout(GetByrefLayout(oldLayout));
2225+
}
2226+
else
2227+
{
2228+
block->SetLayout(GetNonGCLayout(oldLayout));
2229+
}
2230+
}
2231+
}
22122232
}
22132233
}
22142234
break;
@@ -2219,19 +2239,54 @@ void ObjectAllocator::UpdateAncestorTypes(
22192239
{
22202240
// If we are loading from a GC struct field, we may need to retype the load
22212241
//
2222-
if (retypeFields && (varTypeIsGC(parent->TypeGet())))
2242+
if (retypeFields)
22232243
{
2224-
parent->ChangeType(newType);
2244+
bool didRetype = false;
22252245

2226-
if (parent->OperIs(GT_BLK))
2246+
if (varTypeIsGC(parent->TypeGet()))
22272247
{
2228-
parent->AsBlk()->SetLayout(newLayout);
2248+
parent->ChangeType(newType);
2249+
didRetype = true;
22292250
}
2251+
else if (parent->OperIs(GT_BLK))
2252+
{
2253+
GenTreeBlk* const block = parent->AsBlk();
2254+
ClassLayout* const oldLayout = block->GetLayout();
22302255

2231-
++parentIndex;
2232-
keepChecking = true;
2233-
retypeFields = false;
2256+
if (oldLayout->HasGCPtr())
2257+
{
2258+
if (newLayout->GetSize() == oldLayout->GetSize())
2259+
{
2260+
block->SetLayout(newLayout);
2261+
}
2262+
else
2263+
{
2264+
// We must be loading just a portion of the original local
2265+
//
2266+
assert(newLayout->GetSize() > oldLayout->GetSize());
2267+
2268+
if (newLayout->HasGCPtr())
2269+
{
2270+
block->SetLayout(GetByrefLayout(oldLayout));
2271+
}
2272+
else
2273+
{
2274+
block->SetLayout(GetNonGCLayout(oldLayout));
2275+
}
2276+
}
2277+
2278+
didRetype = true;
2279+
}
2280+
}
2281+
2282+
if (didRetype)
2283+
{
2284+
++parentIndex;
2285+
keepChecking = true;
2286+
retypeFields = false;
2287+
}
22342288
}
2289+
22352290
break;
22362291
}
22372292

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
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+
4+
// Generated by Fuzzlyn v3.0 on 2025-05-25 16:50:09
5+
// Run on Arm64 MacOS
6+
// Seed: 18178428402533635742-vectort,vector64,vector128,armadvsimd,armadvsimdarm64,armaes,armarmbase,armarmbasearm64,armcrc32,armcrc32arm64,armdp,armrdm,armrdmarm64,armsha1,armsha256
7+
// Reduced from 127.3 KiB to 0.6 KiB in 00:01:07
8+
// Hits JIT assert in Release:
9+
// Assertion failed 'm_blockLayout->CanAssignFrom(m_src->GetLayout(m_comp))' in 'Program:Main(Fuzzlyn.ExecutionServer.IRuntime)' during 'Morph - Global' (IL size 75; hash 0xade6b36b; FullOpts)
10+
//
11+
// File: /Users/runner/work/1/s/src/coreclr/jit/morphblock.cpp Line: 668
12+
//
13+
14+
using System;
15+
using System.Runtime.CompilerServices;
16+
using System.Runtime.Intrinsics;
17+
// using System.Numerics;
18+
using Xunit;
19+
20+
public class C1
21+
{
22+
}
23+
24+
public struct S1
25+
{
26+
public Vector64<short> F0;
27+
public long F2;
28+
public C1 F6;
29+
public ulong F7;
30+
}
31+
32+
public struct S3
33+
{
34+
public C1 F4;
35+
public S1 F6;
36+
public S3(C1 f4, S1 f6) : this()
37+
{
38+
F4 = f4;
39+
F6 = f6;
40+
}
41+
}
42+
43+
public class Runtime_115979
44+
{
45+
[Fact]
46+
public static int Test()
47+
{
48+
int result = -1;
49+
try
50+
{
51+
Problem();
52+
}
53+
catch (NullReferenceException)
54+
{
55+
result = 100;
56+
}
57+
return result;
58+
}
59+
60+
[MethodImpl(MethodImplOptions.NoInlining)]
61+
static void Problem()
62+
{
63+
S1[] vr0 = default(S1[]);
64+
S3 vr1 = new S3(new C1(), new S1());
65+
S3 vr2 = new S3(vr0[0].F6, vr1.F6);
66+
System.Console.WriteLine(vr2.F6.F0);
67+
}
68+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
<PropertyGroup>
3+
<DebugType>None</DebugType>
4+
<Optimize>True</Optimize>
5+
</PropertyGroup>
6+
<ItemGroup>
7+
<Compile Include="$(MSBuildProjectName).cs" />
8+
</ItemGroup>
9+
</Project>

0 commit comments

Comments
 (0)