Skip to content

Commit f552567

Browse files
authored
JIT: fix try region cloning when try is nested in a handler (#111975)
We were not placing the new EH region in the table properly, and not correctly updating the handler indexes of cloned blocks. Add some test cases where we have cloneable loops with EH inside of finallies. Fixes some issues that came up testing #111473
1 parent 3688b59 commit f552567

File tree

2 files changed

+215
-15
lines changed

2 files changed

+215
-15
lines changed

src/coreclr/jit/fgehopt.cpp

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2761,14 +2761,20 @@ BasicBlock* Compiler::fgCloneTryRegion(BasicBlock* tryEntry, CloneTryInfo& info,
27612761
// this is cheaper than any other insertion point, as no existing regions get renumbered.
27622762
//
27632763
unsigned insertBeforeIndex = enclosingTryIndex;
2764-
if (insertBeforeIndex == EHblkDsc::NO_ENCLOSING_INDEX)
2764+
if ((enclosingTryIndex == EHblkDsc::NO_ENCLOSING_INDEX) && (enclosingHndIndex == EHblkDsc::NO_ENCLOSING_INDEX))
27652765
{
2766-
JITDUMP("Cloned EH clauses will go at the end of the EH table\n");
2766+
JITDUMP("No enclosing EH region; cloned EH clauses will go at the end of the EH table\n");
27672767
insertBeforeIndex = compHndBBtabCount;
27682768
}
2769+
else if ((enclosingTryIndex == EHblkDsc::NO_ENCLOSING_INDEX) || (enclosingHndIndex < enclosingTryIndex))
2770+
{
2771+
JITDUMP("Cloned EH clauses will go before enclosing handler region EH#%02u\n", enclosingHndIndex);
2772+
insertBeforeIndex = enclosingHndIndex;
2773+
}
27692774
else
27702775
{
2771-
JITDUMP("Cloned EH clauses will go before enclosing region EH#%02u\n", enclosingTryIndex);
2776+
JITDUMP("Cloned EH clauses will go before enclosing try region EH#%02u\n", enclosingTryIndex);
2777+
assert(insertBeforeIndex == enclosingTryIndex);
27722778
}
27732779

27742780
// Once we call fgTryAddEHTableEntries with deferCloning = false,
@@ -2989,7 +2995,7 @@ BasicBlock* Compiler::fgCloneTryRegion(BasicBlock* tryEntry, CloneTryInfo& info,
29892995
const unsigned originalTryIndex = block->getTryIndex();
29902996
unsigned cloneTryIndex = originalTryIndex;
29912997

2992-
if (originalTryIndex <= outermostTryIndex)
2998+
if (originalTryIndex < enclosingTryIndex)
29932999
{
29943000
cloneTryIndex += indexShift;
29953001
}
@@ -3003,11 +3009,15 @@ BasicBlock* Compiler::fgCloneTryRegion(BasicBlock* tryEntry, CloneTryInfo& info,
30033009
if (block->hasHndIndex())
30043010
{
30053011
const unsigned originalHndIndex = block->getHndIndex();
3012+
unsigned cloneHndIndex = originalHndIndex;
3013+
3014+
if (originalHndIndex < enclosingHndIndex)
3015+
{
3016+
cloneHndIndex += indexShift;
3017+
}
30063018

3007-
// if (originalHndIndex ==
3008-
const unsigned cloneHndIndex = originalHndIndex + indexShift;
3009-
EHblkDsc* const originalEbd = ehGetDsc(originalHndIndex);
3010-
EHblkDsc* const clonedEbd = ehGetDsc(cloneHndIndex);
3019+
EHblkDsc* const originalEbd = ehGetDsc(originalHndIndex);
3020+
EHblkDsc* const clonedEbd = ehGetDsc(cloneHndIndex);
30113021
newBlock->setHndIndex(cloneHndIndex);
30123022
updateBlockReferences(cloneHndIndex);
30133023

src/tests/JIT/opt/Cloning/loops_with_eh.cs

Lines changed: 197 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
using System.Runtime.CompilerServices;
66
using Xunit;
77

8-
98
// Cheat codes
109
//
1110
// L - loop
@@ -17,6 +16,7 @@
1716
// m - multiple try exits (TF will remain a try finally)
1817
// g - giant finally (TF will remain try finally)
1918
// p - regions are serial, not nested
19+
// TFi - try finally with what follows in the finally
2020
//
2121
// x: we currently cannot clone loops where the try is the first thing
2222
// as the header and preheader are different regions
@@ -26,6 +26,9 @@ public class LoopsWithEH
2626
static int[] data;
2727
static int n;
2828

29+
[MethodImpl(MethodImplOptions.NoInlining)]
30+
static void SideEffect() { }
31+
2932
static LoopsWithEH()
3033
{
3134
data = new int[100];
@@ -178,7 +181,7 @@ public static int Sum_LxTCC(int[] data, int n)
178181
{
179182
return -1;
180183
}
181-
catch(Exception)
184+
catch (Exception)
182185
{
183186
return -2;
184187
}
@@ -201,7 +204,7 @@ public static int Sum_LxTCcC(int[] data, int n)
201204
}
202205
catch (IndexOutOfRangeException)
203206
{
204-
sum +=1;
207+
sum += 1;
205208
}
206209
catch (Exception)
207210
{
@@ -536,7 +539,7 @@ public static int Sum_TCLxTfC(int[] data, int n)
536539
}
537540
}
538541
}
539-
catch (Exception)
542+
catch (Exception)
540543
{
541544
return -1;
542545
}
@@ -642,7 +645,7 @@ public static int Sum_TCLxTF(int[] data, int n)
642645
{
643646
sum += data[i];
644647
}
645-
finally
648+
finally
646649
{
647650
sum += 1;
648651
}
@@ -807,7 +810,7 @@ public static int Sum_LxTFTF(int[] data, int n)
807810

808811
[Fact]
809812
public static int Test_LxTFxTF() => Sum_LxTFTF(data, n) - 110;
810-
813+
811814
public static int Sum_TFLxTF(int[] data, int n)
812815
{
813816
int sum = 0;
@@ -896,7 +899,7 @@ public static int Sum_TCTFLxTF(int[] data, int n)
896899
sum += 1;
897900
}
898901
}
899-
catch(Exception)
902+
catch (Exception)
900903
{
901904
return -1;
902905
}
@@ -937,5 +940,192 @@ public static int Sum_TFTCLxTF(int[] data, int n)
937940
}
938941
return sum;
939942
}
943+
944+
[Fact]
945+
public static int Test_TFiL() => Sum_TFiL(data, n) - 91;
946+
947+
public static int Sum_TFiL(int[] data, int n)
948+
{
949+
int sum = 0;
950+
try
951+
{
952+
SideEffect();
953+
}
954+
finally
955+
{
956+
sum += 1;
957+
for (int i = 0; i < n; i++)
958+
{
959+
sum += data[i];
960+
}
961+
}
962+
963+
return sum;
964+
}
965+
966+
[Fact]
967+
public static int Test_TFiLxTF() => Sum_TFiLxTF(data, n) - 131;
968+
969+
public static int Sum_TFiLxTF(int[] data, int n)
970+
{
971+
int sum = 0;
972+
try
973+
{
974+
SideEffect();
975+
}
976+
finally
977+
{
978+
sum += 1;
979+
for (int i = 0; i < n; i++)
980+
{
981+
sum += 1;
982+
try
983+
{
984+
sum += data[i];
985+
}
986+
finally
987+
{
988+
sum += 1;
989+
}
990+
}
991+
}
992+
993+
return sum;
994+
}
995+
996+
[Fact]
997+
public static int Test_TFiLxTCc() => Sum_TFiLxTCc(data, n) - 111;
998+
999+
public static int Sum_TFiLxTCc(int[] data, int n)
1000+
{
1001+
int sum = 0;
1002+
try
1003+
{
1004+
SideEffect();
1005+
}
1006+
finally
1007+
{
1008+
sum += 1;
1009+
for (int i = 0; i < n; i++)
1010+
{
1011+
sum += 1;
1012+
try
1013+
{
1014+
sum += data[i];
1015+
}
1016+
catch (Exception)
1017+
{
1018+
sum += 1;
1019+
}
1020+
}
1021+
}
1022+
1023+
return sum;
1024+
}
1025+
1026+
[Fact]
1027+
public static int Test_TFiLxTC() => Sum_TFiLxTC(data, n) - 112;
1028+
1029+
public static int Sum_TFiLxTC(int[] data, int n)
1030+
{
1031+
int sum = 0;
1032+
try
1033+
{
1034+
SideEffect();
1035+
}
1036+
finally
1037+
{
1038+
sum += 1;
1039+
for (int i = 0; i < n; i++)
1040+
{
1041+
sum += 1;
1042+
try
1043+
{
1044+
sum += data[i];
1045+
}
1046+
catch (Exception)
1047+
{
1048+
goto after_loop;
1049+
}
1050+
}
1051+
1052+
after_loop:
1053+
sum += 1;
1054+
1055+
}
1056+
1057+
return sum;
1058+
}
1059+
1060+
[Fact]
1061+
public static int Test_TFTFiLxTC() => Sum_TFTFiLxTC(data, n) - 113;
1062+
1063+
public static int Sum_TFTFiLxTC(int[] data, int n)
1064+
{
1065+
int sum = 0;
1066+
try
1067+
{
1068+
try
1069+
{
1070+
SideEffect();
1071+
}
1072+
finally
1073+
{
1074+
sum += 1;
1075+
for (int i = 0; i < n; i++)
1076+
{
1077+
sum += 1;
1078+
try
1079+
{
1080+
sum += data[i];
1081+
}
1082+
catch (Exception)
1083+
{
1084+
goto after_loop;
1085+
}
1086+
}
1087+
1088+
after_loop:
1089+
sum += 1;
1090+
}
1091+
}
1092+
finally
1093+
{
1094+
sum += 1;
1095+
}
1096+
1097+
return sum;
1098+
}
1099+
1100+
1101+
[Fact]
1102+
public static int Test_TFiTFxL() => Sum_TFiTFxL(data, n) - 92;
1103+
1104+
public static int Sum_TFiTFxL(int[] data, int n)
1105+
{
1106+
int sum = 0;
1107+
1108+
try
1109+
{
1110+
SideEffect();
1111+
}
1112+
finally
1113+
{
1114+
try
1115+
{
1116+
sum += 1;
1117+
for (int i = 0; i < n; i++)
1118+
{
1119+
sum += data[i];
1120+
}
1121+
}
1122+
finally
1123+
{
1124+
sum += 1;
1125+
}
1126+
}
1127+
1128+
return sum;
1129+
}
9401130
}
9411131

0 commit comments

Comments
 (0)