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

ARM32: runtime crash inside JIT #57061

Closed
jakobbotsch opened this issue Aug 9, 2021 · 5 comments · Fixed by #57199
Closed

ARM32: runtime crash inside JIT #57061

jakobbotsch opened this issue Aug 9, 2021 · 5 comments · Fixed by #57199
Assignees
Labels
arch-arm32 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.2 on 2021-08-07 03:24:16
// Run on .NET 6.0.0-dev on Arm Linux
// Seed: 12756399625466979010
// Reduced from 798.7 KiB to 1.5 KiB in 03:42:20
// Crashes the runtime
struct S0
{
    public bool F0;
    public bool F1;
    public uint F2;
    public short F3;
    public ulong F4;
    public S0(bool f0, bool f1, uint f2, short f3, ulong f4): this()
    {
        F0 = f0;
        F1 = f1;
        F2 = f2;
        F3 = f3;
        F4 = f4;
    }
}

class C0
{
    public C0(S0 f7, S0 f8)
    {
    }
}

class C1
{
    public S0 F1;
    public ulong F5;
}

struct S2
{
    public S2(C0 f4): this()
    {
    }
}

struct S3
{
    public uint F0;
}

class C2
{
    public C1 F3;
}

public class Program
{
    static C2 s_23;
    static C1 s_37;
    static sbyte s_56;
    static S3 s_60;
    public static void Main()
    {
        uint vr2 = default(uint);
        uint vr3;
        bool vr4 = true;
        if (!vr4)
        {
            try
            {
                System.Console.WriteLine(s_60.F0);
            }
            finally
            {
                var vr5 = new C0(new S0(false, true, 0, 0, 0), new S0(false, false, 0, 0, 0));
            }

            s_37.F5 = s_23.F3.F1.F4++;
        }

        vr4 = vr4;
        for (int vr6 = 0; vr6 < 0; vr6++)
        {
            sbyte vr8 = s_56;
            try
            {
                var vr7 = new S2(new C0(new S0(true, false, 0, 0, 0), new S0(true, true, 0, 0, 0)));
            }
            finally
            {
                vr3 = vr2;
            }

            vr3 = vr3;
        }
    }
}
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Aug 9, 2021
@jakobbotsch jakobbotsch added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI and removed untriaged New issue has not been triaged by the area owner labels Aug 9, 2021
@jakobbotsch jakobbotsch self-assigned this Aug 9, 2021
@jakobbotsch jakobbotsch added this to the 6.0.0 milestone Aug 9, 2021
@jakobbotsch
Copy link
Member Author

I'm looking at this.

@jakobbotsch
Copy link
Member Author

The crash happens because the block that contains

s_37.F5 = s_23.F3.F1.F4++;

does not have SSA form built for it; it is unreachable from the entry and thus not part of the dominator tree. Later, we crash here when trying to query SSA information for it during value numbering:

if (lvaInSsa(argLcl->GetLclNum()))
{
ValueNum argVN =
lvaTable[argLcl->GetLclNum()].GetPerSsaData(argLcl->GetSsaNum())->m_vnPair.GetLiberal();

That's kind of strange, since that code runs only for IR nodes inside loops, and the snippet above is not inside a loop. However, in ARM32 the flow-graph is a little different due to different exception handling blocks being added compared to other platforms and then fgReorderBlocks ends up reordering blocks so that the loop spans the unreachable block. I believe we would see similar crashes for other platforms if we somehow could produce a natural loop that contained the snippet above (I tried but was unable to due to various heuristics/checks blocking it).

Not totally sure how to fix this. It's weird that we have this unreachable code sticking around, but that seems to be a consequence of the exception handlers inside the block and also seems to happen on other platforms (at least x86). Perhaps we should just be checking if the local has SSA form built for it in the value numbering code? Though it seems unfortunate to have to handle this case downstream.

cc @dotnet/jit-contrib

@jakobbotsch
Copy link
Member Author

In x64 we do seem to be able to remove that unreachable block entirely, so not completely sure why it is left over in x86/ARM32.

@jakobbotsch
Copy link
Member Author

That seems to be a bug in fgRemoveUnreachableBlocks. When we "remove" a block that has BBF_DONT_REMOVE set, we set it as BBJ_THROW instead of removing it from the flow-graph. However, fgRemoveUnreachableBlocks only returns true if unreachable blocks are actually removed. In x86 and ARM32 there is only one unreachable block in the first iteration, and it is marked BBF_DONT_REMOVE, so we don't try to see if there are more unreachable blocks after the first iteration. In x64 we import more blocks and the first iteration removes one of these blocks (in addition to marking another as BBJ_THROW), so the loop runs again.

jakobbotsch added a commit to jakobbotsch/runtime that referenced this issue Aug 11, 2021
When 'removing' a BBF_DONT_REMOVE block we change it to BBJ_THROW. After
this it is possible that other blocks become unreachable, so we should
keep looking for such blocks.

In dotnet#57061 that manifested in a case where the unreachable block did not
have SSA built for it, but downstream the compiler was relying on SSA
being built.

Fix dotnet#57061
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 11, 2021
jakobbotsch added a commit that referenced this issue Aug 12, 2021
When 'removing' a BBF_DONT_REMOVE block we change it to BBJ_THROW. After
this it is possible that other blocks become unreachable, so we should
keep looking for such blocks.

In #57061 that manifested in a case where the unreachable block did not
have SSA built for it, but downstream the compiler was relying on SSA
being built.

Fix #57061
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 12, 2021
@dotnet dotnet locked as resolved and limited conversation to collaborators Sep 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-arm32 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.

1 participant