Invalid assertion prop in finally

// Generated by Fuzzlyn v1.2 on 2021-07-03 17:37:53
// Seed: 10069026747560583045
// Reduced from 77.5 KiB to 0.8 KiB in 00:00:53
// Debug: Prints 0 line(s)
// Release: Prints 1 line(s)
public class Program
{
    static ushort s_6;
    static uint[] s_15 = new uint[]{0};
    static bool s_19;
    public static void Main()
    {
        try
        {
            ushort vr4 = s_6;
        }
        finally
        {
            bool vr5 = s_19;
            if (vr5)
            {
                s_15[0] = s_15[0];
            }

            try
            {
                if (vr5)
                {
                    bool[] vr6 = new bool[]{false};
                }
            }
            finally
            {
                if (vr5)
                {
                    bool[] vr7 = new bool[]{false};
                    System.Console.WriteLine(vr7[0]);
                }
            }
        }
    }
}

Looks like we end up somehow removing the vr5 test in the inner finally.

Some notes to help whoever is more familiar with our EH handling figure out where the actual bug is.

Here's a bit less noisy reproduction:

[MethodImpl(MethodImplOptions.NoInlining)]
static bool False() => false;

private static void Problem()
{
    bool condition = False();
    if (condition)
    {
        Console.WriteLine("BB02");
    }

    try
    {
        if (condition)
        {
            Console.WriteLine("BB04");
        }
    }
    finally
    {
        if (condition)
        {
            Console.WriteLine("BB09");
        }
    }
}

Run with DOTNET_JitEnableFinallyCloning=0.

Here's the flow table before assertion prop:

-----------------------------------------------------------------------------------------------------------------------------------------
BBnum BBid ref try hnd preds           weight    lp [IL range]     [jump]      [EH region]         [flags]
-----------------------------------------------------------------------------------------------------------------------------------------
BB01 [0000]  1                             1       [000..009)-> BB05 ( cond )                     i hascall gcsafe 
BB02 [0001]  1       BB01                  1       [009..013)                                     i hascall gcsafe 
BB03 [0003]  1  0    BB02                  1       [013..017)-> BB05 ( cond ) T0      try {       keep i try gcsafe 
BB04 [0004]  1  0    BB03                  1       [017..023)                 T0      }           i hascall gcsafe 
BB05 [0010]  3       BB01,BB03,BB04        1       [???..???)-> BB08 (callf )                     i internal gcsafe 
BB06 [0011]  1       BB10                  1       [???..???)-> BB07 (ALWAYS)                     i internal gcsafe KEEP 
BB07 [0009]  1       BB06                  1       [031..032)        (return)                     i gcsafe 
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ funclets follow
BB08 [0006]  2     0 BB05                  1       [023..026)-> BB10 ( cond )    H0 F finally {   keep i gcsafe flet 
BB09 [0007]  1     0 BB08                  1       [026..030)                    H0               i hascall gcsafe 
BB10 [0008]  2     0 BB08,BB09             1       [030..031)        (finret)    H0   }           i gcsafe 
-----------------------------------------------------------------------------------------------------------------------------------------

The issue arises because dataflow only considers try-blocks to be the predecessors of the handler's first block:

runtime/src/coreclr/jit/dataflow.h

Lines 54 to 58 in ad51267

    
           if (m_pCompiler->bbIsHandlerBeg(block)) 
        
           { 
        
               EHblkDsc* ehDsc = m_pCompiler->ehGetBlockHndDsc(block); 
        
               callback.MergeHandler(block, ehDsc->ebdTryBeg, ehDsc->ebdTryLast); 
        
           }

runtime/src/coreclr/jit/assertionprop.cpp

Lines 4775 to 4779 in 607f98c

    
           void MergeHandler(BasicBlock* block, BasicBlock* firstTryBlock, BasicBlock* lastTryBlock) 
        
           { 
        
               BitVecOps::IntersectionD(apTraits, block->bbAssertionIn, firstTryBlock->bbAssertionIn); 
        
               BitVecOps::IntersectionD(apTraits, block->bbAssertionIn, lastTryBlock->bbAssertionOut); 
        
           }

(This is actually recent code, see #160)

The relevant assertion for the try blocks:

GenTreeNode creates assertion:
N004 (  5,  5) [000006] ------------              *  JTRUE     void  
In BB01 New Global Constant Assertion: ($c0,$40) V00.02 != 0, index = #02

BB03 in = #02
BB04 out = #02

Both are correct on their own, but lead to dataflow concluding that BB08 in = #02, which is not correct because BB08 has an additional predecessor - BB05, the special callf block, which correctly has no assertions generated for it.

Of note is the fact that BB05 has additional predecessors added to it after the redundant branch optimization:

 BBnum BBid ref try hnd preds           weight    lp [IL range]     [jump]      [EH region]         [flags]
 -----------------------------------------------------------------------------------------------------------------------------------------
-BB01 [0000]  1                             1       [000..009)-> BB03 ( cond )                     i hascall gcsafe
+BB01 [0000]  1                             1       [000..009)-> BB05 ( cond )                     i hascall gcsafe
 BB02 [0001]  1       BB01                  1       [009..013)                                     i hascall gcsafe
-BB03 [0003]  2  0    BB01,BB02             1       [013..017)-> BB05 ( cond ) T0      try {       keep i try gcsafe
+BB03 [0003]  1  0    BB02                  1       [013..017)-> BB05 ( cond ) T0      try {       keep i try gcsafe
 BB04 [0004]  1  0    BB03                  1       [017..023)                 T0      }           i hascall gcsafe
-BB05 [0010]  2       BB03,BB04             1       [???..???)-> BB08 (callf )                     i internal gcsafe
+BB05 [0010]  3       BB01,BB03,BB04        1       [???..???)-> BB08 (callf )                     i internal gcsafe
 BB06 [0011]  1       BB10                  1       [???..???)-> BB07 (ALWAYS)                     i internal gcsafe KEEP
 BB07 [0009]  1       BB06                  1       [031..032)        (return)                     i gcsafe

Both successors of idom BB01 reach BB03 -- attempting jump threading
BB01 is a true pred
BB02 is an eh constrained pred
Optimizing via jump threading
Jump flow from pred BB01 -> BB03 implies predicate true; we can safely redirect flow to be BB01 -> BB05

@SingleAccretion

Thanks a lot @SingleAccretion for the short repro and explanation. Helped me in finding the fix!

Triage Area: AssertionProp

Navigation Menu

Repository navigation

Invalid assertion prop in finally #55131

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	if (m_pCompiler->bbIsHandlerBeg(block))
	{
	EHblkDsc* ehDsc = m_pCompiler->ehGetBlockHndDsc(block);
	callback.MergeHandler(block, ehDsc->ebdTryBeg, ehDsc->ebdTryLast);
	}

	void MergeHandler(BasicBlock* block, BasicBlock* firstTryBlock, BasicBlock* lastTryBlock)
	{
	BitVecOps::IntersectionD(apTraits, block->bbAssertionIn, firstTryBlock->bbAssertionIn);
	BitVecOps::IntersectionD(apTraits, block->bbAssertionIn, lastTryBlock->bbAssertionOut);
	}

Invalid assertion prop in finally #55131

Description

Activity

SingleAccretion commented on Jul 4, 2021

kunalspathak commented on Jul 14, 2021

briansull commented on Jul 15, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions