Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JIT: Assertion failed '!"Bad CastToType() in gtFoldExprConst() for a cast from long" #90798

Closed
jakobbotsch opened this issue Aug 18, 2023 · 9 comments · Fixed by #90981
Closed
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone

Comments

@jakobbotsch
Copy link
Member

// Generated by Fuzzlyn v1.6 on 2023-08-18 14:29:26
// Run on X64 Windows
// Seed: 8230539981346104025
// Reduced from 106.2 KiB to 0.3 KiB in 00:00:51
// Hits JIT assert in Release:
// Assertion failed '!"Bad CastToType() in gtFoldExprConst() for a cast from long"' in 'Program:Main(Fuzzlyn.ExecutionServer.IRuntime)' during 'Morph - Global' (IL size 43; hash 0xade6b36b; FullOpts)
//
//     File: C:\dev\dotnet\runtime4\src\coreclr\jit\gentree.cpp Line: 15048
//
using System.Runtime.CompilerServices;

public struct S1
{
    public ulong F0;
}

public class Program
{
    public static void Main()
    {
        S1 vr2 = new S1();
        bool vr3 = Unsafe.ReadUnaligned<bool>(ref Unsafe.As<ulong, byte>(ref vr2.F0));
        System.Console.WriteLine(vr3);
    }
}
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Aug 18, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Aug 18, 2023
@ghost
Copy link

ghost commented Aug 18, 2023

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

Issue Details
// Generated by Fuzzlyn v1.6 on 2023-08-18 14:29:26
// Run on X64 Windows
// Seed: 8230539981346104025
// Reduced from 106.2 KiB to 0.3 KiB in 00:00:51
// Hits JIT assert in Release:
// Assertion failed '!"Bad CastToType() in gtFoldExprConst() for a cast from long"' in 'Program:Main(Fuzzlyn.ExecutionServer.IRuntime)' during 'Morph - Global' (IL size 43; hash 0xade6b36b; FullOpts)
//
//     File: C:\dev\dotnet\runtime4\src\coreclr\jit\gentree.cpp Line: 15048
//
using System.Runtime.CompilerServices;

public struct S1
{
    public ulong F0;
}

public class Program
{
    public static void Main()
    {
        S1 vr2 = new S1();
        bool vr3 = Unsafe.ReadUnaligned<bool>(ref Unsafe.As<ulong, byte>(ref vr2.F0));
        System.Console.WriteLine(vr3);
    }
}
Author: jakobbotsch
Assignees: -
Labels:

area-CodeGen-coreclr, untriaged

Milestone: -

@EgorBo
Copy link
Member

EgorBo commented Aug 18, 2023

*************** Starting PHASE Morph - Structs/AddrExp
LocalAddressVisitor visiting statement:
STMT00000 ( 0x000[E-] ... 0x003 )
               [000002] DA---------                         *  STORE_LCL_VAR struct<S1, 8>(P) V00 loc0         
                                                            *    long   field V00.F0 (fldOffset=0x0) -> V02 tmp1         
               [000001] -----------                         \--*  CNS_INT   int    0

LocalAddressVisitor visiting statement:
STMT00001 ( 0x008[E-] ... 0x01E )
               [000006] --C-G------                         *  CALL      void   System.Console:WriteLine(bool)
               [000005] U---G------ arg0                    \--*  IND       bool  
               [000004] -----------                            \--*  FIELD_ADDR byref  S1:F0
               [000003] -----------                               \--*  LCL_ADDR  byref  V00 loc0         [+0]
                                                                     *    long   field V00.F0 (fldOffset=0x0) -> V02 tmp1         
Replacing the field in promoted struct with local var V02
LocalAddressVisitor modified statement:
STMT00001 ( 0x008[E-] ... 0x01E )
               [000006] --C-G------                         *  CALL      void   System.Console:WriteLine(bool)
               [000008] ----------- arg0                    \--*  CAST      int <- bool <- long
               [000004] -----------                            \--*  LCL_VAR   long   V02 tmp1         

@EgorBo
Copy link
Member

EgorBo commented Aug 18, 2023

Looks like

            case IndirTransform::NarrowCast:
                assert(varTypeIsIntegral(indir));
                assert(varTypeIsIntegral(varDsc));
                assert(genTypeSize(varDsc) >= genTypeSize(indir));
                assert(!isDef);

                lclNode = indir->gtGetOp1()->BashToLclVar(m_compiler, lclNum);
                *use    = m_compiler->gtNewCastNode(genActualType(indir), lclNode, false, indir->TypeGet());
                break;

is the culprit since it woud have to do two casts due to small type?

@jakobbotsch
Copy link
Member Author

@EgorBo It works if you change ReadUnaligned<bool>(ref Unsafe.As<long, byte> to Unsafe.As<long, bool>. I think the problem is how ReadUnaligned and co. were intrinsified, probably TYP_BOOL needs to be changed to TYP_BYTE when we create the loads.

@jakobbotsch
Copy link
Member Author

Hmm, we do create TYP_BOOL indirections in other places, so perhaps the folding in gtFoldExprConst should just be taught about it.

@EgorBo
Copy link
Member

EgorBo commented Aug 18, 2023

so CAST int <- bool <- long is a legal IR node?

Ah, I see it is.

@jakobbotsch
Copy link
Member Author

I don't think we should be creating TYP_BOOL indirections for Read/ReadUnaligned<bool>. If stored to a bool it will cause us to retain lvIsBoolean on the destination's LclVarDsc which seems to imply that we can assume the value is 0 or 1 which is certainly not true for Read/ReadUnaligned. optimizebools.cpp looks to have a few transformations that are illegal for non-1 trues that kick in when this field is true.

@SingleAccretion
Copy link
Contributor

SingleAccretion commented Aug 18, 2023

which seems to imply that we can assume the value is 0 or 1 which is certainly not true for Read/ReadUnaligned

Note that this is not true for TYP_BOOL in general. TYP_BOOL is a lie and the fact optimizebools relies on it is a correctness hole (in the presence of non-canonical booleans).

@MichalPetryka
Copy link
Contributor

I don't think we should be creating TYP_BOOL indirections for Read/ReadUnaligned<bool>. If stored to a bool it will cause us to retain lvIsBoolean on the destination's LclVarDsc which seems to imply that we can assume the value is 0 or 1 which is certainly not true for Read/ReadUnaligned. optimizebools.cpp looks to have a few transformations that are illegal for non-1 trues that kick in when this field is true.

Should we introduce a test for this? Something like:

byte byte2 = 2;
Assert(1 == (Unsafe.ReadUnaligned<byte>(ref byte2) ? 1 : 0));

@JulieLeeMSFT JulieLeeMSFT added this to the 9.0.0 milestone Aug 21, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Aug 21, 2023
jakobbotsch added a commit that referenced this issue Aug 28, 2023
TYP_BOOL servces no purpose but to confuse today. We try to use it in some cases
to make assumptions about the value being 0/1, but this logic is not correct in
the face of unnormalized booleans. This change remove TYP_BOOL replacing its
uses by TYP_UBYTE. This fixes an issue where we forgot to handle TYP_BOOL.

Fix #90798
@ghost ghost locked as resolved and limited conversation to collaborators Sep 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants