From 8d65375c76c27ee003bbfe22e1a3a8c639d5d30a Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Fri, 7 Aug 2026 17:56:26 +0200 Subject: [PATCH] Delay exceptions when copy-propagating an address into a StObj target Below C# 7 ref locals are unavailable, so CopyPropagation is allowed to copy LdFlda/LdElema. When such a copy lands in a StObj target slot whose value is impure, it violates the invariant checked by StObj.CheckTargetSlot: C# computes the value to be stored before dereferencing the target, so the exception moves. ILInlining resolves the same conflict by marking the address as delayed rather than falling back to a ref local; copy propagation now does the same, which keeps the generated code unchanged and only repairs the IL. Unlike inlining, copy propagation has no third arm to fall back to: by the time DoPropagate runs, the defining store is about to disappear, so every load has to be replaced and refusing the copy is no longer an option. That decision can only be made up front, which is what CanPerformCopyPropagation does when ref locals are requested. The assertion covers the remaining hole, the public Propagate() entry point, which bypasses that check -- AsyncAwaitDecompiler copies an ldflda of the builder field through it irrespective of the setting. Propagating an address also un-inlines its arguments into fresh stack slots, and those stores are copy-propagation candidates in their own right. They are inserted before the store being replaced, so the block scan used to step right past them: a slot loaded more than once could never be inlined back and survived into the output as a ref local -- for `s.ShortField >>>= 5` the copied ldflda left behind `stloc C_0(ldloca s)`, printed as `ref CustomStruct reference = ref s`. Rewinding the scan to the first of those stores lets them propagate too. Assisted-by: Claude:claude-opus-5[1m]:Claude Code --- .../ICSharpCode.Decompiler.Tests.csproj | 2 + .../Ugly/NoUnsignedRightShift.Expected.cs | 30 +++++++++++ .../TestCases/Ugly/NoUnsignedRightShift.cs | 52 +++++++++++++++++++ .../UglyTestRunner.cs | 6 +++ .../IL/Transforms/CopyPropagation.cs | 29 +++++++++++ 5 files changed, 119 insertions(+) create mode 100644 ICSharpCode.Decompiler.Tests/TestCases/Ugly/NoUnsignedRightShift.Expected.cs create mode 100644 ICSharpCode.Decompiler.Tests/TestCases/Ugly/NoUnsignedRightShift.cs diff --git a/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj b/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj index 29015d86a..3e05ecf6e 100644 --- a/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj +++ b/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj @@ -253,6 +253,8 @@ + + diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Ugly/NoUnsignedRightShift.Expected.cs b/ICSharpCode.Decompiler.Tests/TestCases/Ugly/NoUnsignedRightShift.Expected.cs new file mode 100644 index 000000000..597834121 --- /dev/null +++ b/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); + } + } +} diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Ugly/NoUnsignedRightShift.cs b/ICSharpCode.Decompiler.Tests/TestCases/Ugly/NoUnsignedRightShift.cs new file mode 100644 index 000000000..b01d6a1d4 --- /dev/null +++ b/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; + } + } +} diff --git a/ICSharpCode.Decompiler.Tests/UglyTestRunner.cs b/ICSharpCode.Decompiler.Tests/UglyTestRunner.cs index 6256ec52a..4dde3afa0 100644 --- a/ICSharpCode.Decompiler.Tests/UglyTestRunner.cs +++ b/ICSharpCode.Decompiler.Tests/UglyTestRunner.cs @@ -144,6 +144,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) { diff --git a/ICSharpCode.Decompiler/IL/Transforms/CopyPropagation.cs b/ICSharpCode.Decompiler/IL/Transforms/CopyPropagation.cs index f55bee55a..c2898ed71 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/CopyPropagation.cs +++ b/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); + } } } }