From 1c7d9705da5fba2b04febc5eeca7e0d790d92f8b Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Wed, 20 May 2020 14:18:41 +0200 Subject: [PATCH] Add Validation of delegate target instructions. --- .../IL/Instructions/Block.cs | 10 +++--- .../IL/Transforms/DelegateConstruction.cs | 36 +++++++++++++++++-- ...ransformCollectionAndObjectInitializers.cs | 4 +-- 3 files changed, 41 insertions(+), 9 deletions(-) diff --git a/ICSharpCode.Decompiler/IL/Instructions/Block.cs b/ICSharpCode.Decompiler/IL/Instructions/Block.cs index bab84c2b4..d819a3092 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/Block.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/Block.cs @@ -134,11 +134,11 @@ namespace ICSharpCode.Decompiler.IL break; case BlockKind.ArrayInitializer: var final = finalInstruction as LdLoc; - Debug.Assert(final != null && final.Variable.Kind == VariableKind.InitializerTarget); + Debug.Assert(final != null && final.Variable.IsSingleDefinition && final.Variable.Kind == VariableKind.InitializerTarget); IType type = null; Debug.Assert(Instructions[0].MatchStLoc(final.Variable, out var init) && init.MatchNewArr(out type)); for (int i = 1; i < Instructions.Count; i++) { - Debug.Assert(Instructions[i].MatchStObj(out ILInstruction target, out ILInstruction value, out var t) && type != null && type.Equals(t)); + Debug.Assert(Instructions[i].MatchStObj(out ILInstruction target, out _, out var t) && type != null && type.Equals(t)); Debug.Assert(target.MatchLdElema(out t, out ILInstruction array) && type.Equals(t)); Debug.Assert(array.MatchLdLoc(out ILVariable v) && v == final.Variable); } @@ -146,7 +146,9 @@ namespace ICSharpCode.Decompiler.IL case BlockKind.CollectionInitializer: case BlockKind.ObjectInitializer: var final2 = finalInstruction as LdLoc; - Debug.Assert(final2 != null && final2.Variable.Kind == VariableKind.InitializerTarget); + Debug.Assert(final2 != null); + var initVar2 = final2.Variable; + Debug.Assert(initVar2.StoreCount == 1 && initVar2.Kind == VariableKind.InitializerTarget); IType type2 = null; Debug.Assert(Instructions[0].MatchStLoc(final2.Variable, out var init2)); Debug.Assert(init2 is NewObj || init2 is DefaultValue || (init2 is Block named && named.Kind == BlockKind.CallWithNamedArgs)); @@ -165,7 +167,7 @@ namespace ICSharpCode.Decompiler.IL break; } for (int i = 1; i < Instructions.Count; i++) { - Debug.Assert(Instructions[i] is StLoc || IL.Transforms.AccessPathElement.GetAccessPath(Instructions[i], type2).Kind != IL.Transforms.AccessPathKind.Invalid); + Debug.Assert(Instructions[i] is StLoc || AccessPathElement.GetAccessPath(Instructions[i], type2).Kind != IL.Transforms.AccessPathKind.Invalid); } break; } diff --git a/ICSharpCode.Decompiler/IL/Transforms/DelegateConstruction.cs b/ICSharpCode.Decompiler/IL/Transforms/DelegateConstruction.cs index 0ab86c1f5..7dfe2306a 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/DelegateConstruction.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/DelegateConstruction.cs @@ -53,9 +53,8 @@ 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) { + if (store.Value is NewObj) { instWithVar.Variable.CaptureScope = BlockContainer.FindClosestContainer(store); } } @@ -141,6 +140,8 @@ namespace ICSharpCode.Decompiler.IL.Transforms if (LocalFunctionDecompiler.IsLocalFunctionMethod(targetMethod, context)) return null; target = value.Arguments[0]; + if (!ValidateDelegateTarget(target)) + return null; var handle = (MethodDefinitionHandle)targetMethod.MetadataToken; if (activeMethods.Contains(handle)) { this.context.Function.Warnings.Add(" Found self-referencing delegate construction. Abort transformation to avoid stack overflow."); @@ -180,6 +181,37 @@ namespace ICSharpCode.Decompiler.IL.Transforms return function; } + private static bool ValidateDelegateTarget(ILInstruction inst) + { + switch (inst) { + case LdNull _: + return true; + case LdLoc ldloc: + return ldloc.Variable.IsSingleDefinition; + case LdObj ldobj: + // TODO : should make sure that the display-class 'this' is unused, + // 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 + // that the value of the field is never changed. + ILInstruction target = ldobj; + while (target is LdObj || target is LdFlda) { + if (target is LdObj o) { + target = o.Target; + continue; + } + if (target is LdFlda f) { + target = f.Target; + continue; + } + } + return target is LdLoc; + default: + return false; + } + } + private IEnumerable GetTransforms() { yield return new CombineExitsTransform(); diff --git a/ICSharpCode.Decompiler/IL/Transforms/TransformCollectionAndObjectInitializers.cs b/ICSharpCode.Decompiler/IL/Transforms/TransformCollectionAndObjectInitializers.cs index 95c681449..ee619d6a8 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/TransformCollectionAndObjectInitializers.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/TransformCollectionAndObjectInitializers.cs @@ -60,10 +60,8 @@ namespace ICSharpCode.Decompiler.IL.Transforms // for the base ctor call) or a compiler-generated delegate method, which might be used in a query expression. return false; } - // Do not try to transform display class usages or delegate construction. + // Do not try to transform delegate construction. // DelegateConstruction transform cannot deal with this. - if (TransformDisplayClassUsage.AnalyzeVariable(v, context)) - return false; if (DelegateConstruction.IsDelegateConstruction(newObjInst) || TransformDisplayClassUsage.IsPotentialClosure(context, newObjInst)) return false; // Cannot build a collection/object initializer attached to an AnonymousTypeCreateExpression:s