From 5a2302089bd7154cae9c6829f9848e7b9786650e Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sat, 29 Jun 2019 11:18:42 +0200 Subject: [PATCH 1/4] Refactor TransformDisplayClassUsage into separate transform. Make display class detection pattern-based instead of name-based. Fixes #1554 --- .../CSharp/CSharpDecompiler.cs | 1 + .../ICSharpCode.Decompiler.csproj | 1 + .../IL/Transforms/DelegateConstruction.cs | 197 +-------------- ...ransformCollectionAndObjectInitializers.cs | 4 +- .../Transforms/TransformDisplayClassUsage.cs | 229 ++++++++++++++++++ .../IL/Transforms/TransformExpressionTrees.cs | 18 +- 6 files changed, 259 insertions(+), 191 deletions(-) create mode 100644 ICSharpCode.Decompiler/IL/Transforms/TransformDisplayClassUsage.cs diff --git a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs index 5863da05e..bb85153bf 100644 --- a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs +++ b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs @@ -156,6 +156,7 @@ namespace ICSharpCode.Decompiler.CSharp }, new ProxyCallReplacer(), new DelegateConstruction(), + new TransformDisplayClassUsage(), new HighLevelLoopTransform(), new ReduceNestingTransform(), new IntroduceDynamicTypeOnLocals(), diff --git a/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj b/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj index 8ab2fbd69..3fca3af6e 100644 --- a/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj +++ b/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj @@ -272,6 +272,7 @@ + diff --git a/ICSharpCode.Decompiler/IL/Transforms/DelegateConstruction.cs b/ICSharpCode.Decompiler/IL/Transforms/DelegateConstruction.cs index aa4148c33..021f34767 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/DelegateConstruction.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/DelegateConstruction.cs @@ -37,7 +37,6 @@ namespace ICSharpCode.Decompiler.IL.Transforms return; this.context = context; this.decompilationContext = new SimpleTypeResolveContext(function.Method); - var orphanedVariableInits = new List(); var targetsToReplace = new List(); var translatedDisplayClasses = new HashSet(); var cancellationToken = context.CancellationToken; @@ -50,48 +49,21 @@ namespace ICSharpCode.Decompiler.IL.Transforms if (instWithVar.Variable.Kind == VariableKind.Local) { instWithVar.Variable.Kind = VariableKind.DisplayClassLocal; } + var displayClassTypeDef = instWithVar.Variable.Type.GetDefinition(); + if (instWithVar.Variable.IsSingleDefinition && instWithVar.Variable.StoreInstructions.SingleOrDefault() is StLoc store) { + if (store.Value is NewObj newObj) { + instWithVar.Variable.CaptureScope = BlockContainer.FindClosestContainer(store); + } + } + if (displayClassTypeDef != null) + translatedDisplayClasses.Add(displayClassTypeDef); targetsToReplace.Add(instWithVar); } context.StepEndGroup(); } - if (inst.MatchStLoc(out ILVariable targetVariable, out ILInstruction value)) { - var newObj = value as NewObj; - // TODO : it is probably not a good idea to remove *all* display-classes - // is there a way to minimize the false-positives? - if (newObj != null && IsInSimpleDisplayClass(newObj.Method)) { - targetVariable.CaptureScope = BlockContainer.FindClosestContainer(inst); - targetsToReplace.Add((IInstructionWithVariableOperand)inst); - translatedDisplayClasses.Add(newObj.Method.DeclaringTypeDefinition); - } - } - } - foreach (var target in targetsToReplace.OrderByDescending(t => ((ILInstruction)t).StartILOffset)) { - context.Step($"TransformDisplayClassUsages {target.Variable}", (ILInstruction)target); - function.AcceptVisitor(new TransformDisplayClassUsages(function, target, target.Variable.CaptureScope, orphanedVariableInits, translatedDisplayClasses)); - } - context.Step($"Remove orphanedVariableInits", function); - foreach (var store in orphanedVariableInits) { - if (store.Parent is Block containingBlock) - containingBlock.Instructions.Remove(store); } } - static bool IsInSimpleDisplayClass(IMethod method) - { - if (!method.IsCompilerGeneratedOrIsInCompilerGeneratedClass()) - return false; - return IsSimpleDisplayClass(method.DeclaringType); - } - - internal static bool IsSimpleDisplayClass(IType type) - { - if (!type.HasGeneratedName() || (!type.Name.Contains("DisplayClass") && !type.Name.Contains("AnonStorey"))) - return false; - if (type.DirectBaseTypes.Any(t => !t.IsKnownType(KnownTypeCode.Object))) - return false; - return true; - } - #region TransformDelegateConstruction internal static bool IsDelegateConstruction(NewObj inst, bool allowTransformed = false) { @@ -101,12 +73,6 @@ namespace ICSharpCode.Decompiler.IL.Transforms return opCode == OpCode.LdFtn || opCode == OpCode.LdVirtFtn || (allowTransformed && opCode == OpCode.ILFunction); } - - internal static bool IsPotentialClosure(ILTransformContext context, NewObj inst) - { - var decompilationContext = new SimpleTypeResolveContext(context.Function.Method); - return IsPotentialClosure(decompilationContext.CurrentTypeDefinition, inst.Method.DeclaringTypeDefinition); - } static bool IsAnonymousMethod(ITypeDefinition decompiledTypeDefinition, IMethod method) { @@ -115,7 +81,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms if (!(method.HasGeneratedName() || method.Name.Contains("$") || method.IsCompilerGeneratedOrIsInCompilerGeneratedClass() - || IsPotentialClosure(decompiledTypeDefinition, method.DeclaringTypeDefinition) + || TransformDisplayClassUsage.IsPotentialClosure(decompiledTypeDefinition, method.DeclaringTypeDefinition) || ContainsAnonymousType(method))) return false; return true; @@ -131,18 +97,6 @@ namespace ICSharpCode.Decompiler.IL.Transforms } return false; } - - static bool IsPotentialClosure(ITypeDefinition decompiledTypeDefinition, ITypeDefinition potentialDisplayClass) - { - if (potentialDisplayClass == null || !potentialDisplayClass.IsCompilerGeneratedOrIsInCompilerGeneratedClass()) - return false; - while (potentialDisplayClass != decompiledTypeDefinition) { - potentialDisplayClass = potentialDisplayClass.DeclaringTypeDefinition; - if (potentialDisplayClass == null) - return false; - } - return true; - } internal static GenericContext? GenericContextFromTypeArguments(TypeParameterSubstitution subst) { @@ -259,139 +213,6 @@ namespace ICSharpCode.Decompiler.IL.Transforms base.VisitLdObj(inst); } } - - /// - /// 1. Stores to display class fields are replaced with stores to local variables (in some - /// cases existing variables are used; otherwise fresh variables are added to the - /// ILFunction-container) and all usages of those fields are replaced with the local variable. - /// (see initValues) - /// 2. Usages of the display class container (or any copy) are removed. - /// - class TransformDisplayClassUsages : ILVisitor - { - ILFunction currentFunction; - BlockContainer captureScope; - readonly IInstructionWithVariableOperand targetLoad; - readonly List targetAndCopies = new List(); - readonly List orphanedVariableInits; - readonly HashSet translatedDisplayClasses; - readonly Dictionary initValues = new Dictionary(); - - struct DisplayClassVariable - { - public ILVariable variable; - public ILInstruction value; - } - - public TransformDisplayClassUsages(ILFunction function, IInstructionWithVariableOperand targetLoad, BlockContainer captureScope, List orphanedVariableInits, HashSet translatedDisplayClasses) - { - this.currentFunction = function; - this.targetLoad = targetLoad; - this.captureScope = captureScope; - this.orphanedVariableInits = orphanedVariableInits; - this.translatedDisplayClasses = translatedDisplayClasses; - this.targetAndCopies.Add(targetLoad.Variable); - } - - protected override void Default(ILInstruction inst) - { - foreach (var child in inst.Children) { - child.AcceptVisitor(this); - } - } - - protected internal override void VisitStLoc(StLoc inst) - { - base.VisitStLoc(inst); - if (targetLoad is ILInstruction instruction && instruction.MatchLdThis()) - return; - if (inst.Variable == targetLoad.Variable) - orphanedVariableInits.Add(inst); - if (MatchesTargetOrCopyLoad(inst.Value)) { - targetAndCopies.Add(inst.Variable); - orphanedVariableInits.Add(inst); - } - } - - bool MatchesTargetOrCopyLoad(ILInstruction inst) - { - return targetAndCopies.Any(v => inst.MatchLdLoc(v)); - } - - protected internal override void VisitStObj(StObj inst) - { - base.VisitStObj(inst); - if (!inst.Target.MatchLdFlda(out ILInstruction target, out IField field) || !MatchesTargetOrCopyLoad(target) || target.MatchLdThis()) - return; - field = (IField)field.MemberDefinition; - ILInstruction value; - if (initValues.TryGetValue(field, out DisplayClassVariable info)) { - inst.ReplaceWith(new StLoc(info.variable, inst.Value).WithILRange(inst)); - } else { - if (inst.Value.MatchLdLoc(out var v) && v.Kind == VariableKind.Parameter && currentFunction == v.Function) { - // special case for parameters: remove copies of parameter values. - orphanedVariableInits.Add(inst); - value = inst.Value; - } else { - if (!translatedDisplayClasses.Contains(field.DeclaringTypeDefinition)) - return; - v = currentFunction.RegisterVariable(VariableKind.Local, field.Type, field.Name); - v.CaptureScope = captureScope; - inst.ReplaceWith(new StLoc(v, inst.Value).WithILRange(inst)); - value = new LdLoc(v); - } - initValues.Add(field, new DisplayClassVariable { value = value, variable = v }); - } - } - - protected internal override void VisitLdObj(LdObj inst) - { - base.VisitLdObj(inst); - if (!inst.Target.MatchLdFlda(out ILInstruction target, out IField field)) - return; - if (!initValues.TryGetValue((IField)field.MemberDefinition, out DisplayClassVariable info)) - return; - var replacement = info.value.Clone(); - replacement.SetILRange(inst); - inst.ReplaceWith(replacement); - } - - protected internal override void VisitLdFlda(LdFlda inst) - { - base.VisitLdFlda(inst); - if (inst.Target.MatchLdThis() && inst.Field.Name == "$this" - && inst.Field.MemberDefinition.ReflectionName.Contains("c__Iterator")) { - var variable = currentFunction.Variables.First((f) => f.Index == -1); - inst.ReplaceWith(new LdLoca(variable).WithILRange(inst)); - } - if (inst.Parent is LdObj || inst.Parent is StObj) - return; - if (!MatchesTargetOrCopyLoad(inst.Target)) - return; - var field = (IField)inst.Field.MemberDefinition; - if (!initValues.TryGetValue(field, out DisplayClassVariable info)) { - if (!translatedDisplayClasses.Contains(field.DeclaringTypeDefinition)) - return; - var v = currentFunction.RegisterVariable(VariableKind.Local, field.Type, field.Name); - v.CaptureScope = captureScope; - inst.ReplaceWith(new LdLoca(v).WithILRange(inst)); - var value = new LdLoc(v); - initValues.Add(field, new DisplayClassVariable { value = value, variable = v }); - } else if (info.value is LdLoc l) { - inst.ReplaceWith(new LdLoca(l.Variable).WithILRange(inst)); - } else { - Debug.Fail("LdFlda pattern not supported!"); - } - } - - protected internal override void VisitNumericCompoundAssign(NumericCompoundAssign inst) - { - base.VisitNumericCompoundAssign(inst); - if (inst.Target.MatchLdLoc(out var v)) { - inst.ReplaceWith(new StLoc(v, new BinaryNumericInstruction(inst.Operator, inst.Target, inst.Value, inst.CheckForOverflow, inst.Sign).WithILRange(inst))); - } - } - } #endregion } } diff --git a/ICSharpCode.Decompiler/IL/Transforms/TransformCollectionAndObjectInitializers.cs b/ICSharpCode.Decompiler/IL/Transforms/TransformCollectionAndObjectInitializers.cs index c982f5f35..14e939cfd 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/TransformCollectionAndObjectInitializers.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/TransformCollectionAndObjectInitializers.cs @@ -62,9 +62,9 @@ namespace ICSharpCode.Decompiler.IL.Transforms } // Do not try to transform display class usages or delegate construction. // DelegateConstruction transform cannot deal with this. - if (DelegateConstruction.IsSimpleDisplayClass(newObjInst.Method.DeclaringType)) + if (TransformDisplayClassUsage.IsSimpleDisplayClass(newObjInst.Method.DeclaringType)) return false; - if (DelegateConstruction.IsDelegateConstruction(newObjInst) || DelegateConstruction.IsPotentialClosure(context, newObjInst)) + if (DelegateConstruction.IsDelegateConstruction(newObjInst) || TransformDisplayClassUsage.IsPotentialClosure(context, newObjInst)) return false; // Cannot build a collection/object initializer attached to an AnonymousTypeCreateExpression:s // anon = new { A = 5 } { 3,4,5 } is invalid syntax. diff --git a/ICSharpCode.Decompiler/IL/Transforms/TransformDisplayClassUsage.cs b/ICSharpCode.Decompiler/IL/Transforms/TransformDisplayClassUsage.cs new file mode 100644 index 000000000..5e26c5a1b --- /dev/null +++ b/ICSharpCode.Decompiler/IL/Transforms/TransformDisplayClassUsage.cs @@ -0,0 +1,229 @@ +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; +using ICSharpCode.Decompiler.TypeSystem; + +namespace ICSharpCode.Decompiler.IL.Transforms +{ + /// + /// Transforms closure fields to local variables. + /// + /// This is a post-processing step of and . + /// + class TransformDisplayClassUsage : ILVisitor, IILTransform + { + class DisplayClass + { + public ILInstruction Initializer; + public ILVariable Variable; + public ITypeDefinition Definition; + public Dictionary Variables; + public BlockContainer CaptureScope; + } + + struct DisplayClassVariable + { + public ILVariable Variable; + public ILInstruction Value; + } + + ILTransformContext context; + ILFunction currentFunction; + readonly Dictionary displayClasses = new Dictionary(); + readonly List orphanedVariableInits = new List(); + + public void Run(ILFunction function, ILTransformContext context) + { + try { + if (this.context != null || this.currentFunction != null) + throw new InvalidOperationException("Reentrancy in " + nameof(TransformDisplayClassUsage)); + this.context = context; + // Traverse nested functions in post-order: + // Inner functions are transformed before outer functions + foreach (var f in function.Descendants.OfType()) { + foreach (var v in f.Variables) { + if (!(v.IsSingleDefinition && v.StoreInstructions.SingleOrDefault() is StLoc inst)) + continue; + if (IsClosureInit(inst, out var closureType)) { + displayClasses.Add(inst.Variable, new DisplayClass { + Initializer = inst, + Variable = v, + Definition = closureType, + Variables = new Dictionary(), + CaptureScope = inst.Variable.CaptureScope + }); + orphanedVariableInits.Add(inst); + } + } + foreach (var displayClass in displayClasses.Values.OrderByDescending(d => d.Initializer.StartILOffset).ToArray()) { + context.Step($"Transform references to " + displayClass.Variable, displayClass.Initializer); + this.currentFunction = f; + VisitILFunction(f); + } + } + context.Step($"Remove orphanedVariableInits", function); + foreach (var store in orphanedVariableInits) { + if (store.Parent is Block containingBlock) + containingBlock.Instructions.Remove(store); + } + } finally { + orphanedVariableInits.Clear(); + displayClasses.Clear(); + this.context = null; + this.currentFunction = null; + } + } + + internal static bool IsSimpleDisplayClass(IType type) + { + if (!type.HasGeneratedName() || (!type.Name.Contains("DisplayClass") && !type.Name.Contains("AnonStorey"))) + return false; + if (type.DirectBaseTypes.Any(t => !t.IsKnownType(KnownTypeCode.Object))) + return false; + return true; + } + + internal static bool IsPotentialClosure(ILTransformContext context, NewObj inst) + { + var decompilationContext = new SimpleTypeResolveContext(context.Function.Method); + return IsPotentialClosure(decompilationContext.CurrentTypeDefinition, inst.Method.DeclaringTypeDefinition); + } + + internal static bool IsPotentialClosure(ITypeDefinition decompiledTypeDefinition, ITypeDefinition potentialDisplayClass) + { + if (potentialDisplayClass == null || !potentialDisplayClass.IsCompilerGeneratedOrIsInCompilerGeneratedClass()) + return false; + while (potentialDisplayClass != decompiledTypeDefinition) { + potentialDisplayClass = potentialDisplayClass.DeclaringTypeDefinition; + if (potentialDisplayClass == null) + return false; + } + return true; + } + + protected override void Default(ILInstruction inst) + { + foreach (var child in inst.Children) { + child.AcceptVisitor(this); + } + } + + protected internal override void VisitStLoc(StLoc inst) + { + base.VisitStLoc(inst); + // Sometimes display class references are copied into other local variables. + // We remove the assignment and store the relationship between the display class and the variable in the + // displayClasses dictionary. + if (inst.Value.MatchLdLoc(out var closureVariable) && displayClasses.TryGetValue(closureVariable, out var displayClass)) { + displayClasses[inst.Variable] = displayClass; + orphanedVariableInits.Add(inst); + } + } + + bool IsClosureInit(StLoc inst, out ITypeDefinition closureType) + { + closureType = null; + if (!(inst.Value is NewObj newObj)) + return false; + closureType = newObj.Method.DeclaringTypeDefinition; + return closureType != null && IsPotentialClosure(this.context, newObj); + } + + protected internal override void VisitStObj(StObj inst) + { + base.VisitStObj(inst); + // This instruction has been marked deletable, do not transform it further + if (orphanedVariableInits.Contains(inst)) + return; + // The target of the store instruction must be a field reference + if (!inst.Target.MatchLdFlda(out ILInstruction target, out IField field)) + return; + // Skip assignments that reference fields of the outer class, this is not a closure assignment. + if (target.MatchLdThis()) + return; + // Get display class info + if (!(target is LdLoc displayClassLoad && displayClasses.TryGetValue(displayClassLoad.Variable, out var displayClass))) + return; + field = (IField)field.MemberDefinition; + if (displayClass.Variables.TryGetValue(field, out DisplayClassVariable info)) { + // If the display class field was previously initialized, we use a simple assignment. + inst.ReplaceWith(new StLoc(info.Variable, inst.Value).WithILRange(inst)); + } else { + // This is an uninitialized variable: + ILInstruction value; + if (inst.Value.MatchLdLoc(out var v) && v.Kind == VariableKind.Parameter && currentFunction == v.Function) { + // Special case for parameters: remove copies of parameter values. + orphanedVariableInits.Add(inst); + value = inst.Value; + } else { + Debug.Assert(displayClass.Definition == field.DeclaringTypeDefinition); + // Introduce a fresh variable for the display class field. + v = currentFunction.RegisterVariable(VariableKind.Local, field.Type, field.Name); + v.CaptureScope = displayClass.CaptureScope; + inst.ReplaceWith(new StLoc(v, inst.Value).WithILRange(inst)); + value = new LdLoc(v); + } + displayClass.Variables.Add(field, new DisplayClassVariable { Value = value, Variable = v }); + } + } + + protected internal override void VisitLdObj(LdObj inst) + { + base.VisitLdObj(inst); + // The target of the store instruction must be a field reference + if (!inst.Target.MatchLdFlda(out var target, out IField field)) + return; + // Get display class info + if (!(target is LdLoc displayClassLoad && displayClasses.TryGetValue(displayClassLoad.Variable, out var displayClass))) + return; + // Get display class variable info + if (!displayClass.Variables.TryGetValue((IField)field.MemberDefinition, out DisplayClassVariable info)) + return; + // Replace usage of display class field with the variable. + var replacement = info.Value.Clone(); + replacement.SetILRange(inst); + inst.ReplaceWith(replacement); + } + + protected internal override void VisitLdFlda(LdFlda inst) + { + base.VisitLdFlda(inst); + // TODO : Figure out why this was added in https://github.com/icsharpcode/ILSpy/pull/1303 + if (inst.Target.MatchLdThis() && inst.Field.Name == "$this" + && inst.Field.MemberDefinition.ReflectionName.Contains("c__Iterator")) { + var variable = currentFunction.Variables.First((f) => f.Index == -1); + inst.ReplaceWith(new LdLoca(variable).WithILRange(inst)); + } + // Skip stfld/ldfld + if (inst.Parent is LdObj || inst.Parent is StObj) + return; + // Get display class info + if (!(inst.Target is LdLoc displayClassLoad && displayClasses.TryGetValue(displayClassLoad.Variable, out var displayClass))) + return; + var field = (IField)inst.Field.MemberDefinition; + if (!displayClass.Variables.TryGetValue(field, out DisplayClassVariable info)) { + // Introduce a fresh variable for the display class field. + Debug.Assert(displayClass.Definition == field.DeclaringTypeDefinition); + var v = currentFunction.RegisterVariable(VariableKind.Local, field.Type, field.Name); + v.CaptureScope = displayClass.CaptureScope; + inst.ReplaceWith(new LdLoca(v).WithILRange(inst)); + displayClass.Variables.Add(field, new DisplayClassVariable { Value = new LdLoc(v), Variable = v }); + } else if (info.Value is LdLoc l) { + inst.ReplaceWith(new LdLoca(l.Variable).WithILRange(inst)); + } else { + Debug.Fail("LdFlda pattern not supported!"); + } + } + + protected internal override void VisitNumericCompoundAssign(NumericCompoundAssign inst) + { + base.VisitNumericCompoundAssign(inst); + // NumericCompoundAssign is only valid when used with fields: -> replace it with a BinaryNumericInstruction. + if (inst.Target.MatchLdLoc(out var v)) { + inst.ReplaceWith(new StLoc(v, new BinaryNumericInstruction(inst.Operator, inst.Target, inst.Value, inst.CheckForOverflow, inst.Sign).WithILRange(inst))); + } + } + + } +} diff --git a/ICSharpCode.Decompiler/IL/Transforms/TransformExpressionTrees.cs b/ICSharpCode.Decompiler/IL/Transforms/TransformExpressionTrees.cs index 89c92bb69..b3ac3e596 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/TransformExpressionTrees.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/TransformExpressionTrees.cs @@ -21,7 +21,6 @@ using System.Collections.Generic; using System.Diagnostics; using System.Linq; using ICSharpCode.Decompiler.CSharp.Resolver; -using ICSharpCode.Decompiler.CSharp.Syntax; using ICSharpCode.Decompiler.Semantics; using ICSharpCode.Decompiler.TypeSystem; using ICSharpCode.Decompiler.TypeSystem.Implementation; @@ -1071,6 +1070,14 @@ namespace ICSharpCode.Decompiler.IL.Transforms && v.StackType.IsIntegerType()) return new LdLoca(v); return null; + } else if (IsClosureReference(ldloc.Variable)) { + if (ldloc.Variable.Kind == VariableKind.Local) { + ldloc.Variable.Kind = VariableKind.DisplayClassLocal; + } + if (ldloc.Variable.CaptureScope == null) { + ldloc.Variable.CaptureScope = BlockContainer.FindClosestContainer(context); + } + return ldloc; } else { return ldloc; } @@ -1079,6 +1086,15 @@ namespace ICSharpCode.Decompiler.IL.Transforms } } + bool IsClosureReference(ILVariable variable) + { + if (!variable.IsSingleDefinition || !(variable.StoreInstructions.SingleOrDefault() is StLoc store)) + return false; + if (!(store.Value is NewObj newObj)) + return false; + return TransformDisplayClassUsage.IsPotentialClosure(this.context, newObj); + } + bool IsExpressionTreeParameter(ILVariable variable) { return variable.Type.FullName == "System.Linq.Expressions.ParameterExpression"; From 7f8856b10fb5d29a3d6450ee2cb014824f4d0d9e Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sun, 7 Jul 2019 10:57:04 +0200 Subject: [PATCH 2/4] Fix #1026: Improve support for mcs in TransformDisplayClassUsage --- .../PrettyTestRunner.cs | 2 +- ICSharpCode.Decompiler/IL/ILVariable.cs | 5 + .../Transforms/TransformDisplayClassUsage.cs | 100 +++++++++++++++--- 3 files changed, 92 insertions(+), 15 deletions(-) diff --git a/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs b/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs index 1e15135f1..a8b4b980b 100644 --- a/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs +++ b/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs @@ -134,7 +134,7 @@ namespace ICSharpCode.Decompiler.Tests } [Test] - public void DelegateConstruction([ValueSource(nameof(defaultOptions))] CompilerOptions cscOptions) + public void DelegateConstruction([ValueSource(nameof(defaultOptionsWithMcs))] CompilerOptions cscOptions) { RunForLibrary(cscOptions: cscOptions); } diff --git a/ICSharpCode.Decompiler/IL/ILVariable.cs b/ICSharpCode.Decompiler/IL/ILVariable.cs index a81f355e0..8a0e5fd17 100644 --- a/ICSharpCode.Decompiler/IL/ILVariable.cs +++ b/ICSharpCode.Decompiler/IL/ILVariable.cs @@ -74,6 +74,11 @@ namespace ICSharpCode.Decompiler.IL static class VariableKindExtensions { + public static bool IsThis(this ILVariable v) + { + return v.Kind == VariableKind.Parameter && v.Index < 0; + } + public static bool IsLocal(this VariableKind kind) { switch (kind) { diff --git a/ICSharpCode.Decompiler/IL/Transforms/TransformDisplayClassUsage.cs b/ICSharpCode.Decompiler/IL/Transforms/TransformDisplayClassUsage.cs index 5e26c5a1b..23b8d4467 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/TransformDisplayClassUsage.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/TransformDisplayClassUsage.cs @@ -15,6 +15,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms { class DisplayClass { + public bool IsMono; public ILInstruction Initializer; public ILVariable Variable; public ITypeDefinition Definition; @@ -31,7 +32,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms ILTransformContext context; ILFunction currentFunction; readonly Dictionary displayClasses = new Dictionary(); - readonly List orphanedVariableInits = new List(); + readonly List instructionsToRemove = new List(); public void Run(ILFunction function, ILTransformContext context) { @@ -39,21 +40,27 @@ namespace ICSharpCode.Decompiler.IL.Transforms if (this.context != null || this.currentFunction != null) throw new InvalidOperationException("Reentrancy in " + nameof(TransformDisplayClassUsage)); this.context = context; + var decompilationContext = new SimpleTypeResolveContext(context.Function.Method); // Traverse nested functions in post-order: // Inner functions are transformed before outer functions foreach (var f in function.Descendants.OfType()) { - foreach (var v in f.Variables) { + foreach (var v in f.Variables.ToArray()) { + if (HandleMonoStateMachine(function, v, decompilationContext, f)) + continue; if (!(v.IsSingleDefinition && v.StoreInstructions.SingleOrDefault() is StLoc inst)) continue; - if (IsClosureInit(inst, out var closureType)) { + if (IsClosureInit(inst, out ITypeDefinition closureType)) { + // TODO : figure out whether it is a mono compiled closure, without relying on the type name + bool isMono = f.StateMachineCompiledWithMono || closureType.Name.Contains("AnonStorey"); displayClasses.Add(inst.Variable, new DisplayClass { + IsMono = isMono, Initializer = inst, Variable = v, Definition = closureType, Variables = new Dictionary(), - CaptureScope = inst.Variable.CaptureScope + CaptureScope = isMono && IsMonoNestedCaptureScope(closureType) ? null : inst.Variable.CaptureScope }); - orphanedVariableInits.Add(inst); + instructionsToRemove.Add(inst); } } foreach (var displayClass in displayClasses.Values.OrderByDescending(d => d.Initializer.StartILOffset).ToArray()) { @@ -62,19 +69,83 @@ namespace ICSharpCode.Decompiler.IL.Transforms VisitILFunction(f); } } - context.Step($"Remove orphanedVariableInits", function); - foreach (var store in orphanedVariableInits) { + context.Step($"Remove instructions", function); + foreach (var store in instructionsToRemove) { if (store.Parent is Block containingBlock) containingBlock.Instructions.Remove(store); } } finally { - orphanedVariableInits.Clear(); + instructionsToRemove.Clear(); displayClasses.Clear(); this.context = null; this.currentFunction = null; } } + bool IsOuterClosureReference(IField field) + { + return displayClasses.Values.Any(disp => disp.Definition == field.DeclaringTypeDefinition); + } + + bool IsMonoNestedCaptureScope(ITypeDefinition closureType) + { + var decompilationContext = new SimpleTypeResolveContext(context.Function.Method); + return closureType.Fields.Any(f => IsPotentialClosure(decompilationContext.CurrentTypeDefinition, f.ReturnType.GetDefinition())); + } + + /// + /// mcs likes to optimize closures in yield state machines away by moving the captured variables' fields into the state machine type, + /// We construct a that spans the whole method body. + /// + bool HandleMonoStateMachine(ILFunction currentFunction, ILVariable thisVariable, SimpleTypeResolveContext decompilationContext, ILFunction nestedFunction) + { + if (!(nestedFunction.StateMachineCompiledWithMono && thisVariable.IsThis())) + return false; + // Special case for Mono-compiled yield state machines + ITypeDefinition closureType = thisVariable.Type.GetDefinition(); + if (!(closureType != decompilationContext.CurrentTypeDefinition + && IsPotentialClosure(decompilationContext.CurrentTypeDefinition, closureType))) + return false; + + var displayClass = new DisplayClass { + IsMono = true, + Initializer = nestedFunction.Body, + Variable = thisVariable, + Definition = thisVariable.Type.GetDefinition(), + Variables = new Dictionary(), + CaptureScope = (BlockContainer)nestedFunction.Body + }; + displayClasses.Add(thisVariable, displayClass); + foreach (var stateMachineVariable in nestedFunction.Variables) { + if (stateMachineVariable.StateMachineField == null) + continue; + displayClass.Variables.Add(stateMachineVariable.StateMachineField, new DisplayClassVariable { + Variable = stateMachineVariable, + Value = new LdLoc(stateMachineVariable) + }); + } + if (!currentFunction.Method.IsStatic && FindThisField(out var thisField)) { + var thisVar = currentFunction.Variables + .FirstOrDefault(t => t.IsThis() && t.Type.GetDefinition() == decompilationContext.CurrentTypeDefinition); + if (thisVar == null) { + thisVar = new ILVariable(VariableKind.Parameter, decompilationContext.CurrentTypeDefinition, -1) { Name = "this" }; + currentFunction.Variables.Add(thisVar); + } + displayClass.Variables.Add(thisField, new DisplayClassVariable { Variable = thisVar, Value = new LdLoc(thisVar) }); + } + return true; + + bool FindThisField(out IField foundField) + { + foundField = null; + foreach (var field in closureType.GetFields(f2 => !f2.IsStatic && !displayClass.Variables.ContainsKey(f2) && f2.Type.GetDefinition() == decompilationContext.CurrentTypeDefinition)) { + thisField = field; + return true; + } + return false; + } + } + internal static bool IsSimpleDisplayClass(IType type) { if (!type.HasGeneratedName() || (!type.Name.Contains("DisplayClass") && !type.Name.Contains("AnonStorey"))) @@ -117,7 +188,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms // displayClasses dictionary. if (inst.Value.MatchLdLoc(out var closureVariable) && displayClasses.TryGetValue(closureVariable, out var displayClass)) { displayClasses[inst.Variable] = displayClass; - orphanedVariableInits.Add(inst); + instructionsToRemove.Add(inst); } } @@ -134,14 +205,11 @@ namespace ICSharpCode.Decompiler.IL.Transforms { base.VisitStObj(inst); // This instruction has been marked deletable, do not transform it further - if (orphanedVariableInits.Contains(inst)) + if (instructionsToRemove.Contains(inst)) return; // The target of the store instruction must be a field reference if (!inst.Target.MatchLdFlda(out ILInstruction target, out IField field)) return; - // Skip assignments that reference fields of the outer class, this is not a closure assignment. - if (target.MatchLdThis()) - return; // Get display class info if (!(target is LdLoc displayClassLoad && displayClasses.TryGetValue(displayClassLoad.Variable, out var displayClass))) return; @@ -154,12 +222,15 @@ namespace ICSharpCode.Decompiler.IL.Transforms ILInstruction value; if (inst.Value.MatchLdLoc(out var v) && v.Kind == VariableKind.Parameter && currentFunction == v.Function) { // Special case for parameters: remove copies of parameter values. - orphanedVariableInits.Add(inst); + instructionsToRemove.Add(inst); value = inst.Value; } else { Debug.Assert(displayClass.Definition == field.DeclaringTypeDefinition); // Introduce a fresh variable for the display class field. v = currentFunction.RegisterVariable(VariableKind.Local, field.Type, field.Name); + if (displayClass.IsMono && displayClass.CaptureScope == null && !IsOuterClosureReference(field)) { + displayClass.CaptureScope = BlockContainer.FindClosestContainer(inst); + } v.CaptureScope = displayClass.CaptureScope; inst.ReplaceWith(new StLoc(v, inst.Value).WithILRange(inst)); value = new LdLoc(v); @@ -192,6 +263,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms // TODO : Figure out why this was added in https://github.com/icsharpcode/ILSpy/pull/1303 if (inst.Target.MatchLdThis() && inst.Field.Name == "$this" && inst.Field.MemberDefinition.ReflectionName.Contains("c__Iterator")) { + //Debug.Assert(false, "This should not be executed!"); var variable = currentFunction.Variables.First((f) => f.Index == -1); inst.ReplaceWith(new LdLoca(variable).WithILRange(inst)); } From c1ac461c267d1b7da38c1ad1b52d707b055d063b Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sun, 7 Jul 2019 11:51:51 +0200 Subject: [PATCH 3/4] Enable MCS yield return pretty tests. --- ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs | 2 +- .../IL/Transforms/RemoveDeadVariableInit.cs | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs b/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs index a8b4b980b..6c66f55c3 100644 --- a/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs +++ b/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs @@ -420,7 +420,7 @@ namespace ICSharpCode.Decompiler.Tests } [Test] - public void YieldReturn([ValueSource(nameof(defaultOptions))] CompilerOptions cscOptions) + public void YieldReturn([ValueSource(nameof(defaultOptionsWithMcs))] CompilerOptions cscOptions) { RunForLibrary(cscOptions: cscOptions); } diff --git a/ICSharpCode.Decompiler/IL/Transforms/RemoveDeadVariableInit.cs b/ICSharpCode.Decompiler/IL/Transforms/RemoveDeadVariableInit.cs index ff8f08ac4..a6dab356b 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/RemoveDeadVariableInit.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/RemoveDeadVariableInit.cs @@ -53,6 +53,10 @@ namespace ICSharpCode.Decompiler.IL.Transforms var v = variableQueue.Dequeue(); if (v.Kind != VariableKind.Local && v.Kind != VariableKind.StackSlot) continue; + // Skip variables that are captured in a mcs yield state-machine + // loads of these will only be visible after DelegateConstruction step. + if (function.StateMachineCompiledWithMono && v.StateMachineField != null) + continue; if (v.LoadCount != 0 || v.AddressCount != 0) continue; foreach (var stloc in v.StoreInstructions.OfType().ToArray()) { From b4a59ae4ddd3c66ba69768885031bb7b564942ef Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sun, 7 Jul 2019 12:18:06 +0200 Subject: [PATCH 4/4] Remove obsolete code from DelegateConstruction.cs --- .../IL/Transforms/DelegateConstruction.cs | 9 --------- .../Transforms/TransformDisplayClassUsage.cs | 20 ++++++++++++++++++- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/ICSharpCode.Decompiler/IL/Transforms/DelegateConstruction.cs b/ICSharpCode.Decompiler/IL/Transforms/DelegateConstruction.cs index 021f34767..ce19a3e84 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/DelegateConstruction.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/DelegateConstruction.cs @@ -16,9 +16,7 @@ // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER // DEALINGS IN THE SOFTWARE. -using System; using System.Collections.Generic; -using System.Diagnostics; using System.Linq; using System.Reflection.Metadata; using ICSharpCode.Decompiler.CSharp; @@ -37,8 +35,6 @@ namespace ICSharpCode.Decompiler.IL.Transforms return; this.context = context; this.decompilationContext = new SimpleTypeResolveContext(function.Method); - var targetsToReplace = new List(); - var translatedDisplayClasses = new HashSet(); var cancellationToken = context.CancellationToken; foreach (var inst in function.Descendants) { cancellationToken.ThrowIfCancellationRequested(); @@ -55,16 +51,12 @@ namespace ICSharpCode.Decompiler.IL.Transforms instWithVar.Variable.CaptureScope = BlockContainer.FindClosestContainer(store); } } - if (displayClassTypeDef != null) - translatedDisplayClasses.Add(displayClassTypeDef); - targetsToReplace.Add(instWithVar); } context.StepEndGroup(); } } } - #region TransformDelegateConstruction internal static bool IsDelegateConstruction(NewObj inst, bool allowTransformed = false) { if (inst == null || inst.Arguments.Count != 2 || inst.Method.DeclaringType.Kind != TypeKind.Delegate) @@ -213,6 +205,5 @@ namespace ICSharpCode.Decompiler.IL.Transforms base.VisitLdObj(inst); } } - #endregion } } diff --git a/ICSharpCode.Decompiler/IL/Transforms/TransformDisplayClassUsage.cs b/ICSharpCode.Decompiler/IL/Transforms/TransformDisplayClassUsage.cs index 23b8d4467..887f345d7 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/TransformDisplayClassUsage.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/TransformDisplayClassUsage.cs @@ -1,4 +1,22 @@ -using System; +// Copyright (c) 2019 Siegfried Pammer +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +using System; using System.Collections.Generic; using System.Diagnostics; using System.Linq;