Fix #2388: Add support for branch from catch-block to try-block (VB `On Error Resume Next`)

ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs

@@ -4408,7 +4408,7 @@ namespace ICSharpCode.Decompiler.CSharp
 
 		protected internal override TranslatedExpression VisitInvalidExpression(InvalidExpression inst, TranslationContext context)
 		{
-			string message = "Error";
+			string message = inst.Severity;
 			if (inst.StartILOffset != 0)
 			{
 				message += $" near IL_{inst.StartILOffset:x4}";

ICSharpCode.Decompiler/IL/BlockBuilder.cs

@@ -24,6 +24,7 @@ using System.Linq;
 using System.Reflection.Metadata;
 using System.Threading;
 
+using ICSharpCode.Decompiler.TypeSystem;
 using ICSharpCode.Decompiler.Util;
 
 namespace ICSharpCode.Decompiler.IL
@@ -32,6 +33,7 @@ namespace ICSharpCode.Decompiler.IL
 	{
 		readonly MethodBodyBlock body;
 		readonly Dictionary<ExceptionRegion, ILVariable> variableByExceptionHandler;
+		readonly ICompilation compilation;
 
 		/// <summary>
 		/// Gets/Sets whether to create extended basic blocks instead of basic blocks.
@@ -40,16 +42,19 @@ namespace ICSharpCode.Decompiler.IL
 		public bool CreateExtendedBlocks;
 
 		internal BlockBuilder(MethodBodyBlock body,
-							  Dictionary<ExceptionRegion, ILVariable> variableByExceptionHandler)
+							  Dictionary<ExceptionRegion, ILVariable> variableByExceptionHandler,
+							  ICompilation compilation)
 		{
 			Debug.Assert(body != null);
 			Debug.Assert(variableByExceptionHandler != null);
+			Debug.Assert(compilation != null);
 			this.body = body;
 			this.variableByExceptionHandler = variableByExceptionHandler;
+			this.compilation = compilation;
 		}
 
 		List<TryInstruction> tryInstructionList = new List<TryInstruction>();
-		Dictionary<int, BlockContainer> handlerContainers = new Dictionary<int, BlockContainer>();
+		readonly Dictionary<int, BlockContainer> handlerContainers = new Dictionary<int, BlockContainer>();
 
 		void CreateContainerStructure()
 		{
@@ -116,7 +121,7 @@ namespace ICSharpCode.Decompiler.IL
 
 		BlockContainer currentContainer;
 		Block currentBlock;
-		Stack<BlockContainer> containerStack = new Stack<BlockContainer>();
+		readonly Stack<BlockContainer> containerStack = new Stack<BlockContainer>();
 
 		public void CreateBlocks(BlockContainer mainContainer, List<ILInstruction> instructions, BitArray incomingBranches, CancellationToken cancellationToken)
 		{
@@ -196,6 +201,7 @@ namespace ICSharpCode.Decompiler.IL
 				FinalizeCurrentBlock(mainContainer.EndILOffset, fallthrough: false);
 			}
 			ConnectBranches(mainContainer, cancellationToken);
+			CreateOnErrorDispatchers();
 		}
 
 		static bool IsStackAdjustment(ILInstruction inst)
@@ -251,9 +257,13 @@ namespace ICSharpCode.Decompiler.IL
 					break;
 				case BlockContainer container:
 					containerStack.Push(container);
-					foreach (var block in container.Blocks)
+					// Note: FindBranchTarget()/CreateBranchTargetForOnErrorJump() may append to container.Blocks while we are iterating.
+					// Don't process those artificial blocks here.
+					int blockCount = container.Blocks.Count;
+					for (int i = 0; i < blockCount; i++)
 					{
 						cancellationToken.ThrowIfCancellationRequested();
+						var block = container.Blocks[i];
 						ConnectBranches(block, cancellationToken);
 						if (block.Instructions.Count == 0 || !block.Instructions.Last().HasFlag(InstructionFlags.EndPointUnreachable))
 						{
@@ -275,11 +285,111 @@ namespace ICSharpCode.Decompiler.IL
 			{
 				foreach (var block in container.Blocks)
 				{
-					if (block.StartILOffset == targetILOffset)
+					if (block.StartILOffset == targetILOffset && !block.ILRangeIsEmpty)
 						return block;
 				}
+				if (container.SlotInfo == TryCatchHandler.BodySlot)
+				{
+					// catch handler is allowed to branch back into try block (VB On Error)
+					TryCatch tryCatch = (TryCatch)container.Parent.Parent;
+					if (tryCatch.TryBlock.StartILOffset < targetILOffset && targetILOffset < tryCatch.TryBlock.EndILOffset)
+					{
+						return CreateBranchTargetForOnErrorJump(tryCatch, targetILOffset);
+					}
+				}
 			}
 			return null;
 		}
+
+		readonly Dictionary<TryCatch, OnErrorDispatch> onErrorDispatchers = new Dictionary<TryCatch, OnErrorDispatch>();
+
+		class OnErrorDispatch
+		{
+			public readonly ILVariable Variable;
+			public readonly HashSet<int> TargetILOffsets = new HashSet<int>();
+			public readonly List<Branch> Branches = new List<Branch>();
+
+			public OnErrorDispatch(ILVariable variable)
+			{
+				Debug.Assert(variable != null);
+				this.Variable = variable;
+			}
+		}
+
+		/// Create a new block that sets a helper variable and then branches to the start of the try-catch
+		Block CreateBranchTargetForOnErrorJump(TryCatch tryCatch, int targetILOffset)
+		{
+			if (!onErrorDispatchers.TryGetValue(tryCatch, out var dispatch))
+			{
+				var int32 = compilation.FindType(KnownTypeCode.Int32);
+				var newDispatchVar = new ILVariable(VariableKind.Local, int32, StackType.I4);
+				newDispatchVar.Name = $"try{tryCatch.StartILOffset:x4}_dispatch";
+				dispatch = new OnErrorDispatch(newDispatchVar);
+				onErrorDispatchers.Add(tryCatch, dispatch);
+			}
+			dispatch.TargetILOffsets.Add(targetILOffset);
+
+			Block block = new Block();
+			block.Instructions.Add(new StLoc(dispatch.Variable, new LdcI4(targetILOffset)));
+			var branch = new Branch(tryCatch.TryBlock.StartILOffset);
+			block.Instructions.Add(branch);
+			dispatch.Branches.Add(branch);
+			containerStack.Peek().Blocks.Add(block);
+			return block;
+		}
+
+		/// New variables introduced for the "on error" dispatchers
+		public IEnumerable<ILVariable> OnErrorDispatcherVariables => onErrorDispatchers.Values.Select(d => d.Variable);
+
+		void CreateOnErrorDispatchers()
+		{
+			foreach (var (tryCatch, dispatch) in onErrorDispatchers)
+			{
+				Block block = (Block)tryCatch.Parent;
+				// Before the regular entry point of the try-catch, insert an. instruction that resets the dispatcher variable
+				block.Instructions.Insert(tryCatch.ChildIndex, new StLoc(dispatch.Variable, new LdcI4(-1)));
+				// Split the block, so that we can introduce branches that jump directly into the try block
+				int splitAt = tryCatch.ChildIndex;
+				Block newBlock = new Block();
+				newBlock.AddILRange(tryCatch);
+				newBlock.Instructions.AddRange(block.Instructions.Skip(splitAt));
+				block.Instructions.RemoveRange(splitAt, block.Instructions.Count - splitAt);
+				block.Instructions.Add(new Branch(newBlock));
+				((BlockContainer)block.Parent).Blocks.Add(newBlock);
+				// Update the branches that jump directly into the try block
+				foreach (var b in dispatch.Branches)
+				{
+					b.TargetBlock = newBlock;
+				}
+
+				// Inside the try-catch, create the dispatch switch
+				BlockContainer tryBody = (BlockContainer)tryCatch.TryBlock;
+				Block dispatchBlock = new Block();
+				dispatchBlock.AddILRange(new Interval(tryCatch.StartILOffset, tryCatch.StartILOffset + 1));
+				var switchInst = new SwitchInstruction(new LdLoc(dispatch.Variable));
+				switchInst.AddILRange(new Interval(tryCatch.StartILOffset, tryCatch.StartILOffset + 1));
+				foreach (int offset in dispatch.TargetILOffsets)
+				{
+					var targetBlock = tryBody.Blocks.FirstOrDefault(b => b.StartILOffset == offset && !b.ILRangeIsEmpty);
+					ILInstruction branchInst;
+					if (targetBlock == null)
+					{
+						branchInst = new InvalidBranch("Could not find block for branch target "
+													+ Disassembler.DisassemblerHelpers.OffsetToString(offset));
+					}
+					else
+					{
+						branchInst = new Branch(targetBlock);
+					}
+					switchInst.Sections.Add(new SwitchSection { Labels = new LongSet(offset), Body = branchInst });
+				}
+				var usedLabels = new LongSet(dispatch.TargetILOffsets.Select(offset => LongInterval.Inclusive(offset, offset)));
+				switchInst.Sections.Add(new SwitchSection { Labels = usedLabels.Invert(), Body = new Branch(tryBody.EntryPoint) });
+				dispatchBlock.Instructions.Add(new InvalidExpression("ILSpy has introduced the following switch to emulate a goto from catch-block to try-block") { Severity = "Note" });
+				dispatchBlock.Instructions.Add(switchInst);
+
+				tryBody.Blocks.Insert(0, dispatchBlock);
+			}
+		}
 	}
 }

ICSharpCode.Decompiler/IL/ILReader.cs

@@ -578,13 +578,14 @@ namespace ICSharpCode.Decompiler.IL
 			cancellationToken.ThrowIfCancellationRequested();
 			Init(method, body, genericContext);
 			ReadInstructions(cancellationToken);
-			var blockBuilder = new BlockBuilder(body, variableByExceptionHandler);
+			var blockBuilder = new BlockBuilder(body, variableByExceptionHandler, compilation);
 			blockBuilder.CreateBlocks(mainContainer, instructionBuilder, isBranchTarget, cancellationToken);
 			var function = new ILFunction(this.method, body.GetCodeSize(), this.genericContext, mainContainer, kind);
 			function.Variables.AddRange(parameterVariables);
 			function.Variables.AddRange(localVariables);
 			function.Variables.AddRange(stackVariables);
 			function.Variables.AddRange(variableByExceptionHandler.Values);
+			function.Variables.AddRange(blockBuilder.OnErrorDispatcherVariables);
 			function.AddRef(); // mark the root node
 			var removedBlocks = new List<Block>();
 			foreach (var c in function.Descendants.OfType<BlockContainer>())

ICSharpCode.Decompiler/IL/Instructions/SimpleInstruction.cs

@@ -89,6 +89,7 @@ namespace ICSharpCode.Decompiler.IL
 
 	partial class InvalidExpression : SimpleInstruction
 	{
+		public string Severity = "Error";
 		public string? Message;
 		public StackType ExpectedResultType = StackType.Unknown;