Skip to content

Commit

Permalink
[release/9.0-staging] JIT: Include more edges in `BlockDominancePreds…
Browse files Browse the repository at this point in the history
…` to avoid a JIT crash (#110568)

Because of spurious flow it is possible that the preds of the try-begin
block are not the only blocks that can dominate a handler. We handled
this possibility, but only for finally/fault blocks that can directly
have these edges. However, even other handler blocks can be reachable
through spurious paths that involves finally/fault blocks, and in these
cases returning the preds of the try-begin block is not enough to
compute the right dominator statically.
  • Loading branch information
github-actions[bot] authored Dec 20, 2024
1 parent 7eb5ef2 commit 4d2ecdc
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 35 deletions.
50 changes: 15 additions & 35 deletions src/coreclr/jit/block.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -267,15 +267,21 @@ FlowEdge* Compiler::BlockPredsWithEH(BasicBlock* blk)
// 'blk'.
//
// Arguments:
// blk - Block to get dominance predecessors for.
// blk - Block to get dominance predecessors for.
//
// Returns:
// List of edges.
// List of edges.
//
// Remarks:
// Differs from BlockPredsWithEH only in the treatment of handler blocks;
// enclosed blocks are never dominance preds, while all predecessors of
// blocks in the 'try' are (currently only the first try block expected).
// Differs from BlockPredsWithEH only in the treatment of handler blocks;
// enclosed blocks are never dominance preds, while all predecessors of
// blocks in the 'try' are (currently only the first try block expected).
//
// There are additional complications due to spurious flow because of
// two-pass EH. In the flow graph with EH edges we can see entries into the
// try from filters outside the try, to blocks other than the "try-begin"
// block. Hence we need to consider the full set of blocks in the try region
// when considering the block dominance preds.
//
FlowEdge* Compiler::BlockDominancePreds(BasicBlock* blk)
{
Expand All @@ -284,44 +290,18 @@ FlowEdge* Compiler::BlockDominancePreds(BasicBlock* blk)
return blk->bbPreds;
}

EHblkDsc* ehblk = ehGetBlockHndDsc(blk);
if (!ehblk->HasFinallyOrFaultHandler() || (ehblk->ebdHndBeg != blk))
{
return ehblk->ebdTryBeg->bbPreds;
}

// Finally/fault handlers can be preceded by enclosing filters due to 2
// pass EH, so add those and keep them cached.
BlockToFlowEdgeMap* domPreds = GetDominancePreds();
FlowEdge* res;
if (domPreds->Lookup(blk, &res))
{
return res;
}

res = ehblk->ebdTryBeg->bbPreds;
if (ehblk->HasFinallyOrFaultHandler() && (ehblk->ebdHndBeg == blk))
EHblkDsc* ehblk = ehGetBlockHndDsc(blk);
res = BlockPredsWithEH(blk);
for (BasicBlock* predBlk : ehblk->ebdTryBeg->PredBlocks())
{
// block is a finally or fault handler; all enclosing filters are predecessors
unsigned enclosing = ehblk->ebdEnclosingTryIndex;
while (enclosing != EHblkDsc::NO_ENCLOSING_INDEX)
{
EHblkDsc* enclosingDsc = ehGetDsc(enclosing);
if (enclosingDsc->HasFilter())
{
for (BasicBlock* filterBlk = enclosingDsc->ebdFilter; filterBlk != enclosingDsc->ebdHndBeg;
filterBlk = filterBlk->Next())
{
res = new (this, CMK_FlowEdge) FlowEdge(filterBlk, blk, res);

assert(filterBlk->VisitEHEnclosedHandlerSecondPassSuccs(this, [blk](BasicBlock* succ) {
return succ == blk ? BasicBlockVisit::Abort : BasicBlockVisit::Continue;
}) == BasicBlockVisit::Abort);
}
}

enclosing = enclosingDsc->ebdEnclosingTryIndex;
}
res = new (this, CMK_FlowEdge) FlowEdge(predBlk, blk, res);
}

domPreds->Set(blk, res);
Expand Down
63 changes: 63 additions & 0 deletions src/tests/JIT/Regression/JitBlue/Runtime_109981/Runtime_109981.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Runtime.CompilerServices;
using Xunit;

public class Runtime_109981
{
[Fact]
public static int TestEntryPoint() => Foo(14);

public static int Foo(int x)
{
if (x == 123)
return 0;

int sum = 9;
for (int i = 0; i < x; i++)
{
sum += i;
}

try
{
if (x != 123)
return sum;

try
{
try
{
Bar();
}
finally
{
sum += 1000;
}
}
catch (ArgumentException)
{
sum += 10000;
}
}
catch when (Filter())
{
sum += 100000;
}

return sum;
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static void Bar()
{
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static bool Filter()
{
return true;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
</Project>

0 comments on commit 4d2ecdc

Please sign in to comment.