From e1ca4db851653aa30e7ad3a35631ab0209a83eb7 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Thu, 28 Oct 2021 23:02:06 +0200 Subject: [PATCH] Fix #2527: Support skip locals init --- .../CSharp/ExpressionBuilder.cs | 4 +- .../CSharp/Transforms/DeclareVariables.cs | 97 +++++++++++++-- .../ReachingDefinitionsVisitor.cs | 11 +- .../IL/ControlFlow/YieldReturnDecompiler.cs | 3 +- ICSharpCode.Decompiler/IL/ILReader.cs | 8 +- ICSharpCode.Decompiler/IL/ILVariable.cs | 114 +++++++++++++++--- .../IL/Transforms/RemoveDeadVariableInit.cs | 6 +- .../IL/Transforms/SplitVariables.cs | 7 +- .../Transforms/TransformDisplayClassUsage.cs | 18 +-- 9 files changed, 209 insertions(+), 59 deletions(-) diff --git a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs b/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs index 6786707c7..802e566db 100644 --- a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs @@ -2444,9 +2444,9 @@ namespace ICSharpCode.Decompiler.CSharp var body = statementBuilder.ConvertAsBlock(container); var comment = new Comment(" Could not convert BlockContainer to single expression"); body.InsertChildAfter(null, comment, Roles.Comment); - // set ILVariable.HasInitialValue for any variables being used inside the container + // set ILVariable.UsesInitialValue for any variables being used inside the container foreach (var stloc in container.Descendants.OfType()) - stloc.Variable.HasInitialValue = true; + stloc.Variable.UsesInitialValue = true; var ame = new AnonymousMethodExpression { Body = body }; var systemFuncType = compilation.FindType(typeof(Func<>)); var blockReturnType = InferReturnType(body); diff --git a/ICSharpCode.Decompiler/CSharp/Transforms/DeclareVariables.cs b/ICSharpCode.Decompiler/CSharp/Transforms/DeclareVariables.cs index 510821104..4eda0ba10 100644 --- a/ICSharpCode.Decompiler/CSharp/Transforms/DeclareVariables.cs +++ b/ICSharpCode.Decompiler/CSharp/Transforms/DeclareVariables.cs @@ -70,6 +70,13 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms } } + enum VariableInitKind + { + None, + NeedsDefaultValue, + NeedsSkipInit + } + [DebuggerDisplay("VariableToDeclare(Name={Name})")] class VariableToDeclare { @@ -80,7 +87,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms /// /// Whether the variable needs to be default-initialized. /// - public bool DefaultInitialization; + public VariableInitKind DefaultInitialization; /// /// Integer value that can be used to compare to VariableToDeclare instances @@ -106,10 +113,24 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms public bool RemovedDueToCollision => ReplacementDueToCollision != null; public bool DeclaredInDeconstruction; - public VariableToDeclare(ILVariable variable, bool defaultInitialization, InsertionPoint insertionPoint, IdentifierExpression firstUse, int sourceOrder) + public VariableToDeclare(ILVariable variable, InsertionPoint insertionPoint, IdentifierExpression firstUse, int sourceOrder) { this.ILVariable = variable; - this.DefaultInitialization = defaultInitialization; + if (variable.UsesInitialValue) + { + if (variable.InitialValueIsInitialized) + { + this.DefaultInitialization = VariableInitKind.NeedsDefaultValue; + } + else + { + this.DefaultInitialization = VariableInitKind.NeedsSkipInit; + } + } + else + { + this.DefaultInitialization = VariableInitKind.None; + } this.InsertionPoint = insertionPoint; this.FirstUse = firstUse; this.SourceOrder = sourceOrder; @@ -304,7 +325,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms else { newPoint = new InsertionPoint { level = nodeLevel, nextNode = identExpr }; - if (variable.HasInitialValue) + if (variable.UsesInitialValue) { // Uninitialized variables are logically initialized at the beginning of the function // Because it's possible that the variable has a loop-carried dependency, @@ -331,8 +352,8 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms } else { - v = new VariableToDeclare(variable, variable.HasInitialValue, - newPoint, identExpr, sourceOrder: variableDict.Count); + v = new VariableToDeclare(variable, newPoint, + identExpr, sourceOrder: variableDict.Count); variableDict.Add(variable, v); } } @@ -623,17 +644,69 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms // Insert a separate declaration statement. Expression initializer = null; AstType type = context.TypeSystemAstBuilder.ConvertType(v.Type); - if (v.DefaultInitialization) + if (v.DefaultInitialization == VariableInitKind.NeedsDefaultValue) { initializer = new DefaultValueExpression(type.Clone()); } var vds = new VariableDeclarationStatement(type, v.Name, initializer); vds.Variables.Single().AddAnnotation(new ILVariableResolveResult(ilVariable)); Debug.Assert(v.InsertionPoint.nextNode.Role == BlockStatement.StatementRole); - v.InsertionPoint.nextNode.Parent.InsertChildBefore( - v.InsertionPoint.nextNode, - vds, - BlockStatement.StatementRole); + if (v.DefaultInitialization == VariableInitKind.NeedsSkipInit) + { + AstType unsafeType = context.TypeSystemAstBuilder.ConvertType( + context.TypeSystem.FindType(KnownTypeCode.Unsafe)); + if (context.Settings.OutVariables) + { + var outVarDecl = new OutVarDeclarationExpression(type.Clone(), v.Name); + outVarDecl.Variable.AddAnnotation(new ILVariableResolveResult(ilVariable)); + v.InsertionPoint.nextNode.Parent.InsertChildBefore( + v.InsertionPoint.nextNode, + new ExpressionStatement { + Expression = new InvocationExpression { + Target = new MemberReferenceExpression { + Target = new TypeReferenceExpression(unsafeType), + MemberName = "SkipInit" + }, + Arguments = { + outVarDecl + } + } + }, + BlockStatement.StatementRole); + } + else + { + v.InsertionPoint.nextNode.Parent.InsertChildBefore( + v.InsertionPoint.nextNode, + vds, + BlockStatement.StatementRole); + v.InsertionPoint.nextNode.Parent.InsertChildBefore( + v.InsertionPoint.nextNode, + new ExpressionStatement { + Expression = new InvocationExpression { + Target = new MemberReferenceExpression { + Target = new TypeReferenceExpression(unsafeType), + MemberName = "SkipInit" + }, + Arguments = { + new DirectionExpression( + FieldDirection.Out, + new IdentifierExpression(v.Name) + .WithRR(new ILVariableResolveResult(ilVariable)) + ) + } + } + }, + BlockStatement.StatementRole); + } + } + else + { + v.InsertionPoint.nextNode.Parent.InsertChildBefore( + v.InsertionPoint.nextNode, + vds, + BlockStatement.StatementRole); + } } } // perform replacements at end, so that we don't replace a node while it is still referenced by a VariableToDeclare @@ -650,7 +723,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms return false; if (!context.Settings.OutVariables) return false; - if (v.DefaultInitialization) + if (v.DefaultInitialization != VariableInitKind.None) return false; for (AstNode node = v.FirstUse; node != null; node = node.Parent) { diff --git a/ICSharpCode.Decompiler/FlowAnalysis/ReachingDefinitionsVisitor.cs b/ICSharpCode.Decompiler/FlowAnalysis/ReachingDefinitionsVisitor.cs index 6b4013c0f..ff32e3f86 100644 --- a/ICSharpCode.Decompiler/FlowAnalysis/ReachingDefinitionsVisitor.cs +++ b/ICSharpCode.Decompiler/FlowAnalysis/ReachingDefinitionsVisitor.cs @@ -296,14 +296,9 @@ namespace ICSharpCode.Decompiler.FlowAnalysis var stores = storesByVar[vi]; if (stores != null) { - int expectedStoreCount = scope.Variables[vi].StoreCount; - if (!scope.Variables[vi].HasInitialValue) - { - // Extra store for the uninitialized state. - expectedStoreCount += 1; - // Note that for variables with HasInitialValue=true, - // this extra store is already accounted for in ILVariable.StoreCount. - } + int expectedStoreCount = scope.Variables[vi].StoreInstructions.Count; + // Extra store for the uninitialized state. + expectedStoreCount += 1; Debug.Assert(stores.Count == expectedStoreCount); stores.CopyTo(allStores, si); // Add all stores except for the first (representing the uninitialized state) diff --git a/ICSharpCode.Decompiler/IL/ControlFlow/YieldReturnDecompiler.cs b/ICSharpCode.Decompiler/IL/ControlFlow/YieldReturnDecompiler.cs index fc98e4b43..73d23329b 100644 --- a/ICSharpCode.Decompiler/IL/ControlFlow/YieldReturnDecompiler.cs +++ b/ICSharpCode.Decompiler/IL/ControlFlow/YieldReturnDecompiler.cs @@ -995,7 +995,8 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow name = fieldDef.Name.Substring(1, pos - 1); } v = function.RegisterVariable(VariableKind.Local, ldflda.Field.ReturnType, name); - v.HasInitialValue = true; // the field was default-initialized, so keep those semantics for the variable + v.InitialValueIsInitialized = true; // the field was default-initialized, so keep those semantics for the variable + v.UsesInitialValue = true; v.StateMachineField = ldflda.Field; fieldToVariableMap.Add(fieldDef, v); } diff --git a/ICSharpCode.Decompiler/IL/ILReader.cs b/ICSharpCode.Decompiler/IL/ILReader.cs index 83e78cb82..a78a26eb6 100644 --- a/ICSharpCode.Decompiler/IL/ILReader.cs +++ b/ICSharpCode.Decompiler/IL/ILReader.cs @@ -118,12 +118,10 @@ namespace ICSharpCode.Decompiler.IL this.methodReturnStackType = method.ReturnType.GetStackType(); InitParameterVariables(); localVariables = InitLocalVariables(); - if (body.LocalVariablesInitialized) + foreach (var v in localVariables) { - foreach (var v in localVariables) - { - v.HasInitialValue = true; - } + v.InitialValueIsInitialized = body.LocalVariablesInitialized; + v.UsesInitialValue = true; } this.mainContainer = new BlockContainer(expectedResultType: methodReturnStackType); this.instructionBuilder = new List(); diff --git a/ICSharpCode.Decompiler/IL/ILVariable.cs b/ICSharpCode.Decompiler/IL/ILVariable.cs index f70707a36..c950523fe 100644 --- a/ICSharpCode.Decompiler/IL/ILVariable.cs +++ b/ICSharpCode.Decompiler/IL/ILVariable.cs @@ -252,13 +252,13 @@ namespace ICSharpCode.Decompiler.IL /// stloc /// TryCatchHandler (assigning the exception variable) /// PinnedRegion (assigning the pointer variable) - /// initial values () + /// initial values () /// /// /// /// This variable is automatically updated when adding/removing stores instructions from the ILAst. /// - public int StoreCount => (hasInitialValue ? 1 : 0) + StoreInstructions.Count; + public int StoreCount => (usesInitialValue ? 1 : 0) + StoreInstructions.Count; readonly List storeInstructions = new List(); @@ -270,7 +270,7 @@ namespace ICSharpCode.Decompiler.IL /// stloc /// TryCatchHandler (assigning the exception variable) /// PinnedRegion (assigning the pointer variable) - /// initial values () -- however, there is no instruction for + /// initial values () -- however, there is no instruction for /// the initial value, so it is not contained in the store list. /// /// @@ -320,27 +320,86 @@ namespace ICSharpCode.Decompiler.IL list.RemoveAt(indexToMove); } - bool hasInitialValue; + bool initialValueIsInitialized; /// - /// Gets/Sets whether the variable has an initial value. + /// Gets/Sets whether the variable's initial value is initialized. /// This is always true for parameters (incl. this). /// - /// Normal variables have an initial value if the function uses ".locals init" - /// and that initialization is not a dead store. + /// Normal variables have an initial value if the function uses ".locals init". + /// + public bool InitialValueIsInitialized { + get { return initialValueIsInitialized; } + set { + if (Kind == VariableKind.Parameter && !value) + throw new InvalidOperationException("Cannot remove InitialValueIsInitialized from parameters"); + initialValueIsInitialized = value; + } + } + + bool usesInitialValue; + + /// + /// Gets/Sets whether the initial value of the variable is used. + /// This is always true for parameters (incl. this). + /// + /// Normal variables use the initial value, if no explicit initialization is done. /// /// - /// An initial value is counted as a store (adds 1 to StoreCount) + /// The following table shows the relationship between + /// and . + /// + /// + /// + /// + /// Meaning + /// + /// + /// + /// + /// This variable's initial value is zero-initialized (.locals init) and the initial value is used. + /// From C#'s point of view a the value default(T) is assigned at the site of declaration. + /// + /// + /// + /// + /// This variable's initial value is zero-initialized (.locals init) and the initial value is not used. + /// From C#'s point of view no implicit initialization occurs, because the code assigns a value + /// explicitly, before the variable is first read. + /// + /// + /// + /// + /// This variable's initial value is uninitialized (.locals without init) and the + /// initial value is used. + /// From C#'s point of view a call to + /// is generated after the declaration. + /// + /// + /// + /// + /// This variable's initial value is uninitialized (.locals without init) and the + /// initial value is not used. + /// From C#'s point of view no implicit initialization occurs, because the code assigns a value + /// explicitly, before the variable is first read. + /// + /// /// - public bool HasInitialValue { - get { return hasInitialValue; } + public bool UsesInitialValue { + get { return usesInitialValue; } set { if (Kind == VariableKind.Parameter && !value) - throw new InvalidOperationException("Cannot remove HasInitialValue from parameters"); - hasInitialValue = value; + throw new InvalidOperationException("Cannot remove UsesInitialValue from parameters"); + usesInitialValue = value; } } + [Obsolete("Use 'UsesInitialValue' instead.")] + public bool HasInitialValue { + get => UsesInitialValue; + set => UsesInitialValue = value; + } + /// /// Gets whether the variable is in SSA form: /// There is exactly 1 store, and every load sees the value from that store. @@ -362,7 +421,7 @@ namespace ICSharpCode.Decompiler.IL /// public bool IsDead { get { - return StoreCount == (HasInitialValue ? 1 : 0) + return StoreInstructions.Count == 0 && LoadCount == 0 && AddressCount == 0; } @@ -388,7 +447,10 @@ namespace ICSharpCode.Decompiler.IL this.StackType = type.GetStackType(); this.Index = index; if (kind == VariableKind.Parameter) - this.HasInitialValue = true; + { + this.InitialValueIsInitialized = true; + this.UsesInitialValue = true; + } CheckInvariant(); } @@ -401,7 +463,10 @@ namespace ICSharpCode.Decompiler.IL this.StackType = stackType; this.Index = index; if (kind == VariableKind.Parameter) - this.HasInitialValue = true; + { + this.InitialValueIsInitialized = true; + this.UsesInitialValue = true; + } CheckInvariant(); } @@ -472,9 +537,24 @@ namespace ICSharpCode.Decompiler.IL output.Write("Index={0}, ", Index); } output.Write("LoadCount={0}, AddressCount={1}, StoreCount={2})", LoadCount, AddressCount, StoreCount); - if (hasInitialValue && Kind != VariableKind.Parameter) + if (Kind != VariableKind.Parameter) { - output.Write(" init"); + if (initialValueIsInitialized) + { + output.Write(" init"); + } + else + { + output.Write(" uninit"); + } + if (usesInitialValue) + { + output.Write(" used"); + } + else + { + output.Write(" unused"); + } } if (CaptureScope != null) { diff --git a/ICSharpCode.Decompiler/IL/Transforms/RemoveDeadVariableInit.cs b/ICSharpCode.Decompiler/IL/Transforms/RemoveDeadVariableInit.cs index 284914cfa..7501338b3 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/RemoveDeadVariableInit.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/RemoveDeadVariableInit.cs @@ -36,7 +36,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms { public void Run(ILFunction function, ILTransformContext context) { - ResetHasInitialValueFlag(function, context); + ResetUsesInitialValueFlag(function, context); // Remove dead stores to variables that are never read from. // If the stored value has some side-effect, the value is unwrapped. // This is necessary to remove useless stores generated by some compilers, e.g., the F# compiler. @@ -105,7 +105,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms } } - internal static void ResetHasInitialValueFlag(ILFunction function, ILTransformContext context) + internal static void ResetUsesInitialValueFlag(ILFunction function, ILTransformContext context) { var visitor = new DefiniteAssignmentVisitor(function, context.CancellationToken); function.AcceptVisitor(visitor); @@ -113,7 +113,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms { if (v.Kind != VariableKind.Parameter && !visitor.IsPotentiallyUsedUninitialized(v)) { - v.HasInitialValue = false; + v.UsesInitialValue = false; } } } diff --git a/ICSharpCode.Decompiler/IL/Transforms/SplitVariables.cs b/ICSharpCode.Decompiler/IL/Transforms/SplitVariables.cs index 9ef159530..4f3bbd698 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/SplitVariables.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/SplitVariables.cs @@ -270,14 +270,15 @@ namespace ICSharpCode.Decompiler.IL.Transforms v.Name = inst.Variable.Name; v.HasGeneratedName = inst.Variable.HasGeneratedName; v.StateMachineField = inst.Variable.StateMachineField; - v.HasInitialValue = false; // we'll set HasInitialValue when we encounter an uninit load + v.InitialValueIsInitialized = inst.Variable.InitialValueIsInitialized; + v.UsesInitialValue = false; // we'll set UsesInitialValue when we encounter an uninit load v.RemoveIfRedundant = inst.Variable.RemoveIfRedundant; newVariables.Add(representative, v); inst.Variable.Function.Variables.Add(v); } - if (inst.Variable.HasInitialValue && uninitVariableUsage.TryGetValue(inst.Variable, out var uninitLoad) && uninitLoad == inst) + if (inst.Variable.UsesInitialValue && uninitVariableUsage.TryGetValue(inst.Variable, out var uninitLoad) && uninitLoad == inst) { - v.HasInitialValue = true; + v.UsesInitialValue = true; } return v; } diff --git a/ICSharpCode.Decompiler/IL/Transforms/TransformDisplayClassUsage.cs b/ICSharpCode.Decompiler/IL/Transforms/TransformDisplayClassUsage.cs index 9a622432d..98848df6e 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/TransformDisplayClassUsage.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/TransformDisplayClassUsage.cs @@ -56,7 +56,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms public string Name => field.Name; public bool CanPropagate { get; private set; } - public bool HasInitialValue { get; set; } + public bool UsesInitialValue { get; set; } public HashSet Initializers { get; } = new HashSet(); @@ -80,7 +80,8 @@ namespace ICSharpCode.Decompiler.IL.Transforms if (declaredVariable != null) return declaredVariable; declaredVariable = container.Variable.Function.RegisterVariable(VariableKind.Local, field.Type, field.Name); - declaredVariable.HasInitialValue = HasInitialValue; + declaredVariable.InitialValueIsInitialized = true; + declaredVariable.UsesInitialValue = UsesInitialValue; declaredVariable.CaptureScope = container.CaptureScope; return declaredVariable; } @@ -185,7 +186,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms if (displayClass.VariablesToDeclare.ContainsKey(f)) continue; var variable = AddVariable(displayClass, null, f); - variable.HasInitialValue = true; + variable.UsesInitialValue = true; displayClass.VariablesToDeclare[(IField)f.MemberDefinition] = variable; } @@ -304,7 +305,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms HandleInitBlock(stloc.Parent as Block, stloc.ChildIndex + 1, result, result.Variable); break; case TypeKind.Struct: - if (v.StoreCount != (v.HasInitialValue ? 1 : 0)) + if (v.StoreInstructions.Count != 0) return null; Debug.Assert(v.StoreInstructions.Count == 0); result = new DisplayClass(v, definition) { CaptureScope = v.CaptureScope }; @@ -509,8 +510,8 @@ namespace ICSharpCode.Decompiler.IL.Transforms variable.Propagate(ResolveVariableToPropagate(statement.Value, field.Type)); variable.Initializers.Add(statement); } - variable.HasInitialValue = - result.Type.IsReferenceType != false || result.Variable.HasInitialValue; + variable.UsesInitialValue = + result.Type.IsReferenceType != false || result.Variable.UsesInitialValue; return variable; } @@ -583,7 +584,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms context.Step($"ResetHasInitialValueFlag", function); foreach (var f in function.Descendants.OfType()) { - RemoveDeadVariableInit.ResetHasInitialValueFlag(f, context); + RemoveDeadVariableInit.ResetUsesInitialValueFlag(f, context); f.CapturedVariables.RemoveWhere(v => v.IsDead); } } @@ -601,7 +602,8 @@ namespace ICSharpCode.Decompiler.IL.Transforms } } closureType = variable.Type.GetDefinition(); - if (context.Settings.LocalFunctions && closureType?.Kind == TypeKind.Struct && variable.HasInitialValue && IsPotentialClosure(context, closureType)) + if (context.Settings.LocalFunctions && closureType?.Kind == TypeKind.Struct + && variable.UsesInitialValue && IsPotentialClosure(context, closureType)) { initializer = LocalFunctionDecompiler.GetStatement(variable.AddressInstructions.OrderBy(i => i.StartILOffset).First()); return true;