JIT: fix bad assumption in non-funclet EH models
authorAndy Ayers <andya@microsoft.com>
Sat, 14 Jan 2017 03:28:30 +0000 (19:28 -0800)
committerAndy Ayers <andya@microsoft.com>
Sat, 14 Jan 2017 03:28:30 +0000 (19:28 -0800)
The newly added finally optimizations mistakenly assumed that in
non-fuclet EH models the GT_END_LFIN in the continuation block would
be the last statement. The test case added below provides an example
where this is not so.

Relax the assumption and instead search the continuation for the
GT_END_LFIN. Assert that there is exactly one.

Commit migrated from https://github.com/dotnet/coreclr/commit/0df4892d1d7e04fc817beb7f10be4faae0dfaf82

src/coreclr/src/jit/flowgraph.cpp
src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_367099/DevDiv_367099.il [new file with mode: 0644]
src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_367099/DevDiv_367099.ilproj [new file with mode: 0644]

index 4b429c3..f48daaa 100644 (file)
@@ -22639,11 +22639,20 @@ void Compiler::fgRemoveEmptyFinally()
 
 #if !FEATURE_EH_FUNCLETS
                 // Remove the GT_END_LFIN from the post-try-finally block.
-                // remove it since there is no finally anymore.
-                GenTreeStmt* endFinallyStmt = postTryFinallyBlock->lastStmt();
-                GenTreePtr   endFinallyExpr = endFinallyStmt->gtStmtExpr;
-                assert(endFinallyExpr->gtOper == GT_END_LFIN);
-                fgRemoveStmt(postTryFinallyBlock, endFinallyStmt);
+                // remove it since there is no finally anymore. Note we only
+                // expect to see one statement.
+                bool foundEndLFin = false;
+                for (GenTreeStmt* stmt = postTryFinallyBlock->firstStmt(); stmt != nullptr; stmt = stmt->gtNextStmt)
+                {
+                    GenTreePtr expr = stmt->gtStmtExpr;
+                    if (expr->gtOper == GT_END_LFIN)
+                    {
+                        assert(!foundEndLFin);
+                        fgRemoveStmt(postTryFinallyBlock, stmt);
+                        foundEndLFin = true;
+                    }
+                }
+                assert(foundEndLFin);
 #endif // !FEATURE_EH_FUNCLETS
 
                 // Make sure iteration isn't going off the deep end.
@@ -23275,12 +23284,21 @@ void Compiler::fgCloneFinally()
 #endif // FEATURE_EH_FUNCLETS && defined(_TARGET_ARM_)
 
 #if !FEATURE_EH_FUNCLETS
-        // Remove the GT_END_LFIN from the normalCallFinallyReturn
-        // since no callfinally returns there anymore.
-        GenTreeStmt* endFinallyStmt = normalCallFinallyReturn->lastStmt();
-        GenTreePtr   endFinallyExpr = endFinallyStmt->gtStmtExpr;
-        assert(endFinallyExpr->gtOper == GT_END_LFIN);
-        fgRemoveStmt(normalCallFinallyReturn, endFinallyStmt);
+        // Remove the GT_END_LFIN from the post-try-finally block.
+        // remove it since there is no finally anymore. Note we only
+        // expect to see one statement.
+        bool foundEndLFin = false;
+        for (GenTreeStmt* stmt = normalCallFinallyReturn->firstStmt(); stmt != nullptr; stmt = stmt->gtNextStmt)
+        {
+            GenTreePtr expr = stmt->gtStmtExpr;
+            if (expr->gtOper == GT_END_LFIN)
+            {
+                assert(!foundEndLFin);
+                fgRemoveStmt(normalCallFinallyReturn, stmt);
+                foundEndLFin = true;
+            }
+        }
+        assert(foundEndLFin);
 #endif
 
         // Todo -- mark cloned blocks as a cloned finally....
diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_367099/DevDiv_367099.il b/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_367099/DevDiv_367099.il
new file mode 100644 (file)
index 0000000..f01b508
--- /dev/null
@@ -0,0 +1,99 @@
+// 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.
+
+.assembly extern legacy library mscorlib {}
+
+.assembly devdiv_367099 {}
+
+.class public auto ansi beforefieldinit P
+       extends [mscorlib]System.Object
+{
+  .method public hidebysig static int32  Main() cil managed
+  {
+    .entrypoint
+    // Code size       14 (0xe)
+    .maxstack  1
+    .locals init (int32 V_0)
+    IL_0000:  nop
+    IL_0001:  call       void P::TestCatchReturn()
+    IL_0006:  nop
+    IL_0007:  ldc.i4.s   100
+    IL_0009:  stloc.0
+    IL_000a:  br.s       IL_000c
+
+    IL_000c:  ldloc.0
+    IL_000d:  ret
+  } // end of method P::Main
+
+  .method public hidebysig static void  TestCatchReturn() cil managed
+  {
+    // Code size       30 (0x1e)
+    .maxstack  1
+    IL_0000:  nop
+    .try
+    {
+      IL_0001:  nop
+      IL_0002:  nop
+      IL_0003:  leave.s    IL_001b
+
+    }  // end .try
+    catch [mscorlib]System.Exception 
+    {
+      IL_0005:  pop
+      IL_0006:  nop
+      .try
+      {
+        IL_0007:  nop
+        .try
+        {
+          .try
+          {
+            IL_0008:  nop
+            IL_0009:  leave.s    IL_001c
+
+          }  // end .try
+          catch [mscorlib]System.Object 
+          {
+            IL_000b:  pop
+            IL_000c:  nop
+            IL_000d:  leave.s    IL_001c
+
+          }  // end handler
+        }  // end .try
+        finally
+        {
+          IL_000f:  nop
+          IL_0010:  nop
+          IL_0011:  endfinally
+        }  // end handler
+      }  // end .try
+      catch [mscorlib]System.Exception 
+      {
+        IL_0012:  pop
+        IL_0013:  nop
+        IL_0014:  nop
+        IL_0015:  leave.s    IL_0017
+
+      }  // end handler
+      IL_0017:  nop
+      IL_0018:  nop
+      IL_0019:  leave.s    IL_001b
+
+    }  // end handler
+    IL_001b:  nop
+    IL_001c:  nop
+    IL_001d:  ret
+  } // end of method P::TestCatchReturn
+
+  .method public hidebysig specialname rtspecialname 
+          instance void  .ctor() cil managed
+  {
+    // Code size       7 (0x7)
+    .maxstack  8
+    IL_0000:  ldarg.0
+    IL_0001:  call       instance void [mscorlib]System.Object::.ctor()
+    IL_0006:  ret
+  } // end of method P::.ctor
+
+} // end of class P
diff --git a/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_367099/DevDiv_367099.ilproj b/src/coreclr/tests/src/JIT/Regression/JitBlue/DevDiv_367099/DevDiv_367099.ilproj
new file mode 100644 (file)
index 0000000..bb47373
--- /dev/null
@@ -0,0 +1,51 @@
+<?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>
+    <AssemblyName>$(MSBuildProjectName)</AssemblyName>
+    <SchemaVersion>2.0</SchemaVersion>
+    <ProjectGuid>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</ProjectGuid>
+    <OutputType>Exe</OutputType>
+    <AppDesignerFolder>Properties</AppDesignerFolder>
+    <FileAlignment>512</FileAlignment>
+    <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+    <ReferencePath>$(ProgramFiles)\Common Files\microsoft shared\VSTT\11.0\UITestExtensionPackages</ReferencePath>
+    <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
+    <NuGetPackageImportStamp>7a9bfb7d</NuGetPackageImportStamp>
+  </PropertyGroup>
+  <!-- Default configurations to help VS understand the configurations -->
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
+  </PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
+  </PropertyGroup>
+  <ItemGroup>
+    <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
+      <Visible>False</Visible>
+    </CodeAnalysisDependentAssemblyPaths>
+  </ItemGroup>
+  <PropertyGroup>
+    <DebugType>None</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="DevDiv_367099.il" />
+  </ItemGroup>
+  <PropertyGroup>
+    <CLRTestBatchPreCommands><![CDATA[
+$(CLRTestBatchPreCommands)
+set COMPlus_JitStressRegs=0x200
+]]></CLRTestBatchPreCommands>
+  <BashCLRTestPreCommands><![CDATA[
+$(BashCLRTestPreCommands)
+export COMPlus_JitStressRegs=0x200
+]]></BashCLRTestPreCommands>
+  </PropertyGroup>
+  <ItemGroup>
+    <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+  </ItemGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+  <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' ">
+  </PropertyGroup> 
+</Project>