From cedd8a6bf66781fd74246a7487a512c2eaecadb8 Mon Sep 17 00:00:00 2001
From: Daniel Grunwald <daniel@danielgrunwald.de>
Date: Tue, 30 Nov 2021 21:06:38 +0100
Subject: [PATCH] Fix #2480: Avoid yield-return decompilation if there are
 unrecognized state assignments in a finally method.

---
 .../IL/ControlFlow/YieldReturnDecompiler.cs   | 49 +++++++++++++++----
 1 file changed, 39 insertions(+), 10 deletions(-)

diff --git a/ICSharpCode.Decompiler/IL/ControlFlow/YieldReturnDecompiler.cs b/ICSharpCode.Decompiler/IL/ControlFlow/YieldReturnDecompiler.cs
index 724aaf51c..099247f61 100644
--- a/ICSharpCode.Decompiler/IL/ControlFlow/YieldReturnDecompiler.cs
+++ b/ICSharpCode.Decompiler/IL/ControlFlow/YieldReturnDecompiler.cs
@@ -135,14 +135,16 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 				ConstructExceptionTable();
 				newBody = AnalyzeMoveNext(function);
 			}
-			catch (SymbolicAnalysisFailedException)
+			catch (SymbolicAnalysisFailedException ex)
 			{
+				function.Warnings.Add($"yield-return decompiler failed: {ex.Message}");
 				return;
 			}
 
 			context.Step("Replacing body with MoveNext() body", function);
 			function.IsIterator = true;
 			function.StateMachineCompiledWithMono = isCompiledWithMono;
+			var oldBody = function.Body;
 			function.Body = newBody;
 			// register any locals used in newBody
 			function.Variables.AddRange(newBody.Descendants.OfType<IInstructionWithVariableOperand>().Select(inst => inst.Variable).Distinct());
@@ -172,14 +174,28 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 			newBody.SortBlocks(deleteUnreachableBlocks: true);
 			function.CheckInvariant(ILPhase.Normal);
 
-			if (isCompiledWithMono)
+			try
 			{
-				CleanSkipFinallyBodies(function);
+				if (isCompiledWithMono)
+				{
+					CleanSkipFinallyBodies(function);
+				}
+				else
+				{
+					DecompileFinallyBlocks();
+					ReconstructTryFinallyBlocks(function);
+				}
 			}
-			else
+			catch (SymbolicAnalysisFailedException ex)
 			{
-				DecompileFinallyBlocks();
-				ReconstructTryFinallyBlocks(function);
+				// Revert the yield-return transformation
+				context.Step("Transform failed, roll it back", function);
+				function.IsIterator = false;
+				function.StateMachineCompiledWithMono = false;
+				function.Body = oldBody;
+				function.Variables.RemoveDead();
+				function.Warnings.Add($"yield-return decompiler failed: {ex.Message}");
+				return;
 			}
 
 			context.Step("Translate fields to local accesses", function);
@@ -336,7 +352,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 			var block = body as Block;
 			if (body is BlockContainer blockContainer && blockContainer.Blocks.Count == 1)
 			{
-				block = blockContainer.Blocks.Single() as Block;
+				block = blockContainer.Blocks.Single();
 			}
 			return block;
 		}
@@ -431,11 +447,11 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 		{
 			var metadata = context.PEFile.Metadata;
 			if (method.IsNil)
-				throw new SymbolicAnalysisFailedException();
+				throw new SymbolicAnalysisFailedException("Method not found");
 
 			var methodDef = metadata.GetMethodDefinition(method);
 			if (!methodDef.HasBody())
-				throw new SymbolicAnalysisFailedException();
+				throw new SymbolicAnalysisFailedException($"Method {methodDef.Name} has no body");
 
 			GenericContext genericContext = context.Function.GenericContext;
 			genericContext = new GenericContext(
@@ -465,7 +481,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 				&& metadata.GetString(metadata.GetMethodDefinition(m).Name).EndsWith(".get_Current", StringComparison.Ordinal));
 			Block body = SingleBlock(CreateILAst(getCurrentMethod, context).Body);
 			if (body == null)
-				throw new SymbolicAnalysisFailedException();
+				throw new SymbolicAnalysisFailedException("get_Current has no body");
 			if (body.Instructions.Count == 1)
 			{
 				// release builds directly return the current field
@@ -1052,6 +1068,12 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 				{
 					body.EntryPoint.Instructions.RemoveAt(0);
 				}
+				// Avoid yield-return decompilation if there are unrecognized state assignments in a finally method.
+				foreach (var inst in body.Descendants)
+				{
+					if (IsStateAssignment(inst))
+						throw new SymbolicAnalysisFailedException($"Unknown state transition in {function.Name ?? "finally"} at IL_{inst.StartILOffset:x4}");
+				}
 				function.ReleaseRef(); // make body reusable outside of function
 				decompiledFinallyMethods.Add(method, (newState, function));
 			}
@@ -1202,6 +1224,13 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 			}
 		}
 
+		bool IsStateAssignment(ILInstruction inst)
+		{
+			return inst.MatchStFld(out var target, out var field, out _)
+				&& target.MatchLdThis()
+				&& field.MemberDefinition.Equals(stateField);
+		}
+
 		// Gets the state that is transitioned to at the start of the block
 		int? GetNewState(Block block)
 		{