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

[Fuzzlyn] Assertion failed 'tgtIG' during 'Generate code' #96224

Closed
amanasifkhalid opened this issue Dec 20, 2023 · 11 comments · Fixed by #97201
Closed

[Fuzzlyn] Assertion failed 'tgtIG' during 'Generate code' #96224

amanasifkhalid opened this issue Dec 20, 2023 · 11 comments · Fixed by #97201
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI os-mac-os-x macOS aka OSX
Milestone

Comments

@amanasifkhalid
Copy link
Member

amanasifkhalid commented Dec 20, 2023

Last Fuzzlyn run failed on OS X ARM64 with the following assert:

Reduced example:

// Generated by Fuzzlyn v1.6 on 2023-12-24 17:50:10
// Run on Arm64 MacOS
// Seed: 17421103114350285504
// Reduced from 177.9 KiB to 1.7 KiB in 00:01:10
// Hits JIT assert in Release:
// Assertion failed 'tgtIG' in 'Program:Main(Fuzzlyn.ExecutionServer.IRuntime)' during 'Generate code' (IL size 96; hash 0xade6b36b; FullOpts)
// 
//     File: /Users/runner/work/1/s/src/coreclr/jit/emit.cpp Line: 5174
// 
using System.Runtime.CompilerServices;

public class Program
{
    public static IRuntime s_rt;
    public static byte[] s_4;
    public static long s_25;
    public static bool s_29;
    public static ushort[] s_46;
    public static long s_59;
    public static void Main()
    {
        CollectibleALC alc = new CollectibleALC();
        System.Reflection.Assembly asm = alc.LoadFromAssemblyPath(System.Reflection.Assembly.GetExecutingAssembly().Location);
        System.Reflection.MethodInfo mi = asm.GetType(typeof(Program).FullName).GetMethod(nameof(MainInner));
        System.Type runtimeTy = asm.GetType(typeof(Runtime).FullName);
        mi.Invoke(null, new object[]{System.Activator.CreateInstance(runtimeTy)});
    }
    
    public static void MainInner(IRuntime rt)
    {
        long[] vr5 = new long[]{0};
        for (int vr6 = 0; vr6 < 2; vr6++)
        {
            var vr7 = new int[]{1};
            vr5[0] = (M50(vr7) & 0) / (int)M58();
            s_rt.WriteLine("c_339", s_46[0]);
            s_rt.WriteLine("c_340", 5566640345371016742UL);
        }
    }

    public static int M50(int[] arg0)
    {
        if (s_29)
        {
            s_59 = s_25;
        }

        return arg0[0];
    }

    public static ulong M58()
    {
        s_4 = new byte[]{0};
        return 5566640345371016742UL;
    }
}

public interface IRuntime
{
    void WriteLine<T>(string site, T value);
}

public class Runtime : IRuntime
{
    public void WriteLine<T>(string site, T value) => System.Console.WriteLine(value);
}

public class CollectibleALC : System.Runtime.Loader.AssemblyLoadContext
{
    public CollectibleALC(): base(true)
    {
    }
}

cc @dotnet/jit-contrib, does anyone have an ARM64 Mac they can repro this failure on?

@amanasifkhalid amanasifkhalid added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI blocking-clean-ci-optional Blocking optional rolling runs labels Dec 20, 2023
@amanasifkhalid amanasifkhalid added this to the 9.0.0 milestone Dec 20, 2023
@ghost
Copy link

ghost commented Dec 20, 2023

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

Issue Details

Last Fuzzlyn run failed on OS X ARM64 with the following assert:

Assert failure(PID 2770 [0x00000ad2], Thread: 3119207 [0x2f9867]): SanityCheck()
    File: /Users/runner/work/1/s/src/coreclr/vm/methodtable.cpp Line: 9435
    Image: /private/tmp/helix/working/BBC30A1C/p/CoreRoot/corerun

Reduced example:

using System.Runtime.CompilerServices;

public interface I0
{
}

public interface I1
{
}

public class C0
{
    public long F0;
    public long F2;
    public bool F3;
    public ulong F4;
    public bool F5;
    public C0(bool f3, bool f5)
    {
        F3 = f3;
    }

    public int M61()
    {
        return 10;
    }
}

public class C1 : I0
{
    public C0 F0;
    public C1(C0 f0)
    {
    }

    public sbyte M3(C0 arg0)
    {
        return default(sbyte);
    }

    public bool M68()
    {
        return true;
    }
}

public struct S0 : I1
{
    public ushort F0;
    public byte F1;
    public S0(ushort f0, byte f1)
    {
        F0 = f0;
        F1 = f1;
    }
}

public struct S1
{
    public long F0;
    public ushort F1;
    public bool F2;
    public byte F3;
    public int F4;
    public bool F5;
    public byte F6;
    public bool F7;
    public sbyte F8;
    public uint F9;
    public S1(long f0, ushort f1, bool f2, byte f3, int f4, bool f5, byte f6, bool f7, sbyte f8, uint f9): this()
    {
        F0 = f0;
        F1 = f1;
        F2 = f2;
        F3 = f3;
        F4 = f4;
        F5 = f5;
        F6 = f6;
        F7 = f7;
        F8 = f8;
        F9 = f9;
    }

    public void M43()
    {
        return;
    }

    public void M59()
    {
        return;
    }
}

public class Program
{
    public static IRuntime s_rt;
    public static I1[] s_1 = new I1[]{new S0(1, 209)};
    public static I0 s_12;
    public static S0[, ] s_17;
    public static S1 s_18;
    public static C1[][] s_19;
    public static ulong s_21;
    public static C0[] s_23;
    public static byte[, ] s_30 = new byte[, ]{{239, 192, 1, 1, 2, 1}, {149, 155, 1, 188, 177, 1}, {10, 1, 109, 241, 1, 99}, {11, 222, 0, 15, 1, 0}};
    public static C0 s_33 = new C0(false, false);
    public static byte s_39;
    public static int[] s_46;
    public static C1 s_49;
    public static byte[] s_50 = new byte[]{82};
    public static I0[] s_52 = new I0[]{new C1(new C0(false, true))};
    public static C1[, ][] s_55;
    public static int s_56;
    public static S0 s_58;
    public static short s_79 = 0;
    public static short s_81;
    public static short[][][, ][][] s_82;
    public static ushort s_86;
    public static void Main()
    {
        CollectibleALC alc = new CollectibleALC();
        System.Reflection.Assembly asm = alc.LoadFromAssemblyPath(System.Reflection.Assembly.GetExecutingAssembly().Location);
        System.Reflection.MethodInfo mi = asm.GetType(typeof(Program).FullName).GetMethod(nameof(MainInner));
        System.Type runtimeTy = asm.GetType(typeof(Runtime).FullName);
        mi.Invoke(null, new object[]{System.Activator.CreateInstance(runtimeTy)});
    }

    public static void MainInner(IRuntime rt)
    {
        s_rt = rt;
        M0();
    }

    public static void M0()
    {
        M49(-114);
    }

    public static int M1()
    {
        return default(int);
    }

    public static void M17(int arg1, sbyte arg2, sbyte arg4, sbyte[][] arg5, ref ulong arg6)
    {
        return;
    }

    public static void M38()
    {
        return;
    }

    public static void M49(sbyte arg0)
    {
        byte[] var0 = s_50;
        for (int var1 = 0; var1 < 2; var1++)
        {
            I1[][][] var2 = new I1[][][]{new I1[][]{new I1[]{new S0(20692, 2)}, new I1[]{new S0(65534, 1)}, new I1[]{new S0(20900, 155)}, new I1[]{new S0(13884, 116)}}};
            short var3 = (short)(((M51() + 1) & 0) / (short)(short)M51());
            s_rt.WriteLine("c_815", var1);
        }

        s_rt.WriteLine("c_817", var0[0]);
        return;
    }

    public static uint M51()
    {
        long var5;
        ref C0 var0 = ref s_23[0];
        var0 = ref var0;
        return 4108078157U;
    }

    public static void M56(I0[] arg0, S1 arg2, S1 arg3, int[][] arg4, long arg6, C1 arg7, byte arg8)
    {
    }

    public static void M63(ref uint arg1)
    {
        return;
    }

    public static uint M79()
    {
        return default(uint);
    }

    public static void M86(ref sbyte arg0, ref ushort arg1)
    {
        return;
    }

    public static void M89(ref C0 arg0)
    {
        return;
    }

    public static void M15(C0 argThis, C0 arg0, long[] arg1)
    {
        return;
    }

    public static void M40(C1 argThis)
    {
        return;
    }

    public static void M62(S0 argThis, S0 arg0, ref ushort arg1, C1 arg3, ulong[] arg4, ref C0 arg5, ulong[][][] arg6, I0 arg7)
    {
        s_rt.WriteLine("c_998", argThis.F0);
        return;
    }

    public static void M39(C1 argThis, I1 arg1)
    {
        return;
    }

    public static void M36(S0 argThis, C0 arg0, uint arg1, bool arg2, long arg3)
    {
        return;
    }

    public static short M26(S0 argThis, ulong arg1, ref bool[] arg2)
    {
        return 20181;
    }
}

public interface IRuntime
{
    void WriteLine<T>(string site, T value);
}

public class Runtime : IRuntime
{
    public void WriteLine<T>(string site, T value) => System.Console.WriteLine(value);
}

public class CollectibleALC : System.Runtime.Loader.AssemblyLoadContext
{
    public CollectibleALC(): base(true)
    {
    }
}

cc @dotnet/jit-contrib, does anyone have an ARM64 Mac they can repro this failure on?

Author: amanasifkhalid
Assignees: -
Labels:

area-CodeGen-coreclr, blocking-clean-ci-optional

Milestone: 9.0.0

@BruceForstall
Copy link
Member

This looks like a possible GC info issue: running with DOTNET_GCStress=0xC might make it reproduce more reliably.

@jakobbotsch
Copy link
Member

The GC issue reproduces intermittently. I have never been able to reproduce it reliably.

The reduced example here looks unrelated to the GC issue (different seed). Not sure why it does not show anything interesting in the comment header (please include it, the seed can be quite important)

@jakobbotsch
Copy link
Member

jakobbotsch commented Dec 20, 2023

From the logs, the original program that resulted in the reduced example above hit the following assert:

Found example with seed 16090003956643131594 that hits error
JIT assert failed:
Assertion failed 'tgtIG' in 'Program:M49(byte):uint' during 'Generate code' (IL size 1628; hash 0x56b90d1c; FullOpts)

File: /Users/runner/work/1/s/src/coreclr/jit/emit.cpp Line: 5174

Reducing 16090003956643131594
Running: /tmp/helix/working/BBC30A1C/p/exploratory/Fuzzlyn --host /tmp/helix/working/BBC30A1C/p/CoreRoot/corerun --reduce --seed 16090003956643131594 --output /tmp/helix/working/BBC30A1C/t/tmpt6tohw_b/16090003956643131594.cs

Based on the comment header the reduced example might or might not hit the same assert (again, not totally sure what happened there, I will add it to my todo list to investigate this from the Fuzzlyn side once I'm back)

@amanasifkhalid
Copy link
Member Author

The tgtIG assert was hit with by a few smaller example programs in the latest run. Here's one such example:

// Generated by Fuzzlyn v1.6 on 2023-12-24 17:50:10
// Run on Arm64 MacOS
// Seed: 17421103114350285504
// Reduced from 177.9 KiB to 1.7 KiB in 00:01:10
// Hits JIT assert in Release:
// Assertion failed 'tgtIG' in 'Program:Main(Fuzzlyn.ExecutionServer.IRuntime)' during 'Generate code' (IL size 96; hash 0xade6b36b; FullOpts)
// 
//     File: /Users/runner/work/1/s/src/coreclr/jit/emit.cpp Line: 5174
// 
using System.Runtime.CompilerServices;

public class Program
{
    public static IRuntime s_rt;
    public static byte[] s_4;
    public static long s_25;
    public static bool s_29;
    public static ushort[] s_46;
    public static long s_59;
    public static void Main()
    {
        CollectibleALC alc = new CollectibleALC();
        System.Reflection.Assembly asm = alc.LoadFromAssemblyPath(System.Reflection.Assembly.GetExecutingAssembly().Location);
        System.Reflection.MethodInfo mi = asm.GetType(typeof(Program).FullName).GetMethod(nameof(MainInner));
        System.Type runtimeTy = asm.GetType(typeof(Runtime).FullName);
        mi.Invoke(null, new object[]{System.Activator.CreateInstance(runtimeTy)});
    }
    
    public static void MainInner(IRuntime rt)
    {
        long[] vr5 = new long[]{0};
        for (int vr6 = 0; vr6 < 2; vr6++)
        {
            var vr7 = new int[]{1};
            vr5[0] = (M50(vr7) & 0) / (int)M58();
            s_rt.WriteLine("c_339", s_46[0]);
            s_rt.WriteLine("c_340", 5566640345371016742UL);
        }
    }

    public static int M50(int[] arg0)
    {
        if (s_29)
        {
            s_59 = s_25;
        }

        return arg0[0];
    }

    public static ulong M58()
    {
        s_4 = new byte[]{0};
        return 5566640345371016742UL;
    }
}

public interface IRuntime
{
    void WriteLine<T>(string site, T value);
}

public class Runtime : IRuntime
{
    public void WriteLine<T>(string site, T value) => System.Console.WriteLine(value);
}

public class CollectibleALC : System.Runtime.Loader.AssemblyLoadContext
{
    public CollectibleALC(): base(true)
    {
    }
}

@jakobbotsch
Copy link
Member

Let's repurpose this issue to track the tgtIG assert. I do not think there is anything actionable here for the GC issue, unfortunately.

@jakobbotsch jakobbotsch changed the title [Fuzzlyn] Assert failure "SanityCheck()" on OS X ARM64 [Fuzzlyn] Assertion failed 'tgtIG' during 'Generate code' Jan 3, 2024
@JulieLeeMSFT JulieLeeMSFT added os-mac-os-x macOS aka OSX and removed blocking-clean-ci-optional Blocking optional rolling runs labels Jan 8, 2024
@BruceForstall
Copy link
Member

@jakobbotsch I'm not able to reproduce this assert locally. Is there anything special I need to do?

Here's what I've done:

  1. dotnet new console
  2. Replace Program.cs contents with one of the test cases from https://dev.azure.com/dnceng-public/public/_build/results?buildId=526788&view=ms.vss-build-web.run-extensions-tab
  3. dotnet build -c Release
  4. build Checked and Release runtime (osx-arm64).
  5. Create Core_Roots
  6. copy libclrjit.dylib from Checked to Release
  7. corerun the test, e.g., ~/gh/runtime2/artifacts/tests/coreclr/osx.arm64.Release/Tests/Core_Root/corerun /Users/bruce/bugs/96224/bin/Release/net8.0/96224.dll

I've tried with and without DOTNET_TieredCompilation=0. I've tried with a pure Checked Core_Root.

I notice the assertion is:

Assertion failed 'tgtIG' in 'Program:Main(Fuzzlyn.ExecutionServer.IRuntime)'

but the test fragment doesn't have a Main that takes a Fuzzlyn.ExecutionServer.IRuntime and in fact I don't see any Fuzzlyn class/namespace. So is there some magic here we're not seeing?

@jakobbotsch
Copy link
Member

jakobbotsch commented Jan 18, 2024

@jakobbotsch I'm not able to reproduce this assert locally. Is there anything special I need to do?

Here's what I've done:

  1. dotnet new console
  2. Replace Program.cs contents with one of the test cases from https://dev.azure.com/dnceng-public/public/_build/results?buildId=526788&view=ms.vss-build-web.run-extensions-tab
  3. dotnet build -c Release
  4. build Checked and Release runtime (osx-arm64).
  5. Create Core_Roots
  6. copy libclrjit.dylib from Checked to Release
  7. corerun the test, e.g., ~/gh/runtime2/artifacts/tests/coreclr/osx.arm64.Release/Tests/Core_Root/corerun /Users/bruce/bugs/96224/bin/Release/net8.0/96224.dll

I've tried with and without DOTNET_TieredCompilation=0. I've tried with a pure Checked Core_Root.

Your steps should replicate exactly what Fuzzlyn is doing (with the checked Core_Root). So it should be able to repro it.

I notice the assertion is:

Assertion failed 'tgtIG' in 'Program:Main(Fuzzlyn.ExecutionServer.IRuntime)'

but the test fragment doesn't have a Main that takes a Fuzzlyn.ExecutionServer.IRuntime and in fact I don't see any Fuzzlyn class/namespace. So is there some magic here we're not seeing?

At the end of the reduction process Fuzzlyn will try to produce a standalone version of the program. Before that, the program that reproduces the issue has dependencies on some interfaces shared with Fuzzlyn (e.g. this IRuntime thing). Fuzzlyn then tries to validate that the standalone program reproduces the error, but it does not update the assertion message, which is why you see that difference.

The bug here is that Fuzzlyn does not check if the very first version of a standalone program reproduces the error. It is unusual for it not to, because it should replicate exactly what the non-standalone program is doing, except with the ALC setup being done in a different place.
I fixed the issue in jakobbotsch/Fuzzlyn@4413822. It means Fuzzlyn will produce reduced programs that do not compile out of the box in these cases. For example, reducing the seeds in this issue now produces

// Generated by Fuzzlyn v1.6 on 2024-01-18 13:26:13
// Run on Arm64 Linux
// Seed: 17421103114350285504
// Reduced from 177.9 KiB to 0.9 KiB in 00:05:33
// Hits JIT assert in Release:
// Assertion failed 'tgtIG' in 'Program:Main(Fuzzlyn.ExecutionServer.IRuntime)' during 'Generate code' (IL size 98; hash 0xade6b36b; FullOpts)
//
//     File: /runtime/src/coreclr/jit/emit.cpp Line: 5174
//
using System.Runtime.CompilerServices;

public class Program
{
    public static Fuzzlyn.ExecutionServer.IRuntime s_rt;
    public static byte[] s_4;
    public static long s_25;
    public static bool s_29;
    public static long s_59;
    public static void Main(Fuzzlyn.ExecutionServer.IRuntime rt)
    {
        long[] vr5 = new long[]{0};
        for (int vr6 = 0; vr6 < 2; vr6++)
        {
            var vr7 = new int[]{1};
            vr5[0] = (M50(vr7) & 0) / (int)M58();
            ulong vr8 = 5566640345371016742UL;
            long vr9 = vr5[0];
            s_rt.Checksum("c_340", vr8);
            s_rt.Checksum("c_341", vr9);
        }
    }

    public static int M50(int[] arg0)
    {
        if (s_29)
        {
            s_59 = s_25;
        }

        return arg0[0];
    }

    public static ulong M58()
    {
        s_4 = new byte[]{0};
        return 5566640345371016742UL;
    }

which is the exact source of the program that Fuzzlyn hit the assert on, but of course won't compile standalone as the interface is in a separate assembly. But at least it won't be misleading anymore.

I'm working on adding the SPMI support you suggested. I've attached an SPMI collection that reproduces the issue (collected on linux-arm64, commit 77d643d)
repro-24575.zip

@BruceForstall
Copy link
Member

One thing I noticed with the original repro is it compiled fine (I couldn't repro the assert), but it failed to run. Maybe that's standard behavior for a Fuzzlyn repro? It was because public static IRuntime s_rt; was never initialized. Making that public static IRuntime s_rt = new Runtime(); fixed it. Is that expected?

@jakobbotsch
Copy link
Member

One thing I noticed with the original repro is it compiled fine (I couldn't repro the assert), but it failed to run. Maybe that's standard behavior for a Fuzzlyn repro? It was because public static IRuntime s_rt; was never initialized. Making that public static IRuntime s_rt = new Runtime(); fixed it. Is that expected?

Yeah, it's expected. Since the interesting behavior here is an assertion, no code was actually running, so Fuzzlyn was able to remove all of the initialization as part of simplifying the program.

@BruceForstall
Copy link
Member

I can repro the test failure with the linked repro-24575.zip MC file.

The failure is because of the following:

  1. In the phase to create throw helper blocks we create 3 blocks; 2 are for DIV (DIV_BY_ZERO, OVERFLOW).
  2. During PHASE Calculate stack level slots, we decide 2 of the throw helper blocks are unreachable, including the OVERFLOW block, due to the dividend being a constant zero such that OVERFLOW is no longer possible:
N003 (  1,  2) [000210] -----------                  t210 =    CNS_INT   int    0 $c5
N005 ( 14,  2) [000028] --CXG------                   t28 =    CALL      long   Program:M58():ulong $183
                                                            /--*  t28    long
N006 ( 15,  4) [000032] --CXG------                   t32 = *  CAST      int <- long $301
                                                            /--*  t210   int
                                                            +--*  t32    int
N007 ( 51, 13) [000033] --CXG------                   t33 = *  DIV       int    $303
  1. Later, LSRA inserts a RELOAD node for the constant zero:
N089 (  1,  2) [000210] ----------Z                  t210 =    CNS_INT   int    0 REG x0 $c5
                                                            /--*  t210   int
               [000243] -----------                  t243 = *  RELOAD    int    REG x1
N091 ( 14,  2) [000028] --CXG------                   t28 =    CALL      long   Program:M58():ulong REG x0 $183
                                                            /--*  t28    long
N093 ( 15,  4) [000032] --CXG------                   t32 = *  CAST      int <- long REG x0 $301
                                                            /--*  t243   int
                                                            +--*  t32    int
N095 ( 51, 13) [000033] --CXG------                   t33 = *  DIV       int    REG x0 $303
  1. After that, GenTree::CanDivOrModPossiblyOverflow() no longer sees the constant zero dividend, and says that overflow is possible.
  2. Thus, during codegen, it looks for the throw helper block that has already been removed.
  3. In the current code, it finds a block that is no longer in the block list despite acdUsed == false.
  4. Since this block isn't in the block list, it was never given a codegen label, so tgtIG is nullptr when branch codegen is done.

One fix is to pay more attention to acdUsed (checking it, asserting on it). The other is to teach GenTree::CanDivOrModPossiblyOverflow() about RELOAD, or otherwise not change the result of GenTree::OperExceptions() between Calculate stack level slots (StackLevelSetter phase) and codegen (basically, through LSRA).

BruceForstall added a commit to BruceForstall/runtime that referenced this issue Jan 19, 2024
Throw helper blocks are created in morph, then possibly removed if
unnecessary in StackLevelSetter (under optimization). However, there
was a case where StackLevelSetter removed an OVERFLOW throw helper
block after optimization proved it unnecessary because of a constant
zero dividend, but between StackLevelSetter and codegen, LSRA introduced
a RELOAD node above the constant zero that `GenTree::CanDivOrModPossiblyOverflow()`
didn't understand, thus causing it to think that overflow was possible.
Codegen looked for the OVERFLOW throw helper block and couldn't find it.

There are multiple fixes here, somewhat "defense in depth":
- If `StackLevelSetter::SetThrowHelperBlocks()` determines a node can't throw
divide-by-zero or ArithmeticException (overflow), it marks the node
GTF_DIV_MOD_NO_BY_ZERO / GTF_DIV_MOD_NO_OVERFLOW, respectively. This is what
morph does earlier in compilation.
- `genMarkLabelsForCodegen()` does not mark throw helper blocks where `acdUsed`
is false, to avoid marking deleted blocks.
- More asserts are added that `acdUsed` is true when codegen goes to generate
a branch to a throw helper.
- `GenTree::OperExceptions` / `CanDivOrModPossiblyOverflow` are changed to skip
COPY/RELOAD nodes.

Fixes dotnet#96224
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jan 19, 2024
BruceForstall added a commit that referenced this issue Jan 19, 2024
Throw helper blocks are created in morph, then possibly removed if
unnecessary in StackLevelSetter (under optimization). However, there
was a case where StackLevelSetter removed an OVERFLOW throw helper
block after optimization proved it unnecessary because of a constant
zero dividend, but between StackLevelSetter and codegen, LSRA introduced
a RELOAD node above the constant zero that `GenTree::CanDivOrModPossiblyOverflow()`
didn't understand, thus causing it to think that overflow was possible.
Codegen looked for the OVERFLOW throw helper block and couldn't find it.

There are multiple fixes here, somewhat "defense in depth":
- If `StackLevelSetter::SetThrowHelperBlocks()` determines a node can't throw
divide-by-zero or ArithmeticException (overflow), it marks the node
GTF_DIV_MOD_NO_BY_ZERO / GTF_DIV_MOD_NO_OVERFLOW, respectively. This is what
morph does earlier in compilation.
- `genMarkLabelsForCodegen()` does not mark throw helper blocks where `acdUsed`
is false, to avoid marking deleted blocks.
- More asserts are added that `acdUsed` is true when codegen goes to generate
a branch to a throw helper.
- `GenTree::OperExceptions` / `CanDivOrModPossiblyOverflow` are changed to skip
COPY/RELOAD nodes.

Fixes #96224
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jan 19, 2024
tmds pushed a commit to tmds/runtime that referenced this issue Jan 23, 2024
Throw helper blocks are created in morph, then possibly removed if
unnecessary in StackLevelSetter (under optimization). However, there
was a case where StackLevelSetter removed an OVERFLOW throw helper
block after optimization proved it unnecessary because of a constant
zero dividend, but between StackLevelSetter and codegen, LSRA introduced
a RELOAD node above the constant zero that `GenTree::CanDivOrModPossiblyOverflow()`
didn't understand, thus causing it to think that overflow was possible.
Codegen looked for the OVERFLOW throw helper block and couldn't find it.

There are multiple fixes here, somewhat "defense in depth":
- If `StackLevelSetter::SetThrowHelperBlocks()` determines a node can't throw
divide-by-zero or ArithmeticException (overflow), it marks the node
GTF_DIV_MOD_NO_BY_ZERO / GTF_DIV_MOD_NO_OVERFLOW, respectively. This is what
morph does earlier in compilation.
- `genMarkLabelsForCodegen()` does not mark throw helper blocks where `acdUsed`
is false, to avoid marking deleted blocks.
- More asserts are added that `acdUsed` is true when codegen goes to generate
a branch to a throw helper.
- `GenTree::OperExceptions` / `CanDivOrModPossiblyOverflow` are changed to skip
COPY/RELOAD nodes.

Fixes dotnet#96224
@github-actions github-actions bot locked and limited conversation to collaborators Feb 19, 2024
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 os-mac-os-x macOS aka OSX
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants