Merge pull request #3964 from icsharpcode/fix-copypropagation-stobj-target

Delay exceptions when copy-propagating an address into a StObj target

ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj

@@ -274,6 +274,8 @@
     <None Include="TestCases\Ugly\NoNewOfT.Expected.cs" />
     <Compile Remove="TestCases\Ugly\NoPropertiesAndEvents.Expected.cs" />
     <None Include="TestCases\Ugly\NoPropertiesAndEvents.Expected.cs" />
+    <Compile Remove="TestCases\Ugly\NoUnsignedRightShift.Expected.cs" />
+    <None Include="TestCases\Ugly\NoUnsignedRightShift.Expected.cs" />
     <!-- top-level statements: neither the input nor the expected output can be compiled
          into the test assembly, which is a library and has its own entry point rules -->
     <Compile Remove="TestCases\Ugly\TopLevelProgram.cs" />

ICSharpCode.Decompiler.Tests/TestCases/Ugly/NoUnsignedRightShift.Expected.cs

@@ -0,0 +1,30 @@
+namespace ICSharpCode.Decompiler.Tests.TestCases.Ugly
+{
+	internal class NoUnsignedRightShift
+	{
+		public struct CustomStruct
+		{
+			public short ShortField;
+		}
+
+		public class CustomClass
+		{
+			public short ShortField;
+		}
+
+		public static void ClassField(CustomClass c)
+		{
+			c.ShortField = (short)((uint)c.ShortField >> 5);
+		}
+
+		public static void StructField(CustomStruct s)
+		{
+			s.ShortField = (short)((uint)s.ShortField >> 5);
+		}
+
+		public static void ArrayElement(short[] a)
+		{
+			a[0] = (short)((uint)a[0] >> 5);
+		}
+	}
+}

ICSharpCode.Decompiler.Tests/TestCases/Ugly/NoUnsignedRightShift.cs

@@ -0,0 +1,52 @@
+// Copyright (c) 2026 Siegfried Pammer
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy of this
+// software and associated documentation files (the "Software"), to deal in the Software
+// without restriction, including without limitation the rights to use, copy, modify, merge,
+// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons
+// to whom the Software is furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in all copies or
+// substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
+// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR
+// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE
+// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+// DEALINGS IN THE SOFTWARE.
+
+namespace ICSharpCode.Decompiler.Tests.TestCases.Ugly
+{
+	// The unsigned right shift operator requires C# 11, so at a lower language version the
+	// compound assignments below have to be expanded into a plain assignment. That turns the
+	// stored value into a non-pure expression, which the target of the store must not be
+	// copy-propagated across.
+	internal class NoUnsignedRightShift
+	{
+		public struct CustomStruct
+		{
+			public short ShortField;
+		}
+
+		public class CustomClass
+		{
+			public short ShortField;
+		}
+
+		public static void ClassField(CustomClass c)
+		{
+			c.ShortField >>>= 5;
+		}
+
+		public static void StructField(CustomStruct s)
+		{
+			s.ShortField >>>= 5;
+		}
+
+		public static void ArrayElement(short[] a)
+		{
+			a[0] >>>= 5;
+		}
+	}
+}

ICSharpCode.Decompiler.Tests/UglyTestRunner.cs

@@ -174,6 +174,12 @@ namespace ICSharpCode.Decompiler.Tests
 			});
 		}
 
+		[Test]
+		public async Task NoUnsignedRightShift([ValueSource(nameof(roslynOnlyOptions))] CompilerOptions cscOptions)
+		{
+			await RunForLibrary(cscOptions: cscOptions, decompilerSettings: new DecompilerSettings(CSharp.LanguageVersion.CSharp6));
+		}
+
 		[Test]
 		public async Task NoNewOfT([ValueSource(nameof(defaultOptions))] CompilerOptions cscOptions)
 		{

ICSharpCode.Decompiler/IL/Transforms/CopyPropagation.cs

@@ -16,6 +16,7 @@
 // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
 // DEALINGS IN THE SOFTWARE.
 
+using System;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;
@@ -145,6 +146,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 		static void DoPropagate(ILVariable v, ILInstruction copiedExpr, Block block, ref int i, ILTransformContext context)
 		{
 			context.Step($"Copy propagate {v.Name}", copiedExpr);
+			int firstUninlinedArg = i;
 			// un-inline the arguments of the ldArg instruction
 			ILVariable[] uninlinedArgs = new ILVariable[copiedExpr.Children.Count];
 			for (int j = 0; j < uninlinedArgs.Length; j++)
@@ -168,11 +170,38 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 				}
 				// We are copying an expression from far away, reusing the ILRange would result in incorrect sequence points.
 				clone.SetILRange(new Interval());
+				if (expr.SlotInfo == StObj.TargetSlot && clone.HasDirectFlag(InstructionFlags.MayThrow)
+					&& !expr.Parent.SatisfiesSlotRestrictionForInlining(expr.ChildIndex, clone))
+				{
+					// A LdFlda/LdElema used as StObj target has to delay its exception, because C#
+					// computes the value to be stored before dereferencing the target. Accept the
+					// changed point at which the exception is thrown, mirroring
+					// InliningOptions.AllowChangingOrderOfEvaluationForExceptions in ILInlining.
+					// Unlike inlining, copy propagation has no third option of just giving up: the
+					// defining store is gone by the end of this loop, so every load must be replaced.
+					// Refusing the copy is only possible before propagation starts, which is what
+					// CanPerformCopyPropagation does when ref locals are requested; the assert
+					// guards the public Propagate() entry point, which bypasses that check.
+					Debug.Assert(!context.Settings.UseRefLocalsForAccurateOrderOfEvaluation);
+					if (clone is LdFlda ldflda)
+						ldflda.DelayExceptions = true;
+					else if (clone is LdElema ldelema)
+						ldelema.DelayExceptions = true;
+				}
 				expr.ReplaceWith(clone);
 			}
 			block.Instructions.RemoveAt(i);
 			int c = ILInlining.InlineInto(block, i, InliningOptions.None, context: context);
 			i -= c + 1;
+			if (uninlinedArgs.Length > 0)
+			{
+				// The stores holding the un-inlined arguments are themselves copy-propagation
+				// candidates: copying an ldflda leaves its target address behind in a stack slot,
+				// and if that address is e.g. an ldloca it can be copied as well. They sit before
+				// the current position, so rewind far enough for the caller's loop to visit them;
+				// otherwise a multiply-loaded stack slot survives as a spurious ref local.
+				i = Math.Min(i, firstUninlinedArg - 1);
+			}
 		}
 	}
 }