From 8cfc21dfab2c35137eb127f8341678cd53d8ce04 Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Sat, 27 Jul 2019 15:43:51 +0200 Subject: [PATCH 1/2] Fix DefiniteAssignmentVisitor bugs handling ILFunction, and add some comments. --- .../FlowAnalysis/DataFlowVisitor.cs | 4 ++++ .../FlowAnalysis/DefiniteAssignmentVisitor.cs | 21 +++++++++++++++++-- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs b/ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs index 6d64b5dd1..a283fcad0 100644 --- a/ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs +++ b/ICSharpCode.Decompiler/FlowAnalysis/DataFlowVisitor.cs @@ -645,8 +645,11 @@ namespace ICSharpCode.Decompiler.FlowAnalysis DebugStartPoint(inst); var oldState = stateOnNullableRewrap.Clone(); stateOnNullableRewrap.ReplaceWithBottom(); + inst.Argument.AcceptVisitor(this); + // Join incoming control flow from the NullableUnwraps. state.JoinWith(stateOnNullableRewrap); + stateOnNullableRewrap = oldState; DebugEndPoint(inst); } @@ -655,6 +658,7 @@ namespace ICSharpCode.Decompiler.FlowAnalysis { DebugStartPoint(inst); inst.Argument.AcceptVisitor(this); + // Conditional control flow edge to the surrounding NullableRewrap. stateOnNullableRewrap.JoinWith(state); DebugEndPoint(inst); } diff --git a/ICSharpCode.Decompiler/FlowAnalysis/DefiniteAssignmentVisitor.cs b/ICSharpCode.Decompiler/FlowAnalysis/DefiniteAssignmentVisitor.cs index af6006009..5b465b735 100644 --- a/ICSharpCode.Decompiler/FlowAnalysis/DefiniteAssignmentVisitor.cs +++ b/ICSharpCode.Decompiler/FlowAnalysis/DefiniteAssignmentVisitor.cs @@ -120,6 +120,7 @@ namespace ICSharpCode.Decompiler.FlowAnalysis readonly CancellationToken cancellationToken; readonly ILFunction scope; readonly BitSet variablesWithUninitializedUsage; + readonly Dictionary stateOfLocalFunctionUse = new Dictionary(); readonly HashSet localFunctionsNeedingAnalysis = new HashSet(); @@ -213,7 +214,19 @@ namespace ICSharpCode.Decompiler.FlowAnalysis DebugStartPoint(inst); State stateBeforeFunction = state.Clone(); State stateOnExceptionBeforeFunction = currentStateOnException.Clone(); + // Note: lambdas are handled at their point of declaration. + // We immediately visit their body, because captured variables need to be definitely initialized at this point. + // We ignore the state after the lambda body (by resetting to the state before), because we don't know + // when the lambda will be invoked. + // This also makes this logic unsuitable for reaching definitions, as we wouldn't see the effect of stores in lambdas. + // Only the simpler case of definite assignment can support lambdas. inst.Body.AcceptVisitor(this); + + // For local functions, the situation is similar to lambdas. + // However, we don't use the state of the declaration site when visiting local functions, + // but instead the state(s) of their point of use. + // Because we might discover additional points of use within the local functions, + // we use a fixed-point iteration. bool changed; do { changed = false; @@ -223,8 +236,8 @@ namespace ICSharpCode.Decompiler.FlowAnalysis localFunctionsNeedingAnalysis.Remove(nestedFunction.ReducedMethod); State stateOnEntry = stateOfLocalFunctionUse[nestedFunction.ReducedMethod]; this.state.ReplaceWith(stateOnEntry); - this.currentStateOnException.ReplaceWithBottom(); - nestedFunction.Body.AcceptVisitor(this); + this.currentStateOnException.ReplaceWith(stateOnEntry); + nestedFunction.AcceptVisitor(this); changed = true; } } while (changed); @@ -258,6 +271,10 @@ namespace ICSharpCode.Decompiler.FlowAnalysis DebugEndPoint(call); } + /// + /// For a use of a local function, remember the current state to use as stateOnEntry when + /// later processing the local function body. + /// void HandleLocalFunctionUse(IMethod method) { if (method.IsLocalFunction) { From afde03a04dcc29a72bc4bf24010e6f90c80da1fc Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Sat, 27 Jul 2019 15:53:38 +0200 Subject: [PATCH 2/2] Fix #1597: Incorrect handling of nullability annotations during generic type substitution. --- .../Implementation/NullabilityAnnotatedType.cs | 9 +++++++-- .../TypeSystem/ParameterizedType.cs | 14 ++++++++++++-- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/ICSharpCode.Decompiler/TypeSystem/Implementation/NullabilityAnnotatedType.cs b/ICSharpCode.Decompiler/TypeSystem/Implementation/NullabilityAnnotatedType.cs index fa87dd4ec..b13bede86 100644 --- a/ICSharpCode.Decompiler/TypeSystem/Implementation/NullabilityAnnotatedType.cs +++ b/ICSharpCode.Decompiler/TypeSystem/Implementation/NullabilityAnnotatedType.cs @@ -53,11 +53,16 @@ namespace ICSharpCode.Decompiler.TypeSystem.Implementation IType newBase = baseType.AcceptVisitor(visitor); if (newBase != baseType) { if (newBase.Nullability == Nullability.Nullable) { - // T?! -> T? + // `T!` with substitution T=`U?` becomes `U?` // This happens during type substitution for generic methods. return newBase; } - return newBase.ChangeNullability(nullability); + if (newBase.Kind == TypeKind.TypeParameter || newBase.IsReferenceType == true) { + return newBase.ChangeNullability(nullability); + } else { + // `T!` with substitution T=`int` becomes `int`, not `int!` + return newBase; + } } else { return this; } diff --git a/ICSharpCode.Decompiler/TypeSystem/ParameterizedType.cs b/ICSharpCode.Decompiler/TypeSystem/ParameterizedType.cs index de7a25c2b..5dd907ace 100644 --- a/ICSharpCode.Decompiler/TypeSystem/ParameterizedType.cs +++ b/ICSharpCode.Decompiler/TypeSystem/ParameterizedType.cs @@ -140,10 +140,20 @@ namespace ICSharpCode.Decompiler.TypeSystem return b.ToString(); } } - + public override string ToString() { - return ReflectionName; + StringBuilder b = new StringBuilder(genericType.ToString()); + b.Append('['); + for (int i = 0; i < typeArguments.Length; i++) { + if (i > 0) + b.Append(','); + b.Append('['); + b.Append(typeArguments[i].ToString()); + b.Append(']'); + } + b.Append(']'); + return b.ToString(); } public IReadOnlyList TypeArguments => typeArguments;