Skip to content

Checked casts can be silently dropped by assertion prop #54842

Description

@SingleAccretion

Reproduction:

.assembly RyuJitReproduction.IL
{
  .ver 1:0:0:0
}

.assembly extern System.Console { auto }

.class public abstract sealed beforefieldinit ILClass extends [netstandard]System.Object
{
   .method public static void Main() cil managed
   {
     .entrypoint
     
     ldc.i8 -1
     call int32 ILClass::CheckedCast(int64)
     call void [System.Console]System.Console::WriteLine(int32)
     
     ret
   }

  .method public static int32 CheckedCast(int64 src) cil managed noinlining
  {
    .locals (
      [0] int32 local
    )

    ldarg src
    ldc.i4 16
    shr
    conv.u4
    stloc local

    ldloc local
    conv.ovf.u4
    ret
  }
}
> dotnet run
Unhandled exception. System.OverflowException: Arithmetic operation resulted in an overflow.

> $env:DOTNET_TieredCompilation=0
> dotnet run
-1

The issue, as one would expect, is the confusion created by the use of "unsigned" type as the target of the cast:

fgMorphTree BB01, STMT00000 (before)
               [000005] -A----------              *  ASG       int   
               [000004] D------N----              +--*  LCL_VAR   int    V01 loc0         
               [000003] ------------              \--*  CAST      int <- uint <- long
               [000002] ------------                 \--*  RSH       long  
               [000000] ------------                    +--*  LCL_VAR   long   V00 arg0         
               [000001] ------------                    \--*  CNS_INT   int    16
Notify VM instruction set (SSE2) must be supported.
GenTreeNode creates assertion:
               [000005] -A----------              *  ASG       int   
In BB01 New Local Subrange Assertion: V01 in [0..-1] index=#01, mask=0000000000000001

fgMorphTree BB01, STMT00001 (before)
               [000008] ---X--------              *  RETURN    int   
               [000007] ---X--------              \--*  CAST_ovfl int <- uint <- int
               [000006] ------------                 \--*  LCL_VAR   int    V01 loc0         

Subrange prop for index #01 in BB01:
               [000007] ---X--------              *  CAST_ovfl int <- uint <- int

fgMorphTree BB01, STMT00001 (after)
               [000008] -----+------              *  RETURN    int   
               [000006] -----+------              \--*  LCL_VAR   int    V01 loc0         

First, we assign the "lower and upper" bound here:

assertion.op2.u2.loBound = AssertionDsc::GetLowerBoundForIntegralType(toType);
assertion.op2.u2.hiBound = AssertionDsc::GetUpperBoundForIntegralType(toType);

Then optAssertionIsSubrange finds the assertion - the only check it performs for TYP_UINTis that the loBound >= 0.

It appears that the assertion prop functions don't quite take 2s complement into account, computing everything "as if to infinite precision". This is correct in case of small types (since TYP_INT can represent any small type), but not so in case of TYP_INT/LONG and their unsigned counterparts.

Since assertions in the form of V01 is in range [INT32_MIN..INT32_MAX] are not very useful anyway, probably the safest and easiest fix is to just not make them. Though with the code as we have it now I wonder if there are similar bugs lurking nearby.

(Separately, we should not import conv.u4 as a cast to TYP_UINT, it is a cast to TYP_INT just like conv.i4).

Activity

  1. ghost added
    area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
    on Jun 28, 2021
  2. ghost added
    untriagedNew issue has not been triaged by the area owner
    on Jun 28, 2021
  3. removed
    untriagedNew issue has not been triaged by the area owner
    on Jun 28, 2021
  4. added this to the Future milestone on Jun 28, 2021
  5. JulieLeeMSFT commented on Jun 28, 2021

    @JulieLeeMSFT
    Member

    Setting it to Future since there is not enough time to handle this in .NET6.

  6. ghost added
    in-prThere is an active PR which will close this issue when it is merged
    on Jul 5, 2021
  7. ghost locked as resolved and limited conversation to collaborators on Aug 14, 2021
  8. ghost removed
    in-prThere is an active PR which will close this issue when it is merged
    on Sep 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions