Incorrect OBJ/IND/BLK removal.

[sandreenko]

@erozenfeld I was experimenting with the suspicious code from #38004 that we have discussed early today and have created a repro, run this on x64 windows checked:

// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Numerics;
using System.Runtime.Intrinsics;
using System.Runtime.Intrinsics.X86;
using System.Runtime.CompilerServices;
using System;
using System.Diagnostics;


class Runtime_39424
{
    struct IntsWrapped
    {
        public int i1;
        public int i2;
        public int i3;
        public int i4;
    };

    [MethodImpl(MethodImplOptions.NoInlining)]
    static unsafe int TestUnusedObjCopy(IntsWrapped* ps)
    {
        IntsWrapped s = *ps;
        return 100;
    }


    public static unsafe int Main()
    {
        try
        {
            TestUnusedObjCopy((IntsWrapped*)0);
            Debug.Assert(false, "unreachable");
        }
        catch
        {
            return 100;
        }
        return -1;
    }
}

the current master will fail with assert or return -1 in release, 3.1 will correctly generate an exception.

The problems are in these two blocks:

runtime/src/coreclr/src/jit/liveness.cpp

Lines 2290 to 2298 in 811c8c7

	// This is a block assignment. An indirection of the rhs is not considered to
	// happen until the assignment, so we will extract the side effects from only
	// the address.
	if (rhsNode->OperIsIndir())
	{
	assert(rhsNode->OperGet() != GT_NULLCHECK);
	rhsNode = rhsNode->AsIndir()->Addr();
	}
	}

runtime/src/coreclr/src/jit/liveness.cpp

Lines 1983 to 1988 in 811c8c7

	if (data->isIndir())
	{
	// This is a block assignment. An indirection of the rhs is not considered
	// to happen until the assignment so mark it as non-faulting.
	data->gtFlags |= GTF_IND_NONFAULTING;
	}

They are not checking that nodes are side-effect free and deleting null-pointer dereferencing. What is interesting, the second block is 4 years old and it what causes the issue in the repro, so I do not know how it works in 3.1.

Another strange thing is that these checks are only for OperIsIndir, I do not see reasons why they should not process OBJ/BLK.

The easiest fix is to delete both blocks and transform IND/OBJ/BLK to NULLCHECK. However, it would be very intersesting to see how it was working in the past.

Found during #39737 work.

I think we should consider implementing #10249 sooner than later with all issues that we see with these flags.

category:correctness
theme:structs
skill-level:intermediate
cost:small

[sandreenko]

cc @erozenfeld , @CarolEidt , @dotnet/jit-contrib