Browse Source

Fix #1767: Fix detection switch-on-string that uses leave instead of branch instructions.

pull/1790/head
Siegfried Pammer 6 years ago
parent
commit
9a77a80e3c
  1. 60
      ICSharpCode.Decompiler.Tests/TestCases/Pretty/Switch.cs
  2. 46
      ICSharpCode.Decompiler/IL/Transforms/SwitchOnStringTransform.cs

60
ICSharpCode.Decompiler.Tests/TestCases/Pretty/Switch.cs

@ -47,6 +47,8 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -47,6 +47,8 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
Null
}
private static char ch1767;
#if !ROSLYN
public static State SwitchOverNullableBool(bool? value)
{
@ -465,7 +467,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -465,7 +467,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
}
Console.WriteLine("End of method");
}
public static void SwitchWithGotoComplex(string s)
{
Console.WriteLine("SwitchWithGotoComplex: " + s);
@ -640,7 +642,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -640,7 +642,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
Console.WriteLine();
}
}
// while condition, return and break cases
public static void SwitchWithContinue2(int i, bool b)
{
@ -681,7 +683,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -681,7 +683,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
i++;
}
}
// for loop version
public static void SwitchWithContinue3(bool b)
{
@ -721,7 +723,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -721,7 +723,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
Console.WriteLine("loop-tail");
}
}
// foreach version
public static void SwitchWithContinue4(bool b)
{
@ -843,7 +845,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -843,7 +845,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
continue;
}
Console.WriteLine("loop-tail");
} while (++i < 10);
} while (++i < 10);
}
// double break from switch to loop exit requires additional pattern matching in HighLevelLoopTransform
@ -918,7 +920,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -918,7 +920,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
}
// These decompile poorly into switch statements and should be left as is
#region Overagressive Switch Use
#region Overagressive Switch Use
#if ROSLYN || OPT
public static void SingleIf1(int i, bool a)
@ -929,7 +931,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -929,7 +931,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
Console.WriteLine(2);
}
#endif
public static void SingleIf2(int i, bool a, bool b)
{
if (i == 1 || (i == 2 && a) || (i == 3 && b)) {
@ -937,7 +939,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -937,7 +939,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
}
Console.WriteLine(2);
}
public static void SingleIf3(int i, bool a, bool b)
{
if (a || i == 1 || (i == 2 && b)) {
@ -945,7 +947,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -945,7 +947,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
}
Console.WriteLine(2);
}
public static void SingleIf4(int i, bool a)
{
if (i == 1 || i == 2 || (i != 3 && a) || i != 4) {
@ -964,7 +966,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -964,7 +966,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
}
Console.WriteLine();
}
public static void IfChainWithCondition(int i)
{
if (i == 0) {
@ -985,7 +987,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -985,7 +987,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
Console.WriteLine();
}
public static bool SwitchlikeIf(int i, int j)
{
if (i != 0 && j != 0) {
@ -1023,7 +1025,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -1023,7 +1025,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
}
return false;
}
return true;
}
@ -1052,7 +1054,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -1052,7 +1054,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
}
Console.WriteLine("end");
}
public static bool Loop8(char c, bool b, Func<char> getChar)
{
if (b) {
@ -1071,8 +1073,8 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -1071,8 +1073,8 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
c = getChar();
} while (c != -1 && c != '\n' && c != '\u2028' && c != '\u2029');
}
#endregion
#endregion
// Ensure correctness of SwitchDetection.UseCSharpSwitch control flow heuristics
public static void SwitchWithBreakCase(int i, bool b)
{
@ -1108,7 +1110,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -1108,7 +1110,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
}
Console.WriteLine();
}
public static int SwitchWithReturnAndBreak2(int i, bool b)
{
switch (i) {
@ -1130,7 +1132,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -1130,7 +1132,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
Console.WriteLine();
return 0;
}
public static void SwitchWithReturnAndBreak3(int i)
{
switch (i) {
@ -1229,5 +1231,29 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty @@ -1229,5 +1231,29 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
}
return key != (ConsoleKey)0;
}
public static void Issue1767(string s)
{
switch (s) {
case "a":
ch1767 = s[0];
break;
case "b":
ch1767 = s[0];
break;
case "c":
ch1767 = s[0];
break;
case "d":
ch1767 = s[0];
break;
case "e":
ch1767 = s[0];
break;
case "f":
ch1767 = s[0];
break;
}
}
}
}

46
ICSharpCode.Decompiler/IL/Transforms/SwitchOnStringTransform.cs

@ -154,26 +154,33 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -154,26 +154,33 @@ namespace ICSharpCode.Decompiler.IL.Transforms
bool SimplifyCascadingIfStatements(InstructionCollection<ILInstruction> instructions, ref int i)
{
if (i < 1) return false;
// match first block: checking switch-value for null or first value (Roslyn)
// if (call op_Equality(ldloc switchValueVar, ldstr value)) br firstBlock
// -or-
// if (comp(ldloc switchValueVar == ldnull)) br defaultBlock
if (!(instructions[i].MatchIfInstruction(out var condition, out var firstBlockJump)))
if (!instructions[i].MatchIfInstruction(out var condition, out var firstBlockOrDefaultJump))
return false;
if (!firstBlockJump.MatchBranch(out var firstBlock))
if (firstBlockOrDefaultJump.MatchBranch(out var firstBlock)) {
// success
} else if (firstBlockOrDefaultJump.MatchLeave(out _)) {
firstBlock = null;
// success
} else {
return false;
List<(string, Block)> values = new List<(string, Block)>();
}
List<(string, ILInstruction)> values = new List<(string, ILInstruction)>();
ILInstruction switchValue = null;
// match call to operator ==(string, string)
if (!MatchStringEqualityComparison(condition, out var switchValueVar, out string firstBlockValue))
return false;
values.Add((firstBlockValue, firstBlock));
values.Add((firstBlockValue, firstBlock ?? firstBlockOrDefaultJump));
bool extraLoad = false;
bool keepAssignmentBefore = false;
if (instructions[i - 1].MatchStLoc(switchValueVar, out switchValue)) {
if (i >= 1 && instructions[i - 1].MatchStLoc(switchValueVar, out switchValue)) {
// stloc switchValueVar(switchValue)
// if (call op_Equality(ldloc switchValueVar, ldstr value)) br firstBlock
@ -183,7 +190,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -183,7 +190,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
switchValue = newSwitchValue;
extraLoad = true;
}
} else if (instructions[i - 1] is StLoc stloc) {
} else if (i >= 1 && instructions[i - 1] is StLoc stloc) {
if (stloc.Value.MatchLdLoc(switchValueVar)) {
// in case of optimized legacy code there are two stlocs:
// stloc otherSwitchValueVar(ldloc switchValue)
@ -212,10 +219,12 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -212,10 +219,12 @@ namespace ICSharpCode.Decompiler.IL.Transforms
return false;
// extract all cases and add them to the values list.
Block currentCaseBlock = nextCaseJump.TargetBlock;
Block nextCaseBlock;
ILInstruction nextCaseBlock;
while ((nextCaseBlock = MatchCaseBlock(currentCaseBlock, switchValueVar, out string value, out Block block)) != null) {
values.Add((value, block));
currentCaseBlock = nextCaseBlock;
currentCaseBlock = nextCaseBlock as Block;
if (currentCaseBlock == null)
break;
}
// We didn't find enough cases, exit
if (values.Count < 3)
@ -225,9 +234,10 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -225,9 +234,10 @@ namespace ICSharpCode.Decompiler.IL.Transforms
keepAssignmentBefore = true;
switchValue = new LdLoc(switchValueVar);
}
var sections = new List<SwitchSection>(values.SelectWithIndex((index, b) => new SwitchSection { Labels = new LongSet(index), Body = new Branch(b.Item2) }));
sections.Add(new SwitchSection { Labels = new LongSet(new LongInterval(0, sections.Count)).Invert(), Body = new Branch(currentCaseBlock) });
var stringToInt = new StringToInt(switchValue, values.SelectArray(item => item.Item1));
int offset = firstBlock == null ? 1 : 0;
var sections = new List<SwitchSection>(values.Skip(offset).SelectWithIndex((index, b) => new SwitchSection { Labels = new LongSet(index), Body = new Branch((Block)b.Item2) }));
sections.Add(new SwitchSection { Labels = new LongSet(new LongInterval(0, sections.Count)).Invert(), Body = currentCaseBlock != null ? (ILInstruction)new Branch(currentCaseBlock) : new Leave((BlockContainer)nextCaseBlock) });
var stringToInt = new StringToInt(switchValue, values.Skip(offset).Select(item => item.Item1).ToArray());
var inst = new SwitchInstruction(stringToInt);
inst.Sections.AddRange(sections);
if (extraLoad) {
@ -357,7 +367,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -357,7 +367,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
/// The <paramref name="switchVariable"/> is updated if the value gets copied to a different variable.
/// See comments below for more info.
/// </summary>
Block MatchCaseBlock(Block currentBlock, ILVariable switchVariable, out string value, out Block caseBlock)
ILInstruction MatchCaseBlock(Block currentBlock, ILVariable switchVariable, out string value, out Block caseBlock)
{
value = null;
caseBlock = null;
@ -369,19 +379,25 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -369,19 +379,25 @@ namespace ICSharpCode.Decompiler.IL.Transforms
if (!caseBlockBranch.MatchBranch(out caseBlock))
return null;
Block nextBlock;
BlockContainer blockContainer = null;
if (condition.MatchLogicNot(out var inner)) {
condition = inner;
nextBlock = caseBlock;
if (!currentBlock.Instructions[1].MatchBranch(out caseBlock))
return null;
} else {
if (!currentBlock.Instructions[1].MatchBranch(out nextBlock))
if (currentBlock.Instructions[1].MatchBranch(out nextBlock)) {
// success
} else if (currentBlock.Instructions[1].MatchLeave(out blockContainer)) {
// success
} else {
return null;
}
}
if (!MatchStringEqualityComparison(condition, switchVariable, out value)) {
return null;
}
return nextBlock;
return nextBlock ?? (ILInstruction)blockContainer;
}
/// <summary>

Loading…
Cancel
Save