JIT: Fix SysV first/second return register GC info mismatch when generating calls

[jakobbotsch]

On SysV it is possible to have mixed SIMD/integer return registers for calls. This leads to a case where the first return register is a SIMD register, while the second return register is an integer register. In that particular case, the emitter would get confused and expect GC information for the second return register to refer to rdx, when in reality it refers to rax.

Fix the problem by detecting the case where the second register is rax, and communicating the right information back to the emitter. Also add a test case that reliably segfaults under DOTNET_GCStress=C without the fix.

Fix #115815

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

[jakobbotsch]

@shushanhf @tomeksowi You may want to check whether LA64/RISCV64 suffer from the same possibility, I am not familiar enough with the ABI there to know.

[jakobbotsch]

/azp run runtime-coreclr gcstress0x3-gcstress0xc

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[tomeksowi]

@shushanhf @tomeksowi You may want to check whether LA64/RISCV64 suffer from the same possibility, I am not familiar enough with the ABI there to know.

Thanks for the heads-up. We have the same problem IMO, the integer field of an int-float struct is passed/returned in a0 regardless whether it comes first or second.

But it doesn't manifest in this testcase because the classification doesn't handle reordered fields well, it bails out here:

runtime/src/coreclr/vm/methodtable.cpp

Lines 2782 to 2794 in 4947a30

	FieldDesc* fields = pMT->GetApproxFieldDescListRaw();
	int elementTypeIndex = typeIndex;
	for (int i = 0; i < nFields; ++i)
	{
	if (i > 0 && fields[i-1].GetOffset() + fields[i-1].GetSize() > fields[i].GetOffset())
	{
	LOG((LF_JIT, LL_EVERYTHING, "FpStructInRegistersInfo:%*s "
	" * fields %s [%i..%i) and %s [%i..%i) overlap, treat as union\n",
	nestingLevel * 4, "",
	fields[i-1].GetDebugName(), fields[i-1].GetOffset(), fields[i-1].GetOffset() + fields[i-1].GetSize(),
	fields[i].GetDebugName(), fields[i].GetOffset(), fields[i].GetOffset() + fields[i].GetSize()));
	return false;
	}

I'll try to fix it so KeyValuePair<Container, double> gets lowered.

[jakobbotsch]

cc @dotnet/jit-contrib PTAL @kunalspathak

[Copilot]

Pull Request Overview

This PR fixes a mismatch in GC info for mixed SIMD/integer return registers on SysV when emitting calls, and adds a repro test that would segfault under GC stress without the fix.

• Adjusts genCallInstruction to swap retSize when the second ABI return register is rax
• Introduces a new JIT test (Runtime_115815) to catch the issue under DOTNET_GCStress=C
• Adds a project file for the new test

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
src/tests/JIT/Regression/JitBlue/Runtime_115815/Runtime_115815.csproj	Adds project file for the new GC-stress repro test
src/tests/JIT/Regression/JitBlue/Runtime_115815/Runtime_115815.cs	Implements the test that exposes the return-reg mismatch
src/coreclr/jit/codegenxarch.cpp	Swaps params.retSize and params.secondRetSize when second return register is REG_INTRET

[kunalspathak]

LGTM

[LuckyXu-HF]

@shushanhf @tomeksowi You may want to check whether LA64/RISCV64 suffer from the same possibility, I am not familiar enough with the ABI there to know.

Thanks for the heads-up. We have the same problem IMO, the integer field of an int-float struct is passed/returned in a0 regardless whether it comes first or second.

But it doesn't manifest in this testcase because the classification doesn't handle reordered fields well, it bails out here:

runtime/src/coreclr/vm/methodtable.cpp

Lines 2782 to 2794 in 4947a30

	FieldDesc* fields = pMT->GetApproxFieldDescListRaw();
	int elementTypeIndex = typeIndex;
	for (int i = 0; i < nFields; ++i)
	{
	if (i > 0 && fields[i-1].GetOffset() + fields[i-1].GetSize() > fields[i].GetOffset())
	{
	LOG((LF_JIT, LL_EVERYTHING, "FpStructInRegistersInfo:%*s "
	" * fields %s [%i..%i) and %s [%i..%i) overlap, treat as union\n",
	nestingLevel * 4, "",
	fields[i-1].GetDebugName(), fields[i-1].GetOffset(), fields[i-1].GetOffset() + fields[i-1].GetSize(),
	fields[i].GetDebugName(), fields[i].GetOffset(), fields[i].GetOffset() + fields[i].GetSize()));
	return false;
	}

I'll try to fix it so KeyValuePair<Container, double> gets lowered.

Thank you, LA64 has the same result with RV64.
Besides, LA64 will get stuck of this new testcase Runtime_115815 under DOTNET_GCStress=1/3, and linux-x64 will also stuck (at least 2h with cpu 100%) under DOTNET_GCStress=1/3, do you encounter the same situation?

[jakobbotsch]

Thank you, LA64 has the same result with RV64.
Besides, LA64 will get stuck of this new testcase Runtime_115815 under DOTNET_GCStress=1/3, and linux-x64 will also stuck (at least 2h with cpu 100%) under DOTNET_GCStress=1/3, do you encounter the same situation?

Yeah, looks like 1000000 allocations is too much for GCStress=1. Thanks for letting me know. Let me open a PR to decrease the number of allocations.