From fcc910cb4d900fdf69708a8e572a46557beaea43 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Fri, 5 Jun 2020 13:01:02 +0200 Subject: [PATCH] Cleanup and refactoring after code review --- .../ICSharpCode.Decompiler.Tests.csproj | 2 + ...eScalarReplacementOfAggregates.Expected.cs | 10 + ...AggressiveScalarReplacementOfAggregates.cs | 12 + ...calarReplacementOfAggregates.opt.roslyn.il | 33 ++ ...iveScalarReplacementOfAggregates.roslyn.il | 41 +++ .../IL/Transforms/DelegateConstruction.cs | 2 +- .../IL/Transforms/LocalFunctionDecompiler.cs | 74 ++-- .../Transforms/TransformDisplayClassUsage.cs | 328 ++++++++++-------- 8 files changed, 320 insertions(+), 182 deletions(-) diff --git a/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj b/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj index b409376e0..6eff9af2c 100644 --- a/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj +++ b/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj @@ -90,6 +90,8 @@ + + diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Ugly/AggressiveScalarReplacementOfAggregates.Expected.cs b/ICSharpCode.Decompiler.Tests/TestCases/Ugly/AggressiveScalarReplacementOfAggregates.Expected.cs index c75aa3f39..8d40f3def 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Ugly/AggressiveScalarReplacementOfAggregates.Expected.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Ugly/AggressiveScalarReplacementOfAggregates.Expected.cs @@ -110,5 +110,15 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Ugly Console.WriteLine(field3); } } + + public void Test6(int i) + { + int field1 = i; + string field2 = "Hello World!"; + if (i < 0) { + i = -i; + } + Console.WriteLine("{0} {1}", field1, field2); + } } } diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Ugly/AggressiveScalarReplacementOfAggregates.cs b/ICSharpCode.Decompiler.Tests/TestCases/Ugly/AggressiveScalarReplacementOfAggregates.cs index e541b6c1d..5f3ed22a0 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Ugly/AggressiveScalarReplacementOfAggregates.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Ugly/AggressiveScalarReplacementOfAggregates.cs @@ -114,5 +114,17 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Ugly } } } + + public void Test6(int i) + { + DisplayClass displayClass = new DisplayClass { + field1 = i, + field2 = "Hello World!" + }; + if (i < 0) { + i = -i; + } + Console.WriteLine("{0} {1}", displayClass.field1, displayClass.field2); + } } } diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Ugly/AggressiveScalarReplacementOfAggregates.opt.roslyn.il b/ICSharpCode.Decompiler.Tests/TestCases/Ugly/AggressiveScalarReplacementOfAggregates.opt.roslyn.il index ce031ab73..d411ad3d4 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Ugly/AggressiveScalarReplacementOfAggregates.opt.roslyn.il +++ b/ICSharpCode.Decompiler.Tests/TestCases/Ugly/AggressiveScalarReplacementOfAggregates.opt.roslyn.il @@ -326,6 +326,39 @@ IL_0085: br.s IL_001a } // end of method Program::Issue1898 + .method public hidebysig instance void + Test6(int32 i) cil managed + { + // Code size 60 (0x3c) + .maxstack 3 + .locals init (class ICSharpCode.Decompiler.Tests.TestCases.Ugly.DisplayClass V_0) + IL_0000: newobj instance void ICSharpCode.Decompiler.Tests.TestCases.Ugly.DisplayClass::.ctor() + IL_0005: dup + IL_0006: ldarg.1 + IL_0007: stfld int32 ICSharpCode.Decompiler.Tests.TestCases.Ugly.DisplayClass::field1 + IL_000c: dup + IL_000d: ldstr "Hello World!" + IL_0012: stfld string ICSharpCode.Decompiler.Tests.TestCases.Ugly.DisplayClass::field2 + IL_0017: stloc.0 + IL_0018: ldarg.1 + IL_0019: ldc.i4.0 + IL_001a: bge.s IL_0020 + + IL_001c: ldarg.1 + IL_001d: neg + IL_001e: starg.s i + IL_0020: ldstr "{0} {1}" + IL_0025: ldloc.0 + IL_0026: ldfld int32 ICSharpCode.Decompiler.Tests.TestCases.Ugly.DisplayClass::field1 + IL_002b: box [mscorlib]System.Int32 + IL_0030: ldloc.0 + IL_0031: ldfld string ICSharpCode.Decompiler.Tests.TestCases.Ugly.DisplayClass::field2 + IL_0036: call void [mscorlib]System.Console::WriteLine(string, + object, + object) + IL_003b: ret + } // end of method Program::Test6 + .method public hidebysig specialname rtspecialname instance void .ctor() cil managed { diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Ugly/AggressiveScalarReplacementOfAggregates.roslyn.il b/ICSharpCode.Decompiler.Tests/TestCases/Ugly/AggressiveScalarReplacementOfAggregates.roslyn.il index e164ad291..133e78c71 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Ugly/AggressiveScalarReplacementOfAggregates.roslyn.il +++ b/ICSharpCode.Decompiler.Tests/TestCases/Ugly/AggressiveScalarReplacementOfAggregates.roslyn.il @@ -371,6 +371,47 @@ IL_0095: br.s IL_001d } // end of method Program::Issue1898 + .method public hidebysig instance void + Test6(int32 i) cil managed + { + // Code size 68 (0x44) + .maxstack 3 + .locals init (class ICSharpCode.Decompiler.Tests.TestCases.Ugly.DisplayClass V_0, + bool V_1) + IL_0000: nop + IL_0001: newobj instance void ICSharpCode.Decompiler.Tests.TestCases.Ugly.DisplayClass::.ctor() + IL_0006: dup + IL_0007: ldarg.1 + IL_0008: stfld int32 ICSharpCode.Decompiler.Tests.TestCases.Ugly.DisplayClass::field1 + IL_000d: dup + IL_000e: ldstr "Hello World!" + IL_0013: stfld string ICSharpCode.Decompiler.Tests.TestCases.Ugly.DisplayClass::field2 + IL_0018: stloc.0 + IL_0019: ldarg.1 + IL_001a: ldc.i4.0 + IL_001b: clt + IL_001d: stloc.1 + IL_001e: ldloc.1 + IL_001f: brfalse.s IL_0027 + + IL_0021: nop + IL_0022: ldarg.1 + IL_0023: neg + IL_0024: starg.s i + IL_0026: nop + IL_0027: ldstr "{0} {1}" + IL_002c: ldloc.0 + IL_002d: ldfld int32 ICSharpCode.Decompiler.Tests.TestCases.Ugly.DisplayClass::field1 + IL_0032: box [mscorlib]System.Int32 + IL_0037: ldloc.0 + IL_0038: ldfld string ICSharpCode.Decompiler.Tests.TestCases.Ugly.DisplayClass::field2 + IL_003d: call void [mscorlib]System.Console::WriteLine(string, + object, + object) + IL_0042: nop + IL_0043: ret + } // end of method Program::Test6 + .method public hidebysig specialname rtspecialname instance void .ctor() cil managed { diff --git a/ICSharpCode.Decompiler/IL/Transforms/DelegateConstruction.cs b/ICSharpCode.Decompiler/IL/Transforms/DelegateConstruction.cs index 78b3e678e..b5dc0423f 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/DelegateConstruction.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/DelegateConstruction.cs @@ -193,7 +193,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms // if the delegate target is ldobj(ldsflda field). if (ldobj.Target is LdsFlda) return true; - // TODO : ldfld chains must be validate more thoroughly, i.e., we should make sure + // TODO : ldfld chains must be validated more thoroughly, i.e., we should make sure // that the value of the field is never changed. ILInstruction target = ldobj; while (target is LdObj || target is LdFlda) { diff --git a/ICSharpCode.Decompiler/IL/Transforms/LocalFunctionDecompiler.cs b/ICSharpCode.Decompiler/IL/Transforms/LocalFunctionDecompiler.cs index b4dabbc70..1f59d6df5 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/LocalFunctionDecompiler.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/LocalFunctionDecompiler.cs @@ -46,6 +46,11 @@ namespace ICSharpCode.Decompiler.IL.Transforms public List UseSites; public IMethod Method; public ILFunction Definition; + /// + /// Used to store all synthesized call-site arguments grouped by the parameter index. + /// We use a dictionary instead of a simple array, because -1 is used for the this parameter + /// and there might be many non-synthesized arguments in between. + /// public Dictionary> LocalFunctionArguments; } @@ -81,13 +86,13 @@ namespace ICSharpCode.Decompiler.IL.Transforms var localFunctions = new Dictionary(); // Find all local functions declared inside this method, including nested local functions or local functions declared in lambdas. FindUseSites(function, context, localFunctions); - ReplaceReferencesToDisplayClassThis(function, localFunctions.Values, context); - DetermineCaptureAndDeclarationScopes(function, localFunctions.Values, context); - PropagateClosureParameterArguments(function, localFunctions, context); - TransformUseSites(localFunctions.Values, context); + ReplaceReferencesToDisplayClassThis(localFunctions.Values); + DetermineCaptureAndDeclarationScopes(localFunctions.Values); + PropagateClosureParameterArguments(localFunctions); + TransformUseSites(localFunctions.Values); } - private void ReplaceReferencesToDisplayClassThis(ILFunction function, Dictionary.ValueCollection localFunctions, ILTransformContext context) + private void ReplaceReferencesToDisplayClassThis(Dictionary.ValueCollection localFunctions) { foreach (var info in localFunctions) { var localFunction = info.Definition; @@ -96,15 +101,15 @@ namespace ICSharpCode.Decompiler.IL.Transforms var thisVar = localFunction.Variables.SingleOrDefault(VariableKindExtensions.IsThis); if (thisVar == null) continue; - var compatibleArgument = FindCompatibleArgument(function, info, info.UseSites.SelectArray(u => u.Arguments[0]), ignoreStructure: true); + var compatibleArgument = FindCompatibleArgument(info, info.UseSites.SelectArray(u => u.Arguments[0]), ignoreStructure: true); Debug.Assert(compatibleArgument != null); context.Step($"Replace 'this' with {compatibleArgument}", localFunction); localFunction.AcceptVisitor(new DelegateConstruction.ReplaceDelegateTargetVisitor(compatibleArgument, thisVar)); - DetermineCaptureAndDeclarationScope(function, info, -1, compatibleArgument); + DetermineCaptureAndDeclarationScope(info, -1, compatibleArgument); } } - private void DetermineCaptureAndDeclarationScopes(ILFunction function, Dictionary.ValueCollection localFunctions, ILTransformContext context) + private void DetermineCaptureAndDeclarationScopes(Dictionary.ValueCollection localFunctions) { foreach (var info in localFunctions) { context.CancellationToken.ThrowIfCancellationRequested(); @@ -118,28 +123,28 @@ namespace ICSharpCode.Decompiler.IL.Transforms var localFunction = info.Definition; foreach (var useSite in info.UseSites) { - DetermineCaptureAndDeclarationScope(function, info, useSite); + DetermineCaptureAndDeclarationScope(info, useSite); - if (function.Method.IsConstructor && localFunction.DeclarationScope == null) { + if (context.Function.Method.IsConstructor && localFunction.DeclarationScope == null) { localFunction.DeclarationScope = BlockContainer.FindClosestContainer(useSite); } } if (localFunction.DeclarationScope == null) { - localFunction.DeclarationScope = (BlockContainer)function.Body; + localFunction.DeclarationScope = (BlockContainer)context.Function.Body; } ILFunction declaringFunction = GetDeclaringFunction(localFunction); - if (declaringFunction != function) { - context.Step($"Move {localFunction.Name} from {function.Name} to {declaringFunction.Name}", localFunction); - function.LocalFunctions.Remove(localFunction); + if (declaringFunction != context.Function) { + context.Step($"Move {localFunction.Name} from {context.Function.Name} to {declaringFunction.Name}", localFunction); + context.Function.LocalFunctions.Remove(localFunction); declaringFunction.LocalFunctions.Add(localFunction); } if (TryValidateSkipCount(info, out int skipCount) && skipCount != localFunction.ReducedMethod.NumberOfCompilerGeneratedTypeParameters) { Debug.Assert(false); - function.Warnings.Add($"Could not decode local function '{info.Method}'"); - if (declaringFunction != function) { + context.Function.Warnings.Add($"Could not decode local function '{info.Method}'"); + if (declaringFunction != context.Function) { declaringFunction.LocalFunctions.Remove(localFunction); } } @@ -149,7 +154,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms } } - private void TransformUseSites(Dictionary.ValueCollection localFunctions, ILTransformContext context) + private void TransformUseSites(Dictionary.ValueCollection localFunctions) { foreach (var info in localFunctions) { context.CancellationToken.ThrowIfCancellationRequested(); @@ -170,9 +175,9 @@ namespace ICSharpCode.Decompiler.IL.Transforms } } - private void PropagateClosureParameterArguments(ILFunction function, Dictionary localFunctions, ILTransformContext context) + private void PropagateClosureParameterArguments(Dictionary localFunctions) { - foreach (var localFunction in function.Descendants.OfType()) { + foreach (var localFunction in context.Function.Descendants.OfType()) { if (localFunction.Kind != ILFunctionKind.LocalFunction) continue; context.CancellationToken.ThrowIfCancellationRequested(); @@ -202,7 +207,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms var targetVariable = info.Definition.Variables.SingleOrDefault(p => p.Kind == VariableKind.Parameter && p.Index == index); if (targetVariable == null) continue; - var compatibleArgument = FindCompatibleArgument(function, info, arguments); + var compatibleArgument = FindCompatibleArgument(info, arguments); Debug.Assert(compatibleArgument != null); context.Step($"Replace '{targetVariable}' with '{compatibleArgument}'", info.Definition); info.Definition.AcceptVisitor(new DelegateConstruction.ReplaceDelegateTargetVisitor(compatibleArgument, targetVariable)); @@ -236,27 +241,27 @@ namespace ICSharpCode.Decompiler.IL.Transforms } } - private ILInstruction FindCompatibleArgument(ILFunction function, LocalFunctionInfo info, IEnumerable arguments, bool ignoreStructure = false) + private ILInstruction FindCompatibleArgument(LocalFunctionInfo info, IList arguments, bool ignoreStructure = false) { foreach (var arg in arguments) { if (arg is IInstructionWithVariableOperand ld2 && (ignoreStructure || info.Definition.IsDescendantOf(ld2.Variable.Function))) return arg; - var v = ResolveAncestorScopeReference(function, arg); + var v = ResolveAncestorScopeReference(arg); if (v != null) return new LdLoc(v); } return null; } - private ILVariable ResolveAncestorScopeReference(ILFunction function, ILInstruction inst) + private ILVariable ResolveAncestorScopeReference(ILInstruction inst) { if (!inst.MatchLdFld(out var target, out var field)) return null; if (field.Type.Kind != TypeKind.Class) return null; - if (!(TransformDisplayClassUsage.IsPotentialClosure(context, field.Type.GetDefinition()) || function.Method.DeclaringType.Equals(field.Type))) + if (!(TransformDisplayClassUsage.IsPotentialClosure(context, field.Type.GetDefinition()) || context.Function.Method.DeclaringType.Equals(field.Type))) return null; - foreach (var v in function.Descendants.OfType().SelectMany(f => f.Variables)) { + foreach (var v in context.Function.Descendants.OfType().SelectMany(f => f.Variables)) { if (v.Kind != VariableKind.Local && v.Kind != VariableKind.DisplayClassLocal && v.Kind != VariableKind.StackSlot) { if (!(v.Kind == VariableKind.Parameter && v.Index == -1)) continue; @@ -482,7 +487,11 @@ namespace ICSharpCode.Decompiler.IL.Transforms static void TransformToLocalFunctionReference(ILFunction function, CallInstruction useSite) { - useSite.Arguments[0].ReplaceWith(new LdNull().WithILRange(useSite.Arguments[0])); + ILInstruction target = useSite.Arguments[0]; + target.ReplaceWith(new LdNull().WithILRange(target)); + if (target is IInstructionWithVariableOperand withVar && withVar.Variable.Kind == VariableKind.Local) { + withVar.Variable.Kind = VariableKind.DisplayClassLocal; + } var fnptr = (IInstructionWithMethodOperand)useSite.Arguments[1]; var specializeMethod = function.ReducedMethod.Specialize(fnptr.Method.Substitution); var replacement = new LdFtn(specializeMethod).WithILRange((ILInstruction)fnptr); @@ -518,19 +527,19 @@ namespace ICSharpCode.Decompiler.IL.Transforms useSite.ReplaceWith(replacement); } - void DetermineCaptureAndDeclarationScope(ILFunction function, LocalFunctionInfo info, CallInstruction useSite) + void DetermineCaptureAndDeclarationScope(LocalFunctionInfo info, CallInstruction useSite) { int firstArgumentIndex = info.Definition.Method.IsStatic ? 0 : 1; for (int i = useSite.Arguments.Count - 1; i >= firstArgumentIndex; i--) { - if (!DetermineCaptureAndDeclarationScope(function, info, i - firstArgumentIndex, useSite.Arguments[i])) + if (!DetermineCaptureAndDeclarationScope(info, i - firstArgumentIndex, useSite.Arguments[i])) break; } if (firstArgumentIndex > 0) { - DetermineCaptureAndDeclarationScope(function, info, -1, useSite.Arguments[0]); + DetermineCaptureAndDeclarationScope(info, -1, useSite.Arguments[0]); } } - bool DetermineCaptureAndDeclarationScope(ILFunction root, LocalFunctionInfo info, int parameterIndex, ILInstruction arg) + bool DetermineCaptureAndDeclarationScope(LocalFunctionInfo info, int parameterIndex, ILInstruction arg) { ILFunction function = info.Definition; ILVariable closureVar; @@ -539,7 +548,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms return false; } if (!(arg.MatchLdLoc(out closureVar) || arg.MatchLdLoca(out closureVar))) { - closureVar = ResolveAncestorScopeReference(root, arg); + closureVar = ResolveAncestorScopeReference(arg); if (closureVar == null) return false; } @@ -557,6 +566,9 @@ namespace ICSharpCode.Decompiler.IL.Transforms Debug.Assert(combinedScope != null); closureVar.CaptureScope = combinedScope; } + if (closureVar.Kind == VariableKind.Local) { + closureVar.Kind = VariableKind.DisplayClassLocal; + } if (function.DeclarationScope == null) function.DeclarationScope = closureVar.CaptureScope; else if (!IsInNestedLocalFunction(function.DeclarationScope, closureVar.CaptureScope.Ancestors.OfType().First())) diff --git a/ICSharpCode.Decompiler/IL/Transforms/TransformDisplayClassUsage.cs b/ICSharpCode.Decompiler/IL/Transforms/TransformDisplayClassUsage.cs index 6b4b1908d..d6e6bb357 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/TransformDisplayClassUsage.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/TransformDisplayClassUsage.cs @@ -104,7 +104,6 @@ namespace ICSharpCode.Decompiler.IL.Transforms ILTransformContext context; ITypeResolveContext decompilationContext; readonly Dictionary displayClasses = new Dictionary(); - readonly List instructionsToRemove = new List(); void IILTransform.Run(ILFunction function, ILTransformContext context) { @@ -122,31 +121,15 @@ namespace ICSharpCode.Decompiler.IL.Transforms void ClearState() { - instructionsToRemove.Clear(); displayClasses.Clear(); this.decompilationContext = null; this.context = null; } - public void Analyze(ILFunction function, ILTransformContext context) + void AnalyzeFunction(ILFunction function) { - ClearState(); - this.context = context; - this.decompilationContext = new SimpleTypeResolveContext(context.Function.Method); - AnalyzeFunction(function); - } - - public static bool AnalyzeVariable(ILVariable v, ILTransformContext context) - { - var tdcu = new TransformDisplayClassUsage(); - tdcu.context = context; - tdcu.decompilationContext = new SimpleTypeResolveContext(context.Function.Method); - return tdcu.AnalyzeVariable(v) != null; - } - - private void AnalyzeFunction(ILFunction function) - { - foreach (var f in function.Descendants.OfType()) { + void VisitFunction(ILFunction f) + { foreach (var v in f.Variables.ToArray()) { var result = AnalyzeVariable(v); if (result == null || displayClasses.ContainsKey(result.Variable)) @@ -156,6 +139,28 @@ namespace ICSharpCode.Decompiler.IL.Transforms } } + void VisitChildren(ILInstruction inst) + { + foreach (var child in inst.Children) { + Visit(child); + } + } + + void Visit(ILInstruction inst) + { + switch (inst) { + case ILFunction f: + VisitFunction(f); + VisitChildren(inst); + break; + default: + VisitChildren(inst); + break; + } + } + + Visit(function); + foreach (var (v, displayClass) in displayClasses.ToArray()) { if (!ValidateDisplayClassUses(v, displayClass)) displayClasses.Remove(v); @@ -172,8 +177,9 @@ namespace ICSharpCode.Decompiler.IL.Transforms } } - bool ValidateDisplayClassUses(ILVariable v, DisplayClass displayClass, bool readOnly = false) + bool ValidateDisplayClassUses(ILVariable v, DisplayClass displayClass) { + Debug.Assert(v == displayClass.Variable); foreach (var ldloc in v.LoadInstructions) { if (!ValidateUse(displayClass, ldloc)) return false; @@ -188,15 +194,13 @@ namespace ICSharpCode.Decompiler.IL.Transforms { IField field; switch (use.Parent) { - case LdFlda ldflda when ldflda.MatchLdFlda(out _, out field): + case LdFlda ldflda when ldflda.MatchLdFlda(out var target, out field) && target == use: var keyField = (IField)field.MemberDefinition; if (!container.VariablesToDeclare.TryGetValue(keyField, out VariableToDeclare variable) || variable == null) { - if (readOnly) - return false; variable = AddVariable(container, null, field); } container.VariablesToDeclare[keyField] = variable; - return variable != null; + return true; case StObj stobj when stobj.MatchStObj(out var target, out ILInstruction value, out _) && value == use: if (target.MatchLdFlda(out var load, out field) && load.MatchLdLocRef(out var otherVariable) && displayClasses.TryGetValue(otherVariable, out var otherDisplayClass)) { if (otherDisplayClass.VariablesToDeclare.TryGetValue((IField)field.MemberDefinition, out var declaredVar)) @@ -222,15 +226,8 @@ namespace ICSharpCode.Decompiler.IL.Transforms return DetectDisplayClass(v); case VariableKind.InitializerTarget: return DetectDisplayClassInitializer(v); - case VariableKind.PinnedLocal: - case VariableKind.UsingLocal: - case VariableKind.ForeachLocal: - case VariableKind.NamedArgument: - case VariableKind.ExceptionStackSlot: - case VariableKind.ExceptionLocal: - return null; default: - throw new ArgumentOutOfRangeException(); + return null; } } @@ -239,27 +236,19 @@ namespace ICSharpCode.Decompiler.IL.Transforms ITypeDefinition definition; if (v.Kind != VariableKind.StackSlot) { definition = v.Type.GetDefinition(); - } else if (v.StoreInstructions.Count > 0 && v.StoreInstructions[0] is StLoc stloc && stloc.Value is NewObj newObj) { - definition = newObj.Method.DeclaringTypeDefinition; + } else if (v.StoreInstructions.Count > 0 && v.StoreInstructions[0] is StLoc stloc) { + definition = stloc.Value.InferType(context.TypeSystem).GetDefinition(); } else { definition = null; } - if (definition == null) + if (!ValidateDisplayClassDefinition(definition)) return null; - if (!context.Settings.AggressiveScalarReplacementOfAggregates) { - if (definition.DeclaringTypeDefinition == null || definition.ParentModule.PEFile != context.PEFile) - return null; - if (!IsPotentialClosure(context, definition)) - return null; - } DisplayClass result; - Block initBlock; - int startIndex; switch (definition.Kind) { case TypeKind.Class: if (!v.IsSingleDefinition) return null; - if (!(v.StoreInstructions.FirstOrDefault() is StLoc stloc)) + if (!(v.StoreInstructions.SingleOrDefault() is StLoc stloc)) return null; if (stloc.Value is NewObj newObj && ValidateConstructor(newObj.Method)) { result = new DisplayClass(v, definition) { @@ -270,56 +259,59 @@ namespace ICSharpCode.Decompiler.IL.Transforms return null; } - initBlock = stloc.Parent as Block; - startIndex = stloc.ChildIndex + 1; + HandleInitBlock(stloc.Parent as Block, stloc.ChildIndex + 1, result, result.Variable); break; case TypeKind.Struct: if (!v.HasInitialValue) return null; if (v.StoreCount != 1) return null; + Debug.Assert(v.StoreInstructions.Count == 0); result = new DisplayClass(v, definition) { CaptureScope = v.CaptureScope }; - initBlock = FindDisplayStructInitBlock(v); - startIndex = 0; + HandleInitBlock(FindDisplayStructInitBlock(v), 0, result, result.Variable); break; default: return null; } - if (initBlock != null) { - for (int i = startIndex; i < initBlock.Instructions.Count; i++) { - var init = initBlock.Instructions[i]; - if (!init.MatchStFld(out var target, out var field, out var value)) - break; - if (!target.MatchLdLocRef(result.Variable)) - break; - if (result.VariablesToDeclare.ContainsKey((IField)field.MemberDefinition)) - break; - var variable = AddVariable(result, (StObj)init, field); - result.VariablesToDeclare[(IField)field.MemberDefinition] = variable; - } - } - if (IsMonoNestedCaptureScope(definition)) { result.CaptureScope = null; } return result; } + void HandleInitBlock(Block initBlock, int startIndex, DisplayClass result, ILVariable targetVariable) + { + if (initBlock == null) + return; + for (int i = startIndex; i < initBlock.Instructions.Count; i++) { + var init = initBlock.Instructions[i]; + if (!init.MatchStFld(out var target, out var field, out _)) + break; + if (!target.MatchLdLocRef(targetVariable)) + break; + if (result.VariablesToDeclare.ContainsKey((IField)field.MemberDefinition)) + break; + var variable = AddVariable(result, (StObj)init, field); + result.VariablesToDeclare[(IField)field.MemberDefinition] = variable; + } + } + private Block FindDisplayStructInitBlock(ILVariable v) { var root = v.Function.Body; return Visit(root)?.Ancestors.OfType().FirstOrDefault(); + // Try to find a common ancestor of all uses of the variable v. ILInstruction Visit(ILInstruction inst) { switch (inst) { - case LdLoc l: - return l.Variable == v ? l : null; - case StLoc s: - return s.Variable == v ? s : null; - case LdLoca la: - return la.Variable == v ? la : null; + case LdLoc l when l.Variable == v: + return l; + case StLoc s when s.Variable == v: + return s; + case LdLoca la when la.Variable == v: + return la; default: return VisitChildren(inst); } @@ -327,15 +319,20 @@ namespace ICSharpCode.Decompiler.IL.Transforms ILInstruction VisitChildren(ILInstruction inst) { + // Visit all children of the instruction ILInstruction result = null; foreach (var child in inst.Children) { var newResult = Visit(child); + // As soon as there is a second use of v in this sub-tree, + // we can skip all other children and just return this node. if (result == null) { result = newResult; } else if (newResult != null) { return inst; } } + // returns null, if v is not used in this sub-tree; + // returns a descendant of inst, if it is the only use of v in this sub-tree. return result; } } @@ -347,36 +344,39 @@ namespace ICSharpCode.Decompiler.IL.Transforms if (!(store.Value is NewObj newObj)) return null; var definition = newObj.Method.DeclaringType.GetDefinition(); - if (definition == null) + if (!ValidateDisplayClassDefinition(definition)) return null; - if (!context.Settings.AggressiveScalarReplacementOfAggregates) { - if (definition.DeclaringTypeDefinition == null || definition.ParentModule.PEFile != context.PEFile) - return null; - if (!IsPotentialClosure(context, definition)) - return null; - } if (!ValidateConstructor(newObj.Method)) return null; if (!initializerBlock.Parent.MatchStLoc(out var referenceVariable)) return null; + if (!referenceVariable.IsSingleDefinition) + return null; + if (!(referenceVariable.StoreInstructions.SingleOrDefault() is StLoc)) + return null; var result = new DisplayClass(referenceVariable, definition) { CaptureScope = referenceVariable.CaptureScope, Initializer = initializerBlock.Parent }; - for (int i = 1; i < initializerBlock.Instructions.Count; i++) { - var init = initializerBlock.Instructions[i]; - if (!init.MatchStFld(out var target, out var field, out _)) - break; - if (!target.MatchLdLocRef(v)) - break; - if (result.VariablesToDeclare.ContainsKey((IField)field.MemberDefinition)) - break; - var variable = AddVariable(result, (StObj)init, field); - result.VariablesToDeclare[(IField)field.MemberDefinition] = variable; - } + HandleInitBlock(initializerBlock, 1, result, v); return result; } + private bool ValidateDisplayClassDefinition(ITypeDefinition definition) + { + if (definition == null) + return false; + if (definition.ParentModule.PEFile != context.PEFile) + return false; + if (!context.Settings.AggressiveScalarReplacementOfAggregates) { + if (definition.DeclaringTypeDefinition == null) + return false; + if (!IsPotentialClosure(context, definition)) + return false; + } + return true; + } + private bool ValidateConstructor(IMethod method) { try { @@ -429,34 +429,55 @@ namespace ICSharpCode.Decompiler.IL.Transforms VariableToDeclare AddVariable(DisplayClass result, StObj statement, IField field) { VariableToDeclare variable = new VariableToDeclare(result, field); - if (statement != null - && statement.Value.MatchLdLoc(out var variableToPropagate) - && variableToPropagate.StateMachineField == null - && (variableToPropagate.Kind == VariableKind.StackSlot || variableToPropagate.Type.Equals(field.Type))) - { - variable.Propagate(variableToPropagate); + if (statement != null) { + variable.Propagate(ResolveVariableToPropagate(statement.Value, field.Type)); variable.Initializers.Add(statement); } return variable; } - private void Transform(ILFunction function) + /// + /// Resolves references to variables that can be propagated. + /// If a value does not match either LdLoc or a LdObj LdLdFlda* LdLoc chain, null is returned. + /// The if any of the variables/fields in the chain cannot be propagated, null is returned. + /// + ILVariable ResolveVariableToPropagate(ILInstruction value, IType expectedType = null) { - VisitILFunction(function); - foreach (var displayClass in displayClasses.Values) { - foreach (var v in displayClass.VariablesToDeclare.Values) { - if (v.CanPropagate && v.Initializers.Count == 1) { - instructionsToRemove.Add(v.Initializers.First()); + ILVariable v; + switch (value) { + case LdLoc load when load.Variable.StateMachineField == null: + v = load.Variable; + // If the variable is a parameter and it is used elsewhere, we cannot propagate it. + if (v.Kind == VariableKind.Parameter && v.Index >= 0 && v.LoadCount != 1) + return null; + if (!(expectedType == null || v.Kind == VariableKind.StackSlot || v.Type.Equals(expectedType))) + return null; + return v; + case LdObj ldfld: + DisplayClass currentDisplayClass = null; + foreach (var item in ldfld.Target.Descendants) { + if (IsDisplayClassLoad(item, out v)) { + if (!displayClasses.TryGetValue(v, out currentDisplayClass)) + return null; + } + if (currentDisplayClass == null) + return null; + if (item is LdFlda ldf && currentDisplayClass.VariablesToDeclare.TryGetValue((IField)ldf.Field.MemberDefinition, out var vd)) { + if (!vd.CanPropagate) + return null; + if (!displayClasses.TryGetValue(vd.GetOrDeclare(), out currentDisplayClass)) + return null; + } } - } - } - if (instructionsToRemove.Count > 0) { - context.Step($"Remove instructions", function); - foreach (var store in instructionsToRemove) { - if (store.Parent is Block containingBlock) - containingBlock.Instructions.Remove(store); - } + return currentDisplayClass.Variable; + default: + return null; } + } + + private void Transform(ILFunction function) + { + VisitILFunction(function); context.Step($"ResetHasInitialValueFlag", function); foreach (var f in TreeTraversal.PostOrder(function, f => f.LocalFunctions)) { RemoveDeadVariableInit.ResetHasInitialValueFlag(f, context); @@ -584,17 +605,16 @@ namespace ICSharpCode.Decompiler.IL.Transforms return true; } - ILFunction currentFunction; + readonly Stack currentFunctions = new Stack(); protected internal override void VisitILFunction(ILFunction function) { - var oldFunction = this.currentFunction; context.StepStartGroup("Visit " + function.Name); try { - this.currentFunction = function; + this.currentFunctions.Push(function); base.VisitILFunction(function); } finally { - this.currentFunction = oldFunction; + this.currentFunctions.Pop(); context.StepEndGroup(keepIfEmpty: true); } } @@ -610,58 +630,63 @@ namespace ICSharpCode.Decompiler.IL.Transforms protected internal override void VisitStLoc(StLoc inst) { - base.VisitStLoc(inst); - + DisplayClass displayClass; if (inst.Parent is Block parentBlock && inst.Variable.IsSingleDefinition) { - if ((inst.Variable.Kind == VariableKind.Local || inst.Variable.Kind == VariableKind.StackSlot) && inst.Variable.LoadCount == 0 && (inst.Value is StLoc || inst.Value is CompoundAssignmentInstruction)) { - context.Step($"Remove unused variable assignment {inst.Variable.Name}", inst); - inst.ReplaceWith(inst.Value); - return; - } - if (displayClasses.TryGetValue(inst.Variable, out _) && inst.Value is NewObj) { - instructionsToRemove.Add(inst); + if ((inst.Variable.Kind == VariableKind.Local || inst.Variable.Kind == VariableKind.StackSlot) && inst.Variable.LoadCount == 0) { + // traverse pre-order, so that we do not have to deal with more special cases afterwards + base.VisitStLoc(inst); + if (inst.Value is StLoc || inst.Value is CompoundAssignmentInstruction) { + context.Step($"Remove unused variable assignment {inst.Variable.Name}", inst); + inst.ReplaceWith(inst.Value); + } return; } - if (displayClasses.TryGetValue(inst.Variable, out var displayClass) && inst.Value is Block initBlock && initBlock.Kind == BlockKind.ObjectInitializer) { - instructionsToRemove.Add(inst); - for (int i = 1; i < initBlock.Instructions.Count; i++) { - var stobj = (StObj)initBlock.Instructions[i]; - var variable = displayClass.VariablesToDeclare[(IField)((LdFlda)stobj.Target).Field.MemberDefinition]; - parentBlock.Instructions.Insert(inst.ChildIndex + i, new StLoc(variable.GetOrDeclare(), stobj.Value).WithILRange(stobj)); + if (displayClasses.TryGetValue(inst.Variable, out displayClass) && displayClass.Initializer == inst) { + // inline contents of object initializer block + if (inst.Value is Block initBlock && initBlock.Kind == BlockKind.ObjectInitializer) { + context.Step($"Remove initializer of {inst.Variable.Name}", inst); + for (int i = 1; i < initBlock.Instructions.Count; i++) { + var stobj = (StObj)initBlock.Instructions[i]; + var variable = displayClass.VariablesToDeclare[(IField)((LdFlda)stobj.Target).Field.MemberDefinition]; + parentBlock.Instructions.Insert(inst.ChildIndex + i, new StLoc(variable.GetOrDeclare(), stobj.Value).WithILRange(stobj)); + } } + context.Step($"Remove initializer of {inst.Variable.Name}", inst); + parentBlock.Instructions.Remove(inst); return; } - if (inst.Value.MatchLdLocRef(out var otherVariable) && displayClasses.TryGetValue(otherVariable, out displayClass) && ValidateDisplayClassUses(inst.Variable, displayClass)) { - instructionsToRemove.Add(inst); - displayClasses.Add(inst.Variable, displayClass); + if (inst.Value is LdLoc || inst.Value is LdObj) { + // in some cases (e.g. if inlining fails), there can be a reference to a display class in a stack slot, + // in that case it is necessary to resolve the reference and iff it can be propagated, replace all loads + // of the single-definition. + var referencedDisplayClass = ResolveVariableToPropagate(inst.Value); + if (referencedDisplayClass != null && displayClasses.TryGetValue(referencedDisplayClass, out _)) { + context.Step($"Propagate reference to {referencedDisplayClass.Name} in {inst.Variable}", inst); + foreach (var ld in inst.Variable.LoadInstructions.ToArray()) { + ld.ReplaceWith(new LdLoc(referencedDisplayClass).WithILRange(ld)); + } + parentBlock.Instructions.Remove(inst); + return; + } } } + base.VisitStLoc(inst); } protected internal override void VisitStObj(StObj inst) { - base.VisitStObj(inst); - var instructions = inst.Parent.Children; - int childIndex = inst.ChildIndex; - EarlyExpressionTransforms.StObjToStLoc(inst, context); - if (inst != instructions[childIndex]) { - foreach (var displayClass in displayClasses.Values) { - foreach (var v in displayClass.VariablesToDeclare.Values) { - if (v.CanPropagate) { - if (v.Initializers.Contains(inst)) { - v.Initializers.Add(instructions[childIndex]); - v.Initializers.Remove(inst); - } - } - } - } - if (instructions[childIndex].MatchStLoc(out var innerDisplayClassVar, out var value) && value.MatchLdLoc(out var outerDisplayClassVar)) { - if (!displayClasses.ContainsKey(innerDisplayClassVar) && displayClasses.TryGetValue(outerDisplayClassVar, out var displayClass)) { - displayClasses.Add(innerDisplayClassVar, displayClass); - instructionsToRemove.Add(instructions[childIndex]); + if (IsDisplayClassFieldAccess(inst.Target, out var v, out var displayClass, out var field)) { + VariableToDeclare vd = displayClass.VariablesToDeclare[(IField)field.MemberDefinition]; + if (vd.CanPropagate && vd.Initializers.Contains(inst)) { + if (inst.Parent is Block containingBlock) { + context.Step($"Remove initializer of {v.Name}.{vd.Name} due to propagation", inst); + containingBlock.Instructions.Remove(inst); + return; } } } + base.VisitStObj(inst); + EarlyExpressionTransforms.StObjToStLoc(inst, context); } protected internal override void VisitLdObj(LdObj inst) @@ -701,8 +726,11 @@ namespace ICSharpCode.Decompiler.IL.Transforms context.Step($"Replace {field.Name} with captured variable {v.Name}", inst); ILVariable variable = v.GetOrDeclare(); inst.ReplaceWith(new LdLoca(variable).WithILRange(inst)); - if (variable.Function != currentFunction && variable.Kind != VariableKind.DisplayClassLocal) { - currentFunction.CapturedVariables.Add(variable); + // add captured variable to all descendant functions from the declaring function to this use-site function + foreach (var f in currentFunctions) { + if (f == variable.Function) + break; + f.CapturedVariables.Add(variable); } } }