Skip to content

Commit de66703

Browse files
[release/8.0-staging] Fix SysV first/second return register GC info mismatch (#116208)
* 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 5b4a588 commit de66703

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
@@ -6173,6 +6173,18 @@ void CodeGen::genCallInstruction(GenTreeCall* call X86_ARG(target_ssize_t stackA
61736173
{
61746174
retSize = emitTypeSize(retTypeDesc->GetReturnRegType(0));
61756175
secondRetSize = emitTypeSize(retTypeDesc->GetReturnRegType(1));
6176+
6177+
if (retTypeDesc->GetABIReturnReg(1) == REG_INTRET)
6178+
{
6179+
// If the second return register is REG_INTRET, then the first
6180+
// return is expected to be in a SIMD register.
6181+
// The emitter has hardcoded belief that retSize corresponds to
6182+
// REG_INTRET and secondRetSize to REG_INTRET_1, so fix up the
6183+
// situation here.
6184+
assert(!EA_IS_GCREF_OR_BYREF(retSize));
6185+
retSize = secondRetSize;
6186+
secondRetSize = EA_UNKNOWN;
6187+
}
61766188
}
61776189
else
61786190
{

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
@@ -1168,6 +1168,15 @@ ReturnKind MethodDesc::ParseReturnKindFromSig(INDEBUG(bool supportStringConstruc
11681168
regKinds[i] = RT_Scalar;
11691169
}
11701170
}
1171+
1172+
if (eeClass->GetEightByteClassification(0) == SystemVClassificationTypeSSE)
1173+
{
1174+
// Skip over SSE types since they do not consume integer registers.
1175+
// An obj/byref in the 2nd eight bytes will be in the first integer register.
1176+
regKinds[0] = regKinds[1];
1177+
regKinds[1] = RT_Scalar;
1178+
}
1179+
11711180
ReturnKind structReturnKind = GetStructReturnKind(regKinds[0], regKinds[1]);
11721181
return structReturnKind;
11731182
}
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)