NullReferenceException with bulk memory copy helper on win-x86

[jakobbotsch]

// Generated by Fuzzlyn v1.6 on 2024-05-12 19:09:35
// Run on X86 Windows
// Seed: 9851999836185511275
// Reduced from 93.1 KiB to 0.8 KiB in 00:03:30
// Debug: Runs successfully
// Release: Throws 'System.NullReferenceException'
using System.Runtime.CompilerServices;

public class C0
{
}

public struct S0
{
    public C0 F0;
    public C0 F1;
}

public struct S1
{
    public C0 F1;
    public S0 F2;
    public C0 F3;
}

public struct S2
{
    public S1 F4;
}

public struct S3
{
    public S2 F0;
}

public struct S5
{
    public int F0;
}

public class Program
{
    public static S3 s_10;
    public static byte[] s_11;
    public static void Main()
    {
        var vr3 = new short[][][]{new short[][]{new short[]{0}}};
        var vr8 = new S5();
        s_10 = M8(vr8, ref s_11, vr3);
    }

    public static ref S3 M8(S5 argThis, ref byte[] arg0, short[][][] arg1)
    {
        arg1[0] = new short[][]{new short[]{0}};
        return ref s_10;
    }
}

cc @EgorBo, it looks like the transformation in lowering is not correct for the x86 invariants around the ordering of arguments when they need to be pushed on the stack. The size that is supposed to be passed to the bulk write barrier ends up pushed to M8 as its last parameter here, so inside M8 we have arg1 = 16.

[dotnet-policy-service]

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

[jakobbotsch]

Might also be related to #101890

[EgorBo]

Thanks, this repro is definitely simpler to work with than #101890

[jakobbotsch]

I think the problem is this check inside MoveCFGCallArgs:

runtime/src/coreclr/jit/lower.cpp

Lines 3594 to 3599 in 4b87c13

	// Non-value nodes in early args are setup nodes for late args.
	if (node->IsValue())
	{
	assert(node->OperIsPutArg() || node->OperIsFieldList());
	MoveCFGCallArg(call, node);
	}

On x86 we have PUTARG_STK nodes in the early args that definitely need to be handled here, and unlike other PUTARG nodes this one isn't a value. The check should likely just be deleted, we no longer have setup nodes in calls after rationalization.

[jakobbotsch]

That probably means this issue is unrelated to #101890.

[EgorBo]

can confirm that if (node->IsValue()) being removed fixes the issue 🙂