Skip to content

Commit

Permalink
JIT: fix filter liveness computation
Browse files Browse the repository at this point in the history
When a filter is finished executing, control can logically pass to the
associated handler, any enclosing handler or filter, or any finally or fault
handler nested within the associated try. This is a consequence of two-pass EH.

The jit was not propagating liveness from the nested handlers, which lead to a
live object being collected inadvertently.

This change updates `fgGetHandlerLiveVars` to find the nested handlers and
merge their live-in into the filter block live sets.

Because these implicit EH flow edges can create cycles in the liveness dataflow
equations, the jit will also now always iterate liveness when it sees there is
exception flow, to ensure livness reaches the appropriate fixed point.

Added test case.

Closes #22820.
  • Loading branch information
AndyAyersMS committed Mar 5, 2019
1 parent 3c0d755 commit 1751366
Show file tree
Hide file tree
Showing 4 changed files with 185 additions and 6 deletions.
97 changes: 92 additions & 5 deletions src/jit/liveness.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1056,6 +1056,26 @@ void Compiler::fgExtendDbgLifetimes()
#endif // DEBUG
}

//------------------------------------------------------------------------
// fgGetHandlerLiveVars: determine set of locals live because of implicit
// exception flow from a block.
//
// Arguments:
// block - the block in question
//
// Returns:
// Additional set of locals to be considered live throughout the block.
//
// Notes:
// Assumes caller has screened candidate blocks to only those with
// exception flow, via `ehBlockHasExnFlowDsc`.
//
// Exception flow can arise because of a newly raised exception (for
// blocks within try regions) or because of an actively propagating exception
// (for filter blocks). This flow effectively creates additional successor
// edges in the flow graph that the jit does not model. This method computes
// the net contribution from all the missing successor edges.

VARSET_VALRET_TP Compiler::fgGetHandlerLiveVars(BasicBlock* block)
{
noway_assert(block);
Expand All @@ -1067,7 +1087,6 @@ VARSET_VALRET_TP Compiler::fgGetHandlerLiveVars(BasicBlock* block)
do
{
/* Either we enter the filter first or the catch/finally */

if (HBtab->HasFilter())
{
VarSetOps::UnionD(this, liveVars, HBtab->ebdFilter->bbLiveIn);
Expand Down Expand Up @@ -1099,6 +1118,72 @@ VARSET_VALRET_TP Compiler::fgGetHandlerLiveVars(BasicBlock* block)

} while (true);

// If this block is within a filter, we also need to report as live
// any vars live into enclosed finally or fault handlers, since the
// filter will run during the first EH pass, and enclosed or enclosing
// handlers will run during the second EH pass. So all these handlers
// are "exception flow" successors of the filter.
//
// Note we are relying on ehBlockHasExnFlowDsc to return true
// for any filter block that we should examine here.
if (block->hasHndIndex())
{
const unsigned thisHndIndex = block->getHndIndex();
EHblkDsc* enclosingHBtab = ehGetDsc(thisHndIndex);

if (enclosingHBtab->InFilterRegionBBRange(block))
{
assert(enclosingHBtab->HasFilter());

// Search the EH table for enclosed regions.
//
// All the enclosed regions will be lower numbered and
// immediately prior to and contiguous with the enclosing
// region in the EH tab.
unsigned index = thisHndIndex;

while (index > 0)
{
index--;
unsigned enclosingIndex = ehGetEnclosingTryIndex(index);
bool isEnclosed = false;

// To verify this is an enclosed region, search up
// through the enclosing regions until we find the
// region associated with the filter.
while (enclosingIndex != EHblkDsc::NO_ENCLOSING_INDEX)
{
if (enclosingIndex == thisHndIndex)
{
isEnclosed = true;
break;
}

enclosingIndex = ehGetEnclosingTryIndex(enclosingIndex);
}

// If we found an enclosed region, check if the region
// is a try fault or try finally, and if so, add any
// locals live into the enclosed region's handler into this
// block's live-in set.
if (isEnclosed)
{
EHblkDsc* enclosedHBtab = ehGetDsc(index);

if (enclosedHBtab->HasFinallyOrFaultHandler())
{
VarSetOps::UnionD(this, liveVars, enclosedHBtab->ebdHndBeg->bbLiveIn);
}
}
// Once we run across a non-enclosed region, we can stop searching.
else
{
break;
}
}
}
}

return liveVars;
}

Expand Down Expand Up @@ -1171,14 +1256,17 @@ class LiveVarAnalysis
// since (without proof otherwise) the use and def may touch different memory at run-time.
m_memoryLiveIn = m_memoryLiveOut | block->bbMemoryUse;

/* Can exceptions from this block be handled (in this function)? */

// Does this block have implicit exception flow to a filter or handler?
// If so, include the effects of that flow.
if (m_compiler->ehBlockHasExnFlowDsc(block))
{
const VARSET_TP& liveVars(m_compiler->fgGetHandlerLiveVars(block));

VarSetOps::UnionD(m_compiler, m_liveIn, liveVars);
VarSetOps::UnionD(m_compiler, m_liveOut, liveVars);

// Implicit eh edges can induce loop-like behavior,
// so make sure we iterate to closure.
m_hasPossibleBackEdge = true;
}

/* Has there been any change in either live set? */
Expand Down Expand Up @@ -2399,7 +2487,6 @@ void Compiler::fgInterBlockLocalVarLiveness()
if (block->bbCatchTyp != BBCT_NONE)
{
/* Note the set of variables live on entry to exception handler */

VarSetOps::UnionD(this, exceptVars, block->bbLiveIn);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Runtime.CompilerServices;

Expand Down Expand Up @@ -46,4 +50,4 @@ public static int Main()
Console.WriteLine($"Result={result}");
return result;
}
}
}
55 changes: 55 additions & 0 deletions tests/src/JIT/Regression/JitBlue/GitHub_22820/GitHub_22820.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;

// Repro for issue 22820. On x86 we need to report enclosed handler
// live-in locals as live into any enclosing filter.
//
// Run with optimized codegen and COMPlus_GCStress=0x4

class DisposableObject : IDisposable
{
public void Dispose()
{
Console.WriteLine("In dispose");
}
}

class Program
{
public static bool IsExpectedException(Exception e)
{
Console.WriteLine("In filter");
GC.Collect();
return e is OperationCanceledException;
}

public static IDisposable AllocateObject()
{
return new DisposableObject();
}

static void Main(string[] args)
{
try
{
try
{
using (AllocateObject())
{
throw new Exception();
}
}
catch (Exception e1) when (IsExpectedException(e1))
{
Console.WriteLine("In catch 1");
}
}
catch (Exception e2)
{
Console.WriteLine("In catch 2");
}
}
}
33 changes: 33 additions & 0 deletions tests/src/JIT/Regression/JitBlue/GitHub_22820/GitHub_22820.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
<PropertyGroup>
<Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
<Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
<SchemaVersion>2.0</SchemaVersion>
<ProjectGuid>{2649FAFE-07BF-4F93-8120-BA9A69285ABB}</ProjectGuid>
<OutputType>Exe</OutputType>
<ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
<SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
</PropertyGroup>
<!-- Default configurations to help VS understand the configurations -->
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "></PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' "></PropertyGroup>
<PropertyGroup>
<DebugType>None</DebugType>
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
<Visible>False</Visible>
</CodeAnalysisDependentAssemblyPaths>
</ItemGroup>
<ItemGroup>
<Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
</ItemGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
<PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "></PropertyGroup>
</Project>

0 comments on commit 1751366

Please sign in to comment.