#1292: Fix some more problems with pinned locals.

Let's distinguish between the original pinned locals and the PinnedRegion locals.
The format need declarations if any are left over after transformations; the latter don't.

ICSharpCode.Decompiler.Tests/TestCases/ILPretty/Unsafe.cs

@@ -25,6 +25,21 @@ internal sealed class ExtraUnsafeTests
 	{
 		return (uint*)Unsafe.AsPointer(ref managedPtr);
 	}
+
+	public unsafe static byte[] Issue1292(int val, byte[] arr)
+	{
+		//The blocks IL_0019 are reachable both inside and outside the pinned region starting at IL_0013. ILSpy has duplicated these blocks in order to place them both within and outside the `fixed` statement.
+		byte[] array;
+		if ((array = arr) != null && array.Length != 0) {
+			fixed (byte* ptr = &array[0]) {
+				*(int*)ptr = val;
+			}
+		} else {
+			/*pinned*/ref byte reference = ref *(byte*)null;
+			*(int*)Unsafe.AsPointer(ref reference) = val;
+		}
+		return arr;
+	}
 }
 
 namespace System.Runtime.CompilerServices

ICSharpCode.Decompiler.Tests/TestCases/ILPretty/Unsafe.il

@@ -474,4 +474,50 @@
     ldarg.0
     ret
   }
+
+  .method public hidebysig static 
+	  uint8[] Issue1292 (
+		  int32 val,
+		  uint8[] arr
+	  ) cil managed 
+  {
+	  .maxstack 2
+	  .locals init (
+		  [0] uint8[],
+		  [1] uint8[],
+		  [2] uint8& pinned
+	  )
+
+	  IL_0000: ldarg.1
+	  IL_0001: stloc.0
+	  IL_0002: ldloc.0
+	  IL_0003: dup
+	  IL_0004: stloc.1
+	  IL_0005: brfalse.s IL_0016
+
+	  IL_0007: ldloc.1
+	  IL_0008: ldlen
+	  IL_0009: conv.i4
+	  IL_000a: brfalse.s IL_0016
+
+	  IL_000c: ldloc.1
+	  IL_000d: ldc.i4.0
+	  IL_000e: ldelema [mscorlib]System.Byte
+	  IL_0013: stloc.2
+	  IL_0014: br.s IL_0019
+
+	  IL_0016: ldc.i4.0
+	  IL_0017: conv.u
+	  IL_0018: stloc.2
+
+	  IL_0019: ldloc.2
+	  IL_001a: conv.i
+	  IL_001b: ldarg.0
+	  IL_001c: stind.i4
+	  IL_001d: ldc.i4.0
+	  IL_001e: conv.u
+	  IL_001f: stloc.2
+	  IL_0020: ldloc.0
+	  IL_0021: ret
+  } // end of method Issue1292
 }

ICSharpCode.Decompiler/CSharp/Transforms/DeclareVariables.cs

@@ -308,7 +308,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
 		internal static bool VariableNeedsDeclaration(VariableKind kind)
 		{
 			switch (kind) {
-				case VariableKind.PinnedLocal:
+				case VariableKind.PinnedRegionLocal:
 				case VariableKind.Parameter:
 				case VariableKind.ExceptionLocal:
 				case VariableKind.ExceptionStackSlot:
@@ -468,6 +468,9 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
 					if (v.ILVariable.IsRefReadOnly && type is ComposedType composedType && composedType.HasRefSpecifier) {
 						composedType.HasReadOnlySpecifier = true;
 					}
+					if (v.ILVariable.Kind == VariableKind.PinnedLocal) {
+						type.InsertChildAfter(null, new Comment("pinned", CommentType.MultiLine), Roles.Comment);
+					}
 					var vds = new VariableDeclarationStatement(type, v.Name, assignment.Right.Detach());
 					var init = vds.Variables.Single();
 					init.AddAnnotation(assignment.Left.GetResolveResult());

ICSharpCode.Decompiler/IL/ControlFlow/DetectPinnedRegions.cs

@@ -83,7 +83,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 			}
 			this.context = null;
 		}
-		
+
 		/// <summary>
 		/// Ensures that every write to a pinned local is followed by a branch instruction.
 		/// This ensures the 'pinning region' does not involve any half blocks, which makes it easier to extract.
@@ -349,7 +349,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 				return false;
 			return block.Instructions[1].MatchBranch(nullOrEmptyBlock);
 		}
-		
+
 		bool IsNullSafeArrayToPointerNotNullAndNotEmptyBlock(Block block, ILVariable v, ILVariable p, Block targetBlock)
 		{
 			// Block B_not_null_and_not_empty {
@@ -380,7 +380,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 				return false;
 			return block.Instructions[1].MatchBranch(targetBlock);
 		}
-		
+
 		bool IsNullSafeArrayToPointerNullOrEmptyBlock(Block block, out ILVariable p, out Block targetBlock)
 		{
 			p = null;
@@ -490,7 +490,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 					body.Blocks.Add(innerBlock); // move block into body
 					if (!cloneBlocks) {
 						sourceContainer.Blocks[i] = new Block(); // replace with dummy block
-						// we'll delete the dummy block later
+																 // we'll delete the dummy block later
 					}
 				}
 			}
@@ -506,14 +506,24 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 				}
 				// Replace unreachable blocks in sourceContainer with dummy blocks:
 				bool[] isAlive = new bool[sourceContainer.Blocks.Count];
+				List<int> duplicatedBlockStartOffsets = new List<int>();
 				foreach (var remainingBlock in sourceContainer.TopologicalSort(deleteUnreachableBlocks: true)) {
 					isAlive[remainingBlock.ChildIndex] = true;
+					if (clonedBlocks[remainingBlock.ChildIndex] != null) {
+						duplicatedBlockStartOffsets.Add(remainingBlock.StartILOffset);
+					}
 				}
 				for (int i = 0; i < isAlive.Length; i++) {
 					if (!isAlive[i])
 						sourceContainer.Blocks[i] = new Block();
 				}
 				// we'll delete the dummy blocks later
+				Debug.Assert(duplicatedBlockStartOffsets.Count > 0);
+				duplicatedBlockStartOffsets.Sort();
+				context.Function.Warnings.Add("The blocks "
+					+ string.Join(", ", duplicatedBlockStartOffsets.Select(o => $"IL_{o:x4}"))
+					+ $" are reachable both inside and outside the pinned region starting at IL_{stLoc.StartILOffset:x4}."
+					+ " ILSpy has duplicated these blocks in order to place them both within and outside the `fixed` statement.");
 			}
 			if (body.Blocks.Count == 0) {
 				// empty body, the entryBlock itself doesn't belong into the pinned region
@@ -539,7 +549,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 			return inst.MatchLdcI4(0) || inst.MatchLdNull();
 		}
 		#endregion
-		
+
 		#region ProcessPinnedRegion
 		/// <summary>
 		/// After a pinned region was detected; process its body; replacing the pin variable
@@ -553,15 +563,14 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 				ILVariable oldVar = pinnedRegion.Variable;
 				IType elementType = ((ByReferenceType)oldVar.Type).ElementType;
 				if (elementType.Kind == TypeKind.Pointer && pinnedRegion.Init.MatchLdFlda(out _, out var field)
-					&& ((PointerType)elementType).ElementType.Equals(field.Type))
-				{
+					&& ((PointerType)elementType).ElementType.Equals(field.Type)) {
 					// Roslyn 2.6 (C# 7.2) uses type "int*&" for the pinned local referring to a
 					// fixed field of type "int".
 					// Remove the extra level of indirection.
 					elementType = ((PointerType)elementType).ElementType;
 				}
 				ILVariable newVar = new ILVariable(
-					VariableKind.PinnedLocal,
+					VariableKind.PinnedRegionLocal,
 					new PointerType(elementType),
 					oldVar.Index);
 				newVar.Name = oldVar.Name;
@@ -610,7 +619,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 			Debug.Assert(arrayToPointer.IsDescendantOf(pinnedRegion));
 			ILVariable oldVar = pinnedRegion.Variable;
 			ILVariable newVar = new ILVariable(
-				VariableKind.PinnedLocal,
+				VariableKind.PinnedRegionLocal,
 				new PointerType(((ArrayType)oldVar.Type).ElementType),
 				oldVar.Index);
 			newVar.Name = oldVar.Name;
@@ -680,7 +689,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 				// potentially a special case with legacy csc and an unused pinned variable:
 				if (pinnedRegion.Variable.AddressCount == 0 && pinnedRegion.Variable.LoadCount == 0) {
 					var charPtr = new PointerType(context.TypeSystem.FindType(KnownTypeCode.Char));
-					newVar = new ILVariable(VariableKind.PinnedLocal, charPtr, pinnedRegion.Variable.Index);
+					newVar = new ILVariable(VariableKind.PinnedRegionLocal, charPtr, pinnedRegion.Variable.Index);
 					newVar.Name = pinnedRegion.Variable.Name;
 					newVar.HasGeneratedName = pinnedRegion.Variable.HasGeneratedName;
 					pinnedRegion.Variable.Function.Variables.Add(newVar);
@@ -732,7 +741,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 				nativeVar = otherVar;
 			}
 			if (nativeVar.Kind == VariableKind.Local) {
-				newVar = new ILVariable(VariableKind.PinnedLocal, nativeVar.Type, nativeVar.Index);
+				newVar = new ILVariable(VariableKind.PinnedRegionLocal, nativeVar.Type, nativeVar.Index);
 				newVar.Name = nativeVar.Name;
 				newVar.HasGeneratedName = nativeVar.HasGeneratedName;
 				nativeVar.Function.Variables.Add(newVar);
@@ -801,7 +810,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow
 			if (v.Kind != VariableKind.Local)
 				return;
 			// replace V_1 with V_0
-			v.Kind = VariableKind.PinnedLocal;
+			v.Kind = VariableKind.PinnedRegionLocal;
 			pinnedRegion.Variable = v;
 			body.EntryPoint.Instructions.RemoveAt(0);
 		}

ICSharpCode.Decompiler/IL/ILVariable.cs

@@ -31,10 +31,14 @@ namespace ICSharpCode.Decompiler.IL
 		/// </summary>
 		Local,
 		/// <summary>
-		/// A pinned local variable
+		/// A pinned local variable (not associated with a pinned region)
 		/// </summary>
 		PinnedLocal,
 		/// <summary>
+		/// A pinned local variable (associated with a pinned region)
+		/// </summary>
+		PinnedRegionLocal,
+		/// <summary>
 		/// A local variable used as using-resource variable.
 		/// </summary>
 		UsingLocal,
@@ -88,6 +92,7 @@ namespace ICSharpCode.Decompiler.IL
 				case VariableKind.ForeachLocal:
 				case VariableKind.UsingLocal:
 				case VariableKind.PinnedLocal:
+				case VariableKind.PinnedRegionLocal:
 				case VariableKind.DisplayClassLocal:
 					return true;
 				default:
@@ -157,6 +162,7 @@ namespace ICSharpCode.Decompiler.IL
 				case VariableKind.Local:
 				case VariableKind.ForeachLocal:
 				case VariableKind.PinnedLocal:
+				case VariableKind.PinnedRegionLocal:
 				case VariableKind.UsingLocal:
 				case VariableKind.ExceptionLocal:
 				case VariableKind.DisplayClassLocal:
@@ -397,6 +403,9 @@ namespace ICSharpCode.Decompiler.IL
 				case VariableKind.PinnedLocal:
 					output.Write("pinned local ");
 					break;
+				case VariableKind.PinnedRegionLocal:
+					output.Write("PinnedRegion local ");
+					break;
 				case VariableKind.Parameter:
 					output.Write("param ");
 					break;
@@ -431,7 +440,7 @@ namespace ICSharpCode.Decompiler.IL
 			output.Write(" : ");
 			Type.WriteTo(output);
 			output.Write('(');
-			if (Kind == VariableKind.Parameter || Kind == VariableKind.Local || Kind == VariableKind.PinnedLocal) {
+			if (Kind == VariableKind.Parameter || Kind == VariableKind.Local || Kind == VariableKind.PinnedLocal || Kind == VariableKind.PinnedRegionLocal) {
 				output.Write("Index={0}, ", Index);
 			}
 			output.Write("LoadCount={0}, AddressCount={1}, StoreCount={2})", LoadCount, AddressCount, StoreCount);