From 6c72d1c5f0607e22427b94d4398c7eca2b4d2082 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sat, 5 Apr 2025 23:35:14 +0200 Subject: [PATCH] Add transform to remove unconstrained generic reference type check. --- .../ICSharpCode.Decompiler.Tests.csproj | 2 +- .../TestCases/Pretty/Generics.cs | 48 ++++++ .../CSharp/CSharpDecompiler.cs | 2 + ICSharpCode.Decompiler/DecompilerSettings.cs | 8 +- .../ICSharpCode.Decompiler.csproj | 1 + ICSharpCode.Decompiler/IL/ILReader.cs | 2 + .../IL/Instructions/PatternMatching.cs | 12 ++ .../IL/Transforms/ExpressionTransforms.cs | 10 ++ .../IL/Transforms/IILTransform.cs | 1 + .../IL/Transforms/ILInlining.cs | 45 +++++- ...eUnconstrainedGenericReferenceTypeCheck.cs | 145 ++++++++++++++++++ ILSpy/Languages/ILAstLanguage.cs | 1 + 12 files changed, 265 insertions(+), 12 deletions(-) create mode 100644 ICSharpCode.Decompiler/IL/Transforms/RemoveUnconstrainedGenericReferenceTypeCheck.cs diff --git a/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj b/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj index b40bf5b35..ee4393ea7 100644 --- a/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj +++ b/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj @@ -17,7 +17,7 @@ True 1701;1702;1705,67,169,1058,728,1720,649,168,251,660,661,675;1998;162;8632;626;8618;8714;8602;8981 - ROSLYN;NET60;CS60;CS70;CS71;CS72;CS73;CS80;CS90;CS100;CS110;CS120 + ROSLYN;ROSLYN2;ROSLYN3;ROSLYN4;NET60;CS60;CS70;CS71;CS72;CS73;CS80;CS90;CS100;CS110;CS120 False False diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Generics.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Generics.cs index 82a81c43e..c02845c9c 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Generics.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Generics.cs @@ -84,6 +84,11 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty { void Method1() where T : class; void Method2() where T : class; + + void Method3(int a, string b, Type c); +#if CS72 + void Method4(in int a); +#endif } public abstract class Base : IInterface @@ -95,6 +100,16 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty void IInterface.Method2() { } + + void IInterface.Method3(int a, string b, Type c) + { + } + +#if CS72 + void IInterface.Method4(in int a) + { + } +#endif } public class Derived : Base @@ -302,5 +317,38 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty x.Dispose(); y.Dispose(); } + + // prior to C# 7.0 UseRefLocalsForAccurateOrderOfEvaluation is disabled, so we will inline. + // Roslyn 4 generates the explicit ldobj.if.ref pattern, so we can also inline. + // The versions in between, we don't inline, so the code doesn't look pretty. +#if ROSLYN4 || !CS70 + public static int[] Issue3438(T[] array) + { + List list = new List(); + for (int i = 0; i < array.Length; i++) + { + if (!array[i].Equals(default(T))) + { + list.Add(i); + } + } + return list.ToArray(); + } + public void Issue3438b(T[] item1, T item2, int item3) where T : IInterface + { + item1[CastToInt(item2)].Method3(CastToInt(item2), CastToString(item2), TestTypeOf()[1]); + } + public void Issue3438c(T item, T item2, int item3) where T : IInterface + { + CastFromInt(item3).Method3(CastToInt(item2), CastToString(item2), TestTypeOf()[1]); + } + //#if CS72 + // Disabled because ILInlining does not support inlining ldloca currently. + // public void Issue3438d(T[] item1, T item2, int item3) where T : IInterface + // { + // item1[CastToInt(item2)].Method4(CastToInt(item2)); + // } + //#endif +#endif } } diff --git a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs index b239da3a2..07a4126de 100644 --- a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs +++ b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs @@ -149,6 +149,7 @@ namespace ICSharpCode.Decompiler.CSharp new IndexRangeTransform(), new DeconstructionTransform(), new NamedArgumentTransform(), + new RemoveUnconstrainedGenericReferenceTypeCheck(), new UserDefinedLogicTransform(), new InterpolatedStringTransform() ), @@ -1717,6 +1718,7 @@ namespace ICSharpCode.Decompiler.CSharp { var ilReader = new ILReader(typeSystem.MainModule) { UseDebugSymbols = settings.UseDebugSymbols, + UseRefLocalsForAccurateOrderOfEvaluation = settings.UseRefLocalsForAccurateOrderOfEvaluation, DebugInfo = DebugInfoProvider }; var methodDef = metadata.GetMethodDefinition((MethodDefinitionHandle)method.MetadataToken); diff --git a/ICSharpCode.Decompiler/DecompilerSettings.cs b/ICSharpCode.Decompiler/DecompilerSettings.cs index f4fac4485..a0bd68f36 100644 --- a/ICSharpCode.Decompiler/DecompilerSettings.cs +++ b/ICSharpCode.Decompiler/DecompilerSettings.cs @@ -92,7 +92,6 @@ namespace ICSharpCode.Decompiler stringInterpolation = false; dictionaryInitializers = false; extensionMethodsInCollectionInitializers = false; - useRefLocalsForAccurateOrderOfEvaluation = false; getterOnlyAutomaticProperties = false; } if (languageVersion < CSharp.LanguageVersion.CSharp7) @@ -105,6 +104,7 @@ namespace ICSharpCode.Decompiler localFunctions = false; deconstruction = false; patternMatching = false; + useRefLocalsForAccurateOrderOfEvaluation = false; } if (languageVersion < CSharp.LanguageVersion.CSharp7_2) { @@ -190,11 +190,11 @@ namespace ICSharpCode.Decompiler return CSharp.LanguageVersion.CSharp7_2; // C# 7.1 missing if (outVariables || throwExpressions || tupleTypes || tupleConversions - || discards || localFunctions || deconstruction || patternMatching) + || discards || localFunctions || deconstruction || patternMatching || useRefLocalsForAccurateOrderOfEvaluation) return CSharp.LanguageVersion.CSharp7; if (awaitInCatchFinally || useExpressionBodyForCalculatedGetterOnlyProperties || nullPropagation || stringInterpolation || dictionaryInitializers || extensionMethodsInCollectionInitializers - || useRefLocalsForAccurateOrderOfEvaluation || getterOnlyAutomaticProperties) + || getterOnlyAutomaticProperties) return CSharp.LanguageVersion.CSharp6; if (asyncAwait) return CSharp.LanguageVersion.CSharp5; @@ -1126,7 +1126,7 @@ namespace ICSharpCode.Decompiler /// order of evaluation. /// See https://github.com/icsharpcode/ILSpy/issues/2050 /// - [Category("C# 6.0 / VS 2015")] + [Category("C# 7.0 / VS 2017")] [Description("DecompilerSettings.UseRefLocalsForAccurateOrderOfEvaluation")] public bool UseRefLocalsForAccurateOrderOfEvaluation { get { return useRefLocalsForAccurateOrderOfEvaluation; } diff --git a/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj b/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj index 2a9eb220b..87797610f 100644 --- a/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj +++ b/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj @@ -107,6 +107,7 @@ + diff --git a/ICSharpCode.Decompiler/IL/ILReader.cs b/ICSharpCode.Decompiler/IL/ILReader.cs index 254eb068e..272a8ed91 100644 --- a/ICSharpCode.Decompiler/IL/ILReader.cs +++ b/ICSharpCode.Decompiler/IL/ILReader.cs @@ -122,6 +122,7 @@ namespace ICSharpCode.Decompiler.IL readonly MetadataReader metadata; public bool UseDebugSymbols { get; set; } + public bool UseRefLocalsForAccurateOrderOfEvaluation { get; set; } public DebugInfo.IDebugInfoProvider? DebugInfo { get; set; } public List Warnings { get; } = new List(); @@ -1770,6 +1771,7 @@ namespace ICSharpCode.Decompiler.IL StackType expectedStackType = CallInstruction.ExpectedTypeForThisPointer(method.DeclaringType, constrainedPrefix); bool requiresLdObjIfRef = firstArgument == 1 && !firstArgumentIsStObjTarget + && UseRefLocalsForAccurateOrderOfEvaluation && expectedStackType == StackType.Ref && typeOfThis.IsReferenceType != false; for (int i = method.Parameters.Count - 1; i >= 0; i--) { diff --git a/ICSharpCode.Decompiler/IL/Instructions/PatternMatching.cs b/ICSharpCode.Decompiler/IL/Instructions/PatternMatching.cs index 5c5391723..25e9a6498 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/PatternMatching.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/PatternMatching.cs @@ -581,5 +581,17 @@ namespace ICSharpCode.Decompiler.IL { return this; } + + public bool MatchLdObj([NotNullWhen(true)] out ILInstruction? target, IType type) + { + var inst = this as LdObj; + if (inst != null && inst.Type.Equals(type)) + { + target = inst.Target; + return true; + } + target = default(ILInstruction); + return false; + } } } diff --git a/ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs b/ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs index 744008c7e..57b475694 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs @@ -485,6 +485,16 @@ namespace ICSharpCode.Decompiler.IL.Transforms inst.ReplaceWith(inst.Target); return; } + if (inst.Target.MatchLdLoc(out var s) && s.IsSingleDefinition && s.LoadCount == 1 + && s.StoreInstructions.SingleOrDefault() is StLoc { + Value: LdLoca { Variable: { AddressCount: 1, StoreCount: 1 } } + }) + { + context.Step("Single use of ldobj.if.ref(ldloc v) -> ldloc v", inst); + // there already is a temporary, so the ldobj.if.ref is a no-op in both cases + inst.ReplaceWith(inst.Target); + return; + } } protected internal override void VisitStObj(StObj inst) diff --git a/ICSharpCode.Decompiler/IL/Transforms/IILTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/IILTransform.cs index e42551471..347e8e35c 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/IILTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/IILTransform.cs @@ -79,6 +79,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms { return new ILReader(TypeSystem.MainModule) { UseDebugSymbols = Settings.UseDebugSymbols, + UseRefLocalsForAccurateOrderOfEvaluation = Settings.UseRefLocalsForAccurateOrderOfEvaluation, DebugInfo = DebugInfo }; } diff --git a/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs b/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs index 88b2b95c4..2b92f6cc5 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs @@ -170,14 +170,9 @@ namespace ICSharpCode.Decompiler.IL.Transforms public static bool InlineOneIfPossible(Block block, int pos, InliningOptions options, ILTransformContext context) { context.CancellationToken.ThrowIfCancellationRequested(); - StLoc stloc = block.Instructions[pos] as StLoc; - if (stloc == null || stloc.Variable.Kind == VariableKind.PinnedLocal) + if (block.Instructions[pos] is not StLoc stloc) return false; - ILVariable v = stloc.Variable; - // ensure the variable is accessed only a single time - if (v.StoreCount != 1) - return false; - if (v.LoadCount > 1 || v.LoadCount + v.AddressCount != 1) + if (!VariableCanBeUsedForInlining(stloc.Variable)) return false; // TODO: inlining of small integer types might be semantically incorrect, // but we can't avoid it this easily without breaking lots of tests. @@ -186,6 +181,18 @@ namespace ICSharpCode.Decompiler.IL.Transforms return InlineOne(stloc, options, context); } + public static bool VariableCanBeUsedForInlining(ILVariable v) + { + if (v.Kind == VariableKind.PinnedLocal) + return false; + // ensure the variable is accessed only a single time + if (v.StoreCount != 1) + return false; + if (v.LoadCount + v.AddressCount != 1) + return false; + return true; + } + /// /// Inlines the stloc instruction at block.Instructions[pos] into the next instruction. /// @@ -908,6 +915,30 @@ namespace ICSharpCode.Decompiler.IL.Transforms return true; } + /// + /// Gets whether 'expressionBeingMoved' can be moved from somewhere before 'stmt' to become the replacement of 'targetLoad'. + /// + public static bool CanMoveIntoCallVirt(ILInstruction expressionBeingMoved, ILInstruction stmt, CallVirt nestedCallVirt, ILInstruction targetLoad) + { + Debug.Assert(targetLoad.IsDescendantOf(stmt) && nestedCallVirt.IsDescendantOf(stmt)); + ILInstruction thisArg = nestedCallVirt.Arguments[0]; + Debug.Assert(thisArg is LdObjIfRef); + for (ILInstruction inst = targetLoad; inst != stmt; inst = inst.Parent) + { + if (!inst.Parent.CanInlineIntoSlot(inst.ChildIndex, expressionBeingMoved)) + return false; + // Check whether re-ordering with predecessors is valid: + int childIndex = inst.ChildIndex; + for (int i = 0; i < childIndex; ++i) + { + ILInstruction predecessor = inst.Parent.Children[i]; + if (predecessor != thisArg && !IsSafeForInlineOver(predecessor, expressionBeingMoved)) + return false; + } + } + return true; + } + /// /// Gets whether arg can be un-inlined out of stmt. /// diff --git a/ICSharpCode.Decompiler/IL/Transforms/RemoveUnconstrainedGenericReferenceTypeCheck.cs b/ICSharpCode.Decompiler/IL/Transforms/RemoveUnconstrainedGenericReferenceTypeCheck.cs new file mode 100644 index 000000000..83f48b5f6 --- /dev/null +++ b/ICSharpCode.Decompiler/IL/Transforms/RemoveUnconstrainedGenericReferenceTypeCheck.cs @@ -0,0 +1,145 @@ +// Copyright (c) 2025 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. + +#nullable enable + +using System.Collections.Generic; +using System.Linq; + +namespace ICSharpCode.Decompiler.IL.Transforms +{ + /// + /// [stloc address(...)] + /// stloc temp(default.value ``0) + /// if (comp.o(ldloc temp == ldnull)) Block IL_002a { + /// stloc temp(ldobj ``0(ldloc address)) + /// stloc address(ldloca temp) + /// } + /// stloc V_i(expr_i) + /// ...(constrained[``0].callvirt Method(ldobj.if.ref ``0(ldloc address), ldloc V_i ...))... + /// + ///=> + /// + /// [stloc address(...)] + /// stloc address(ldobj.if.ref(ldloc address)) + /// stloc V_i(expr_i) + /// ...(constrained[``0].callvirt Method(ldobj.if.ref ``0(ldloc address), ldloc V_i ...))... + /// + /// Then ldobj.if.ref in the call is redundant because any object reference was already loaded into an immutable temporary. + /// So we can removed and inlining of the arguments (V_i) becomes possible. + /// + /// Finally the newly created ldobj.if.ref is inlined into the place where the old ldobj.if.ref was. + /// + /// => + /// + /// [stloc address(...)] + /// ...(constrained[``0].callvirt Method(ldobj.if.ref ``0(ldloc address), expr_i ...))... + /// + /// + class RemoveUnconstrainedGenericReferenceTypeCheck : IStatementTransform + { + void IStatementTransform.Run(Block block, int pos, StatementTransformContext context) + { + int startPos = pos; + // stloc temp(default.value ``0) + if (!block.Instructions[pos].MatchStLoc(out var temp, out var defaultValue)) + return; + if (!defaultValue.MatchDefaultValue(out var type)) + return; + if (temp.StoreCount != 2 || temp.LoadCount != 1 || temp.AddressCount != 1) + return; + pos++; + // if (comp.o(ldloc temp == ldnull)) Block IL_002a { + // stloc temp(ldobj ``0(ldloc address)) + // stloc address(ldloca temp) + // } + if (block.Instructions.ElementAtOrDefault(pos) is not IfInstruction ifInst) + return; + if (!ifInst.Condition.MatchCompEqualsNull(out var tempLoadForNullCheck)) + return; + if (!tempLoadForNullCheck.MatchLdLoc(temp)) + return; + if (ifInst.TrueInst is not Block dereferenceBlock) + return; + if (ifInst.FalseInst is not Nop) + return; + if (dereferenceBlock.Instructions is not [StLoc tempStore, StLoc addressReassign]) + return; + if (tempStore.Variable != temp) + return; + if (!tempStore.Value.MatchLdObj(out var addressLoadForLdObj, type)) + return; + if (!addressLoadForLdObj.MatchLdLoc(addressReassign.Variable)) + return; + if (!addressReassign.Value.MatchLdLoca(temp)) + return; + var address = addressReassign.Variable; + if (address.StoreCount != 2 || address.LoadCount != 2 || address.AddressCount != 0) + return; + pos++; + // pos now is the first store to V_i + // ...(constrained[``0].callvirt Method(ldobj.if.ref ``0(ldloc address), ldloc V_i ...))... + var callTarget = address.LoadInstructions.Single(l => addressLoadForLdObj != l); + if (callTarget.Parent is not LdObjIfRef { Parent: CallVirt call } ldobjIfRef) + return; + if (call.Arguments.Count == 0 || call.Arguments[0] != ldobjIfRef || !type.Equals(call.ConstrainedTo)) + return; + ILInstruction containingStmt = call; + while (containingStmt.Parent != block) + { + if (containingStmt.Parent == null) + return; + containingStmt = containingStmt.Parent; + } + if (containingStmt.ChildIndex < pos) + return; + // check if we can inline all temporaries used in the call: + int temporaryInitIndex = containingStmt.ChildIndex - 1; + List<(ILInstruction, ILInstruction)> replacements = new(); + for (int argIndex = call.Arguments.Count - 1; argIndex > 0 && temporaryInitIndex >= pos; argIndex--) + { + var argument = call.Arguments[argIndex]; + switch (argument) + { + case LdLoc load: + if (block.Instructions[temporaryInitIndex].MatchStLoc(load.Variable, out var expr) && ILInlining.VariableCanBeUsedForInlining(load.Variable)) + { + if (!ILInlining.CanMoveIntoCallVirt(expr, containingStmt, call, argument)) + { + return; + } + replacements.Add((argument, expr)); + temporaryInitIndex--; + } + break; + } + } + // all stores to V_i processed? + if (temporaryInitIndex != pos - 1) + { + return; + } + context.Step("RemoveUnconstrainedGenericReferenceTypeCheck", block.Instructions[startPos]); + foreach (var (argument, expr) in replacements) + { + argument.ReplaceWith(expr); + } + block.Instructions.RemoveRange(startPos, containingStmt.ChildIndex - startPos); + } + } +} \ No newline at end of file diff --git a/ILSpy/Languages/ILAstLanguage.cs b/ILSpy/Languages/ILAstLanguage.cs index 46ccd0fe4..948a79511 100644 --- a/ILSpy/Languages/ILAstLanguage.cs +++ b/ILSpy/Languages/ILAstLanguage.cs @@ -109,6 +109,7 @@ namespace ICSharpCode.ILSpy var typeSystem = new DecompilerTypeSystem(module, assemblyResolver); var reader = new ILReader(typeSystem.MainModule); reader.UseDebugSymbols = options.DecompilerSettings.UseDebugSymbols; + reader.UseRefLocalsForAccurateOrderOfEvaluation = options.DecompilerSettings.UseRefLocalsForAccurateOrderOfEvaluation; var methodBody = module.GetMethodBody(methodDef.RelativeVirtualAddress); ILFunction il = reader.ReadIL((SRM.MethodDefinitionHandle)method.MetadataToken, methodBody, kind: ILFunctionKind.TopLevelFunction, cancellationToken: options.CancellationToken); var decompiler = new CSharpDecompiler(typeSystem, options.DecompilerSettings) { CancellationToken = options.CancellationToken };