Failures in x86 coreclr pgostress

[AndyAyersMS]

See #95616 (comment)

last good run was at 0dd958b7 dcf2241 5470c79
bad run at e0d447c9

https://dev.azure.com/dnceng-public/public/_build/results?buildId=494156&view=ms.vss-test-web.build-test-results-tab

Windows x86

set DOTNET_TieredCompilation=1
set DOTNET_ReadyToRun=0
set DOTNET_TC_QuickJitForLoops=1
set DOTNET_JitSynthesizeCounts=1

07:38:42.102 Running test: Interop/COM/NETClients/Primitives/NETClientPrimitives/NETClientPrimitives.cmd

Assert failure(PID 3664 [0x00000e50], Thread: 7728 [0x1e30]): m_crawl.pFunc->SanityCheck()

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

Issue Details

---

See #95616 (comment)

last good run was at 0dd958b7
bad run at e0d447c9

https://dev.azure.com/dnceng-public/public/_build/results?buildId=494156&view=ms.vss-test-web.build-test-results-tab

Windows x86

set DOTNET_TieredCompilation=1
set DOTNET_ReadyToRun=0
set DOTNET_TC_QuickJitForLoops=1
set DOTNET_JitSynthesizeCounts=1

07:38:42.102 Running test: Interop/COM/NETClients/Primitives/NETClientPrimitives/NETClientPrimitives.cmd

Assert failure(PID 3664 [0x00000e50], Thread: 7728 [0x1e30]): m_crawl.pFunc->SanityCheck()

Author:	AndyAyersMS
Assignees:	-
Labels:	area-CodeGen-coreclr
Milestone:	-

[AndyAyersMS]

Also noted this in the log...

07:37:16.849 Running test: global::SizeParamIndex.ReversePInvoke.PassingByOutTest.TestEntryPoint()
ReversePInvoke C-Style Array marshaled by out with SizeParamIndex attribute(by out Array size).
	Scenario 1 : byte ==> uint8_t, Array_Size = byte.MinValue, Return_Array_Size = 20
		MarshalByteArray_AsReversePInvokeByOut_AsCdecl Passed!
	Scenario 2 : sbyte ==> CHAR, Array_Size = 1, Return_Array_Size = sbyte.Max
		MarshalSbyteArray_AsReversePInvokeByOut_AsCdecl Passed!
	Scenario 3 : short ==> int16_t, Array_Size = -1, Return_Array_Size = 20
		MarshalShortArray_AsReversePInvokeByOut_AsCdecl Failed!

@jkoritzinsky is the "Failed!" expected here?

[AndyAyersMS]

Tried bisecting just the jit with no luck.

Bisecting full builds from e0d447c down to 5470c79 points at 6dc1087:

FAILS >>> 261bc862aab Generate interface lists for necessary EETypes (#95683)
FAILS >>> 6dc10870f54 Convert HELPER_METHOD_FRAME to QCalls (4/N)  (#95670)
WORKS >>> b79f27cbb36 [Mono] Stop falling back to interpreter for runtime invoke (#95469)
WORKS >>> 31933a32801 JIT: Added SVE_GK_2A and SVE_GL_1A arm64 encodings (#95707)

cc @jkotas

[jkoritzinsky]

is the "Failed!" expected here?

Yes that is expected (it's testing a failure case). This test is one of the ones that was ported from the FX tree (a lot of them are like this for failure case testing).

[jkotas]

Minimum repro

Compile with /o+, run with DOTNET_JitSynthesizeCounts=1 (the crash does not repro with /o- or with DOTNET_JitSynthesizeCounts=0):

using System.Runtime.InteropServices;

CoGetMalloc(1, out var iface);
iface.M(default);

[DllImport("ole32.dll")]
extern static int CoGetMalloc(int reserved, out IFace iface);

[Guid("00000002-0000-0000-C000-000000000046")]
public interface IFace
{
    void M([MarshalAs(UnmanagedType.Currency)] string x);
}

[jkotas]

points at 6dc1087:

My change added more PInvokes. It exposed what looks like a pre-existing problem.

[jkotas]

JitSynthesizeCounts tricks the JIT into inlining PInvoke transition for ThrowInteropParamException helper into IL_STUB_CLRtoCOM.

Working case (JitSynthesizeCounts=0):

test!ILStubClass.IL_STUB_CLRtoCOM(System.String):
051f45f8 55              push    ebp
051f45f9 8bec            mov     ebp,esp
051f45fb b947170000      mov     ecx,1747h
051f4600 ba01000000      mov     edx,1
051f4605 e8ae75adff      call    System.StubHelpers.StubHelpers.ThrowInteropParamException(Int32, Int32) (04ccbbb8)
...

Failing case (JitSynthesizeCounts=1):

test!ILStubClass.IL_STUB_CLRtoCOM(System.String):
03de22b0 55              push    ebp
03de22b1 8bec            mov     ebp,esp
03de22b3 57              push    edi
03de22b4 56              push    esi
03de22b5 53              push    ebx
03de22b6 83ec20          sub     esp,20h
03de22b9 8d7dd8          lea     edi,[ebp-28h]
03de22bc e8331129ff      call    CORINFO_HELP_INIT_PINVOKE_FRAME (030733f4)
03de22c1 8bc5            mov     eax,ebp
03de22c3 8945ec          mov     dword ptr [ebp-14h],eax
03de22c6 6a01            push    1
03de22c8 6847170000      push    1747h
03de22cd c745e078deca03  mov     dword ptr [ebp-20h],3CADE78h
03de22d4 8bc4            mov     eax,esp
03de22d6 8945e4          mov     dword ptr [ebp-1Ch],eax
03de22d9 8d05ec22de03    lea     eax,[test!ILStubClass.IL_STUB_CLRtoCOM(System.String)+0x3c (03de22ec)]
03de22df 8945e8          mov     dword ptr [ebp-18h],eax
03de22e2 c6460800        mov     byte ptr [esi+8],0
03de22e6 ff1500e2ca03    call    dword ptr ds:[3CAE200h] ds:002b:03cae200={System.StubHelpers.StubHelpers.ThrowInteropParamException(Int32, Int32) (0429b048)}
...

@janvorli Is this expected to work, or do we need to further restrict inlining of PInvoke transitions into interop stubs? We have it restricted for stubs with EH (look for impCanPInvokeInlineCallSite in the JIT), but it is not the case here.

[janvorli]

@jkotas Let me take a look, I don't remember

[janvorli]

It is interesting. The code fails because it tries to use a garbage MethodDesc. It gets it here:

runtime/src/coreclr/vm/stackwalk.cpp

Line 3151 in 3e24b58

m_crawl.pFunc = ((PTR_InlinedCallFrame)m_crawl.pFrame)->GetActualInteropMethodDesc();

The implementation of the GetActualInteropMethodDesc fetches it from a memory location right after the InlinedCallFrame:

runtime/src/coreclr/vm/frames.h

Lines 2842 to 2860 in 3e24b58

	PTR_MethodDesc GetActualInteropMethodDesc()
	{
	#if defined(TARGET_X86) || defined(TARGET_ARM)
	// Important: This code relies on the way JIT lays out frames. Keep it in sync
	// with code:Compiler.lvaAssignFrameOffsets.
	//
	// | ... |
	// +--------------------+
	// | lvaStubArgumentVar | <= filled with EAX in prolog |
	// +--------------------+ |
	// | | |
	// | InlinedCallFrame | |
	// | | <= m_pCrawl.pFrame | to lower addresses
	// +--------------------+ V
	// | ... |
	//
	// Extract the actual MethodDesc to report from the InlinedCallFrame.
	TADDR addr = dac_cast<TADDR>(this) + sizeof(InlinedCallFrame);
	return PTR_MethodDesc(*PTR_TADDR(addr));

However the ILStubClass.IL_STUB_CLRtoCOM code stores it into the InlinedCallFrame::m_Datum only, not after the InlinedCallFrame storage.

[jkotas]

Does it mean we need to disallow inlining of any other other PInvokes into the PInvoke stub?