From 30da0b7525c3e53dec9b41f5b1318ce4baa59aa0 Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Tue, 23 Jun 2020 23:22:41 +0200 Subject: [PATCH] Add some missing checks to MatchRoslynSwitchOnString --- .../ControlFlow/ControlFlowSimplification.cs | 2 +- .../IL/ControlFlow/SwitchDetection.cs | 10 ++++--- .../Transforms/SwitchOnNullableTransform.cs | 2 +- .../IL/Transforms/SwitchOnStringTransform.cs | 26 +++++++++++++------ 4 files changed, 26 insertions(+), 14 deletions(-) diff --git a/ICSharpCode.Decompiler/IL/ControlFlow/ControlFlowSimplification.cs b/ICSharpCode.Decompiler/IL/ControlFlow/ControlFlowSimplification.cs index 5ef4f8ad3..59e4387f7 100644 --- a/ICSharpCode.Decompiler/IL/ControlFlow/ControlFlowSimplification.cs +++ b/ICSharpCode.Decompiler/IL/ControlFlow/ControlFlowSimplification.cs @@ -51,7 +51,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow InlineVariableInReturnBlock(block, context); // 1st pass SimplifySwitchInstruction before SimplifyBranchChains() // starts duplicating return instructions. - SwitchDetection.SimplifySwitchInstruction(block); + SwitchDetection.SimplifySwitchInstruction(block, context); } SimplifyBranchChains(function, context); CleanUpEmptyBlocks(function, context); diff --git a/ICSharpCode.Decompiler/IL/ControlFlow/SwitchDetection.cs b/ICSharpCode.Decompiler/IL/ControlFlow/SwitchDetection.cs index e7d771ddf..9ae80f766 100644 --- a/ICSharpCode.Decompiler/IL/ControlFlow/SwitchDetection.cs +++ b/ICSharpCode.Decompiler/IL/ControlFlow/SwitchDetection.cs @@ -24,6 +24,7 @@ using System.Diagnostics; using ICSharpCode.Decompiler.FlowAnalysis; using ICSharpCode.Decompiler.Util; using ICSharpCode.Decompiler.TypeSystem; +using ICSharpCode.Decompiler.CSharp.Transforms; namespace ICSharpCode.Decompiler.IL.ControlFlow { @@ -198,11 +199,11 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow } else { // 2nd pass of SimplifySwitchInstruction (after duplicating return blocks), // (1st pass was in ControlFlowSimplification) - SimplifySwitchInstruction(block); + SimplifySwitchInstruction(block, context); } } - internal static void SimplifySwitchInstruction(Block block) + internal static void SimplifySwitchInstruction(Block block, ILTransformContext context) { // due to our of of basic blocks at this point, // switch instructions can only appear as last insturction @@ -228,7 +229,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow } return false; }); - AdjustLabels(sw); + AdjustLabels(sw, context); SortSwitchSections(sw); } @@ -237,10 +238,11 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow sw.Sections.ReplaceList(sw.Sections.OrderBy(s => (s.Body as Branch)?.TargetILOffset).ThenBy(s => s.Labels.Values.FirstOrDefault())); } - static void AdjustLabels(SwitchInstruction sw) + static void AdjustLabels(SwitchInstruction sw, ILTransformContext context) { if (sw.Value is BinaryNumericInstruction bop && !bop.CheckForOverflow && bop.Right.MatchLdcI(out long val)) { // Move offset into labels: + context.Step("Move offset into switch labels", bop); long offset; switch (bop.Operator) { case BinaryNumericOperator.Add: diff --git a/ICSharpCode.Decompiler/IL/Transforms/SwitchOnNullableTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/SwitchOnNullableTransform.cs index 9048bb21b..b63ba77d9 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/SwitchOnNullableTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/SwitchOnNullableTransform.cs @@ -59,7 +59,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms } } if (!changed) continue; - SwitchDetection.SimplifySwitchInstruction(block); + SwitchDetection.SimplifySwitchInstruction(block, context); if (block.Parent is BlockContainer container) changedContainers.Add(container); } diff --git a/ICSharpCode.Decompiler/IL/Transforms/SwitchOnStringTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/SwitchOnStringTransform.cs index 7b39f389c..216c1c79f 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/SwitchOnStringTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/SwitchOnStringTransform.cs @@ -33,13 +33,16 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// class SwitchOnStringTransform : IILTransform { + ILTransformContext context; + public void Run(ILFunction function, ILTransformContext context) { if (!context.Settings.SwitchStatementOnString) return; + this.context = context; BlockContainer body = (BlockContainer)function.Body; - var hashtableInitializers = ScanHashtableInitializerBlocks(body.EntryPoint); + var hashtableInitializers = ScanHashtableInitializerBlocks(body.EntryPoint); HashSet changedContainers = new HashSet(); @@ -70,7 +73,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms } } if (!changed) continue; - SwitchDetection.SimplifySwitchInstruction(block); + SwitchDetection.SimplifySwitchInstruction(block, context); if (block.Parent is BlockContainer container) changedContainers.Add(container); } @@ -83,6 +86,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms if (!transformed) continue; if (!omittedBlocks.TryGetValue(previous, out var actual)) actual = previous; + context.Step("Remove hashtable initializer", actual); if (jumpToNext != null) { actual.Instructions.SecondToLastOrDefault().ReplaceWith(jumpToNext); } @@ -95,6 +99,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms if (hashtableInitializers.Count > 0 && omittedBlocks.Count == hashtableInitializers.Count && body.EntryPoint.Instructions.Count == 2) { if (body.EntryPoint.Instructions[0] is IfInstruction ifInst && ifInst.TrueInst.MatchBranch(out var beginOfMethod) && body.EntryPoint.Instructions[1].MatchBranch(beginOfMethod)) { + context.Step("Remove initial null check", body); body.EntryPoint.Instructions.RemoveAt(0); } } @@ -345,7 +350,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms var stringToInt = new StringToInt(switchValue, values.SelectArray(item => item.Item1)); var inst = new SwitchInstruction(stringToInt); inst.Sections.AddRange(sections); - + inst.AddILRange(instructions[i - 1]); instructions[i].ReplaceWith(inst); instructions.RemoveAt(i + 1); @@ -828,9 +833,9 @@ namespace ICSharpCode.Decompiler.IL.Transforms InstructionCollection switchBlockInstructions = instructions; int switchBlockInstructionsOffset = i; Block nullValueCaseBlock = null; + ILInstruction instForNullCheck = null; if (instructions[i].MatchIfInstruction(out var condition, out var exitBlockJump) - && condition.MatchCompEquals(out var left, out var right) && right.MatchLdNull()) - { + && condition.MatchCompEqualsNull(out instForNullCheck)) { var nextBlockJump = instructions[i + 1] as Branch; if (nextBlockJump == null || nextBlockJump.TargetBlock.IncomingEdgeCount != 1) return false; @@ -849,9 +854,14 @@ namespace ICSharpCode.Decompiler.IL.Transforms MatchComputeStringHashCall(switchBlockInstructions[switchBlockInstructionsOffset], switchValueVar, out LdLoc switchValueLoad))) return false; + if (instForNullCheck != null && !instForNullCheck.MatchLdLoc(switchValueLoad.Variable)) { + return false; + } + var stringValues = new List<(string Value, ILInstruction TargetBlockOrLeave)>(); SwitchSection defaultSection = switchInst.Sections.MaxBy(s => s.Labels.Count()); - Block exitOrDefaultBlock = null; + if (!defaultSection.Body.MatchBranch(out Block exitOrDefaultBlock)) + return false; foreach (var section in switchInst.Sections) { if (section == defaultSection) continue; // extract target block @@ -866,9 +876,8 @@ namespace ICSharpCode.Decompiler.IL.Transforms return false; } - if (exitOrDefaultBlock != null && exitOrDefaultBlock != currentExitBlock) + if (currentExitBlock != exitOrDefaultBlock) return false; - exitOrDefaultBlock = currentExitBlock; if (emptyStringEqualsNull && string.IsNullOrEmpty(stringValue)) { stringValues.Add((null, targetOrLeave)); stringValues.Add((string.Empty, targetOrLeave)); @@ -881,6 +890,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms stringValues.Add((null, nullValueCaseBlock)); } + context.Step(nameof(MatchRoslynSwitchOnString), switchValueLoad); ILInstruction switchValueInst = switchValueLoad; if (instructions == switchBlockInstructions) { // stloc switchValueLoadVariable(switchValue)