From d166101387b497d81a6bc76a510688e100c130c9 Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Thu, 12 Jul 2018 23:05:01 +0200 Subject: [PATCH] Allow splitting variables that have their address taken. This works if all addresses are immediately used in calls (as common with method calls on value-type, which take 'this' by-reference); as long as the call doesn't return the reference again. Closes #1136. --- .../TestCases/ILPretty/FSharpUsing_Debug.cs | 6 ++--- .../TestCases/ILPretty/FSharpUsing_Release.cs | 6 ++--- .../IL/ControlFlow/YieldReturnDecompiler.cs | 1 + .../IL/Transforms/SplitVariables.cs | 22 ++++++++++++++----- 4 files changed, 24 insertions(+), 11 deletions(-) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/FSharpUsing_Debug.cs b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/FSharpUsing_Debug.cs index 972149116..914c94a4e 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/FSharpUsing_Debug.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/FSharpUsing_Debug.cs @@ -31,7 +31,7 @@ public static class FSharpUsingPatterns public static void sample4() { Console.WriteLine("some text"); - int num = default(int); + int num; using (FileStream fileStream = File.OpenRead("x.txt")) { num = fileStream.ReadByte(); } @@ -42,12 +42,12 @@ public static class FSharpUsingPatterns public static void sample5() { Console.WriteLine("some text"); - int num = default(int); + int num; using (FileStream fileStream = File.OpenRead("x.txt")) { num = fileStream.ReadByte(); } int num2 = num; - int num3 = default(int); + int num3; using (FileStream fileStream = File.OpenRead("x.txt")) { fileStream.ReadByte(); num3 = fileStream.ReadByte(); diff --git a/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/FSharpUsing_Release.cs b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/FSharpUsing_Release.cs index 9585b51a1..2a9e58dd9 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/FSharpUsing_Release.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/FSharpUsing_Release.cs @@ -31,7 +31,7 @@ public static class FSharpUsingPatterns public static void sample4() { Console.WriteLine("some text"); - int num = default(int); + int num; using (FileStream fileStream = File.OpenRead("x.txt")) { num = fileStream.ReadByte(); } @@ -42,12 +42,12 @@ public static class FSharpUsingPatterns public static void sample5() { Console.WriteLine("some text"); - int num = default(int); + int num; using (FileStream fileStream = File.OpenRead("x.txt")) { num = fileStream.ReadByte(); } int num2 = num; - int num3 = default(int); + int num3; using (FileStream fileStream = File.OpenRead("x.txt")) { fileStream.ReadByte(); num3 = fileStream.ReadByte(); diff --git a/ICSharpCode.Decompiler/IL/ControlFlow/YieldReturnDecompiler.cs b/ICSharpCode.Decompiler/IL/ControlFlow/YieldReturnDecompiler.cs index 408844da6..f9e5c2303 100644 --- a/ICSharpCode.Decompiler/IL/ControlFlow/YieldReturnDecompiler.cs +++ b/ICSharpCode.Decompiler/IL/ControlFlow/YieldReturnDecompiler.cs @@ -799,6 +799,7 @@ 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.StateMachineField = ldflda.Field; fieldToVariableMap.Add(fieldDef, v); } diff --git a/ICSharpCode.Decompiler/IL/Transforms/SplitVariables.cs b/ICSharpCode.Decompiler/IL/Transforms/SplitVariables.cs index 838ee598f..4a22cb963 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/SplitVariables.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/SplitVariables.cs @@ -99,6 +99,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms if (p.Type.SkipModifiers() is ByReferenceType brt && brt.ElementType.IsByRefLike) return AddressUse.Unknown; } + /* Currently there's not really any need to distinguish between readonly and readwrite method calls: var addrParam = call.GetParameter(addressLoadingInstruction.ChildIndex); bool isReadOnly; if (addrParam == null) { @@ -106,8 +107,8 @@ namespace ICSharpCode.Decompiler.IL.Transforms || (call.Method.DeclaringType?.IsKnownType(KnownTypeCode.NullableOfT) ?? false); } else { isReadOnly = false; - } - return isReadOnly ? AddressUse.LocalRead : AddressUse.Unknown; // TODO AddressUse.LocalReadWrite; + }*/ + return AddressUse.LocalReadWrite; default: return AddressUse.Unknown; } @@ -122,7 +123,13 @@ namespace ICSharpCode.Decompiler.IL.Transforms class GroupStores : ReachingDefinitionsVisitor { readonly UnionFind unionFind = new UnionFind(); - readonly HashSet uninitVariableUsage = new HashSet(); + + /// + /// For each uninitialized variable, one representative instruction that + /// potentially observes the unintialized value of the variable. + /// Used to merge together all such loads of the same uninitialized value. + /// + readonly Dictionary uninitVariableUsage = new Dictionary(); public GroupStores(ILFunction scope, CancellationToken cancellationToken) : base(scope, IsCandidateVariable, cancellationToken) { @@ -144,7 +151,12 @@ namespace ICSharpCode.Decompiler.IL.Transforms { if (IsAnalyzedVariable(inst.Variable)) { if (IsPotentiallyUninitialized(state, inst.Variable)) { - uninitVariableUsage.Add(inst); + // merge all uninit loads together: + if (uninitVariableUsage.TryGetValue(inst.Variable, out var uninitLoad)) { + unionFind.Merge(inst, uninitLoad); + } else { + uninitVariableUsage.Add(inst.Variable, inst); + } } foreach (var store in GetStores(state, inst.Variable)) { unionFind.Merge(inst, (IInstructionWithVariableOperand)store); @@ -170,7 +182,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms newVariables.Add(representative, v); inst.Variable.Function.Variables.Add(v); } - if (uninitVariableUsage.Contains(inst)) { + if (inst.Variable.HasInitialValue && uninitVariableUsage.TryGetValue(inst.Variable, out var uninitLoad) && uninitLoad == inst) { v.HasInitialValue = true; } return v;