runtime: Fix incorrect `GTF_IND_NONFAULTING` setting that leads to an incorrect execution.

For such a test case:

public sealed class TestClass<T>
{
    private const int MaxBuffersPerArraySizePerCore = 8;
    private T[] _arrays;

    [MethodImpl(MethodImplOptions.NoInlining)]
    public void TestNullRef()
    {
        _arrays = new T[GetSize()] ;
    }
}

we will generate a tree like:

               [000008] -ACXG+------              *  ASG       ref   
               [000007] ---XG+-N----              +--*  IND       ref   
               [000014] -----+------              |  \--*  ADD       byref 
               [000000] -----+------              |     +--*  LCL_VAR   ref    V00 this         
               [000013] -----+------              |     \--*  CNS_INT   long   8 field offset Fseq[_arrays]
               [000006] --CXG+------              \--*  CALL help ref    HELPER.CORINFO_HELP_NEWARR_1_R2R_DIRECT
               [000017] -ACXGO----L- arg0 SETUP      +--*  ASG       long  
               [000016] D------N----                 |  +--*  LCL_VAR   long   V02 tmp1         
               [000004] --CXG+------                 |  \--*  CALL help r2r_ind long   HELPER.CORINFO_HELP_READYTORUN_GENERIC_HANDLE
               [000003] #----+------ arg0 in rcx     |     \--*  IND       long  
               [000002] -----+------                 |        \--*  LCL_VAR   ref    V00 this         
               [000018] ------------ arg0 in rcx     +--*  LCL_VAR   long   V02 tmp1         
               [000001] -----+------ arg1 in rdx     \--*  CNS_INT   long   8

where 000003 INDIR doesn’t have an exception flag and because clever morph can see that we have already dereferenced it in 000007 INDIR:

Morphing args for 4.CALL:

Non-null prop for index dotnet/coreclr#1 in BB01:
               [000003] #--X--------              *  IND       long  
argSlots=1, preallocatedArgCount=4, nextSlotNum=4, outgoingArgSpaceSize=32

Sorting the arguments:
Deferred argument ('rcx'):
               [000003] #----+------              *  IND       long  
               [000002] -----+------              \--*  LCL_VAR   ref    V00 this   

so far so good, but then fgSetBlockOrder could decide to revert 000008 and guess what now it can clear the exception flag from 000007 because now we execute it after 000003 that is known not to throw (because it got GTF_IND_NONFAULTING during the previous phase) so we end up proving that this indirection can’t throw:

N015 ( 41, 22) [000008] -ACXGO--R---              *  ASG       ref    $VN.Void
N014 (  4,  4) [000007] n---GO-N----              +--*  IND       ref    $280
N013 (  2,  2) [000014] -------N----              |  \--*  ADD       byref  $2c0
N011 (  1,  1) [000000] ------------              |     +--*  LCL_VAR   ref    V00 this         u:1 $80
N012 (  1,  1) [000013] ------------              |     \--*  CNS_INT   long   8 field offset Fseq[_arrays] $200
N010 ( 36, 17) [000006] -ACXGO------              \--*  CALL help ref    HELPER.CORINFO_HELP_NEWARR_1_R2R_DIRECT $103
N006 ( 17,  8) [000017] -ACXGO--R-L- arg0 SETUP      +--*  ASG       long   $180
N005 (  1,  1) [000016] D------N----                 |  +--*  LCL_VAR   long   V02 tmp1         d:1 $c1
N004 ( 17,  8) [000004] --CXGO------                 |  \--*  CALL help r2r_ind long   HELPER.CORINFO_HELP_READYTORUN_GENERIC_HANDLE $180
N003 (  3,  2) [000003] #----O------ arg0 in rcx     |     \--*  IND       long   $100
N002 (  1,  1) [000002] ------------                 |        \--*  LCL_VAR   ref    V00 this         u:1 $80
N008 (  1,  1) [000018] ------------ arg0 in rcx     +--*  LCL_VAR   long   V02 tmp1         u:1 (last use) $c1
N009 (  1,  1) [000001] ------------ arg1 in rdx     \--*  CNS_INT   long   8 $200

when it is, of course, incorrect: Update: that is correct in this case because this is never null (we check it before a call to an instance method).

TestClass<Object> obj = null; obj.TestNullRef();

we set revert flag here: https://github.com/dotnet/coreclr/blob/68043c6306a0449bb5ed10fbdea9f14dafc99df4/src/jit/gentree.cpp#L4004-L4023

under conditions that are not clear to me.

Summary: if we fix dotnet/runtime#13758 we will have GTF_ASG on 000006. Then 000008 won’t be reverted (because of the strange condition that I showed earlier that checks GTF_ASG), so we won’t incorrectly drop GTF_EXC from both indirections. However, it will lead to 0.07% code size regression in System.Private.Corelib and 0.31% on framework libraries and it is not a complete fix for the problem.

Link dotnet/runtime#10813 and dotnet/coreclr#19334.

category:correctness theme:optimization skill-level:intermediate cost:medium