Skip to content

Commit 74a39ca

Browse files
[release/9.0-staging] Fix SysV first/second return register GC info mismatch (#116206)
* JIT: Fix SysV first/second return register GC info mismatch when generating calls * Fix struct ReturnKind on SysV AMD64. On SysV AMD64, structs returned in a float and int reg pair were being classified as RT_Scalar_XX. This causes downstream consumers (e.g., HijackFrame::GcScanRoots) to look for obj/byref's in the second int reg. Per the ABI, however, the first float is passed through a float reg and the obj/byref is passed through the _first_ int reg. We now detect and fix this case by skipping the first float type in the ReturnKind encoding and moving the second type into the first. Fix #115815 --------- Co-authored-by: zengandrew <[email protected]>
1 parent c7e6e39 commit 74a39ca

File tree

5 files changed

+86
-2
lines changed

5 files changed

+86
-2
lines changed

src/coreclr/jit/codegenxarch.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6268,6 +6268,18 @@ void CodeGen::genCallInstruction(GenTreeCall* call X86_ARG(target_ssize_t stackA
62686268
{
62696269
retSize = emitTypeSize(retTypeDesc->GetReturnRegType(0));
62706270
secondRetSize = emitTypeSize(retTypeDesc->GetReturnRegType(1));
6271+
6272+
if (retTypeDesc->GetABIReturnReg(1, call->GetUnmanagedCallConv()) == REG_INTRET)
6273+
{
6274+
// If the second return register is REG_INTRET, then the first
6275+
// return is expected to be in a SIMD register.
6276+
// The emitter has hardcoded belief that retSize corresponds to
6277+
// REG_INTRET and secondRetSize to REG_INTRET_1, so fix up the
6278+
// situation here.
6279+
assert(!EA_IS_GCREF_OR_BYREF(retSize));
6280+
retSize = secondRetSize;
6281+
secondRetSize = EA_UNKNOWN;
6282+
}
62716283
}
62726284
else
62736285
{

src/coreclr/jit/gcencode.cpp

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,21 @@ ReturnKind GCInfo::getReturnKind()
4949
case 1:
5050
return VarTypeToReturnKind(retTypeDesc.GetReturnRegType(0));
5151
case 2:
52-
return GetStructReturnKind(VarTypeToReturnKind(retTypeDesc.GetReturnRegType(0)),
53-
VarTypeToReturnKind(retTypeDesc.GetReturnRegType(1)));
52+
{
53+
var_types first = retTypeDesc.GetReturnRegType(0);
54+
var_types second = retTypeDesc.GetReturnRegType(1);
55+
#ifdef UNIX_AMD64_ABI
56+
if (varTypeUsesFloatReg(first))
57+
{
58+
// first does not consume an int register in this case so an obj/ref
59+
// in the second ReturnKind would actually be found in the first int reg.
60+
first = second;
61+
second = TYP_UNDEF;
62+
}
63+
#endif // UNIX_AMD64_ABI
64+
return GetStructReturnKind(VarTypeToReturnKind(first),
65+
VarTypeToReturnKind(second));
66+
}
5467
default:
5568
#ifdef DEBUG
5669
for (unsigned i = 0; i < regCount; i++)

src/coreclr/vm/method.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1324,6 +1324,15 @@ ReturnKind MethodDesc::ParseReturnKindFromSig(INDEBUG(bool supportStringConstruc
13241324
regKinds[i] = RT_Scalar;
13251325
}
13261326
}
1327+
1328+
if (eeClass->GetEightByteClassification(0) == SystemVClassificationTypeSSE)
1329+
{
1330+
// Skip over SSE types since they do not consume integer registers.
1331+
// An obj/byref in the 2nd eight bytes will be in the first integer register.
1332+
regKinds[0] = regKinds[1];
1333+
regKinds[1] = RT_Scalar;
1334+
}
1335+
13271336
ReturnKind structReturnKind = GetStructReturnKind(regKinds[0], regKinds[1]);
13281337
return structReturnKind;
13291338
}
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
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+
using System.Threading;
5+
using System.Collections.Generic;
6+
using System.Runtime.CompilerServices;
7+
using Xunit;
8+
9+
public class Runtime_115815
10+
{
11+
[Fact]
12+
public static void TestEntryPoint()
13+
{
14+
var destination = new KeyValuePair<Container, double>[1_000];
15+
16+
// loop to make this method fully interruptible + to get into OSR version
17+
for (int i = 0; i < destination.Length * 1000; i++)
18+
{
19+
destination[i / 1000] = default;
20+
}
21+
22+
for (int i = 0; i < 5; i++)
23+
{
24+
for (int j = 0; j < destination.Length; j++)
25+
{
26+
destination[j] = GetValue(j);
27+
}
28+
29+
Thread.Sleep(10);
30+
}
31+
}
32+
33+
[MethodImpl(MethodImplOptions.NoInlining)]
34+
private static KeyValuePair<Container, double> GetValue(int i)
35+
=> KeyValuePair.Create(new Container(i.ToString()), (double)i);
36+
37+
private struct Container
38+
{
39+
public string Name;
40+
public Container(string name) { this.Name = name; }
41+
}
42+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
<PropertyGroup>
3+
<Optimize>True</Optimize>
4+
</PropertyGroup>
5+
<ItemGroup>
6+
<Compile Include="$(MSBuildProjectName).cs" />
7+
</ItemGroup>
8+
</Project>

0 commit comments

Comments
 (0)