From cbe8dd43d75a6b17c1b377632f0586c96d37a8ef Mon Sep 17 00:00:00 2001 From: ds5678 <49847914+ds5678@users.noreply.github.com> Date: Sun, 9 Mar 2025 21:00:36 -0700 Subject: [PATCH 1/3] Fix null check in MatchLegacySwitchOnStringWithDict Updated the condition for `nullValueCaseBlock` to ensure it is not null and not equal to `defaultBlock`. --- ICSharpCode.Decompiler/IL/Transforms/SwitchOnStringTransform.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ICSharpCode.Decompiler/IL/Transforms/SwitchOnStringTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/SwitchOnStringTransform.cs index 8302038b8..9f7752951 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/SwitchOnStringTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/SwitchOnStringTransform.cs @@ -668,7 +668,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms if (!FixCasesWithoutValue(sections, stringValues)) return false; // switch contains case null: - if (nullValueCaseBlock != defaultBlock) + if (nullValueCaseBlock != null && nullValueCaseBlock != defaultBlock) { if (!AddNullSection(sections, stringValues, nullValueCaseBlock)) { From a1b3b14b0b5967bd4e9588790012829690a2eb79 Mon Sep 17 00:00:00 2001 From: ds5678 <49847914+ds5678@users.noreply.github.com> Date: Sat, 15 Mar 2025 00:51:53 -0700 Subject: [PATCH 2/3] Add test --- .../ICSharpCode.Decompiler.Tests.csproj | 2 + .../ILPrettyTestRunner.cs | 6 + .../TestCases/ILPretty/Issue3421.cs | 28 +++++ .../TestCases/ILPretty/Issue3421.il | 116 ++++++++++++++++++ 4 files changed, 152 insertions(+) create mode 100644 ICSharpCode.Decompiler.Tests/TestCases/ILPretty/Issue3421.cs create mode 100644 ICSharpCode.Decompiler.Tests/TestCases/ILPretty/Issue3421.il diff --git a/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj b/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj index 3ff44e2bb..fc8734e1a 100644 --- a/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj +++ b/ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj @@ -104,6 +104,7 @@ + @@ -129,6 +130,7 @@ + diff --git a/ICSharpCode.Decompiler.Tests/ILPrettyTestRunner.cs b/ICSharpCode.Decompiler.Tests/ILPrettyTestRunner.cs index 04c94d089..5a86d4e81 100644 --- a/ICSharpCode.Decompiler.Tests/ILPrettyTestRunner.cs +++ b/ICSharpCode.Decompiler.Tests/ILPrettyTestRunner.cs @@ -207,6 +207,12 @@ namespace ICSharpCode.Decompiler.Tests await Run(); } + [Test] + public async Task Issue3421() + { + await Run(); + } + [Test] public async Task Issue2260SwitchString() { diff --git a/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/Issue3421.cs b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/Issue3421.cs new file mode 100644 index 000000000..d09c674fd --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/Issue3421.cs @@ -0,0 +1,28 @@ +internal class Issue3421 +{ + private string name; + private object value; + + public virtual void SetValue(object value) + { + if (name == null) + { + return; + } + switch (name) + { + case "##Name##": + return; + case "##Value##": + this.value = value; + return; + case "##InnerText##": + this.value = value.ToString(); + return; + } + if (this.value == null) + { + this.value = ""; + } + } +} diff --git a/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/Issue3421.il b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/Issue3421.il new file mode 100644 index 000000000..541c8094b --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/Issue3421.il @@ -0,0 +1,116 @@ +#define CORE_ASSEMBLY "System.Runtime" + +.assembly extern CORE_ASSEMBLY +{ + .publickeytoken = (B0 3F 5F 7F 11 D5 0A 3A ) // .?_....: + .ver 4:0:0:0 +} + +.class private auto ansi beforefieldinit Issue3421 + extends [CORE_ASSEMBLY]System.Object +{ + // Fields + .field private string name + .field private object 'value' + .field private static class [CORE_ASSEMBLY]System.Collections.Generic.Dictionary`2 '<>f__switch$map1D' + .custom instance void [CORE_ASSEMBLY]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( + 01 00 00 00 + ) + + // Methods + .method public hidebysig virtual + instance void SetValue ( + object 'value' + ) cil managed + { + // Method begins at RVA 0x2050 + // Header size: 12 + // Code size: 180 (0xb4) + .maxstack 27 + .locals init ( + [0] string, + [1] class [CORE_ASSEMBLY]System.Collections.Generic.Dictionary`2, + [2] int32 + ) + + IL_0000: ldarg.0 + IL_0001: ldfld string Issue3421::name + IL_0006: stloc.0 + IL_0007: ldloc.0 + IL_0008: brfalse IL_0093 + + IL_000d: ldsfld class [CORE_ASSEMBLY]System.Collections.Generic.Dictionary`2 Issue3421::'<>f__switch$map1D' + IL_0012: brtrue IL_0048 + + IL_0017: ldc.i4.3 + IL_0018: newobj instance void class [CORE_ASSEMBLY]System.Collections.Generic.Dictionary`2::.ctor(int32) + IL_001d: stloc.1 + IL_001e: ldloc.1 + IL_001f: ldstr "##Name##" + IL_0024: ldc.i4.0 + IL_0025: callvirt instance void class [CORE_ASSEMBLY]System.Collections.Generic.Dictionary`2::Add(!0, !1) + IL_002a: ldloc.1 + IL_002b: ldstr "##Value##" + IL_0030: ldc.i4.1 + IL_0031: callvirt instance void class [CORE_ASSEMBLY]System.Collections.Generic.Dictionary`2::Add(!0, !1) + IL_0036: ldloc.1 + IL_0037: ldstr "##InnerText##" + IL_003c: ldc.i4.2 + IL_003d: callvirt instance void class [CORE_ASSEMBLY]System.Collections.Generic.Dictionary`2::Add(!0, !1) + IL_0042: ldloc.1 + IL_0043: stsfld class [CORE_ASSEMBLY]System.Collections.Generic.Dictionary`2 Issue3421::'<>f__switch$map1D' + + IL_0048: ldsfld class [CORE_ASSEMBLY]System.Collections.Generic.Dictionary`2 Issue3421::'<>f__switch$map1D' + IL_004d: ldloc.0 + IL_004e: ldloca.s 2 + IL_0050: callvirt instance bool class [CORE_ASSEMBLY]System.Collections.Generic.Dictionary`2::TryGetValue(!0, !1&) + IL_0055: brfalse IL_0098 + + IL_005a: ldloc.2 + IL_005b: switch (IL_0071, IL_0076, IL_0082) + + IL_006c: br IL_0098 + + IL_0071: br IL_00b3 + + IL_0076: ldarg.0 + IL_0077: ldarg.1 + IL_0078: stfld object Issue3421::'value' + IL_007d: br IL_00b3 + + IL_0082: ldarg.0 + IL_0083: ldarg.1 + IL_0084: callvirt instance string [CORE_ASSEMBLY]System.Object::ToString() + IL_0089: stfld object Issue3421::'value' + IL_008e: br IL_00b3 + + IL_0093: br IL_00b3 + + IL_0098: ldarg.0 + IL_0099: ldfld object Issue3421::'value' + IL_009e: brtrue IL_00ae + + IL_00a3: ldarg.0 + IL_00a4: ldstr "" + IL_00a9: stfld object Issue3421::'value' + + IL_00ae: br IL_00b3 + + IL_00b3: ret + } // end of method Issue3421::SetValue + + .method public hidebysig specialname rtspecialname + instance void .ctor () cil managed + { + // Method begins at RVA 0x2110 + // Header size: 1 + // Code size: 8 (0x8) + .maxstack 8 + + IL_0000: ldarg.0 + IL_0001: call instance void [CORE_ASSEMBLY]System.Object::.ctor() + IL_0006: nop + IL_0007: ret + } // end of method Issue3421::.ctor + +} // end of class Issue3421 From e4000c8a5c74fabe6f1cf5051a1cfdd9e99b466f Mon Sep 17 00:00:00 2001 From: ds5678 <49847914+ds5678@users.noreply.github.com> Date: Sat, 15 Mar 2025 01:29:58 -0700 Subject: [PATCH 3/3] Enhance null handling in switch transformations - Updated `Issue3421.cs`. - Updated `MatchLegacySwitchOnStringWithDict` to check for `leaveContainer` and handle null sections accordingly. - Introduced an overload for `AddNullSection` to accept `ILInstruction` as the body, improving flexibility. - Modified existing `AddNullSection` to utilize the new overload, allowing for varied body types in `SwitchSection`. --- .../TestCases/ILPretty/Issue3421.cs | 6 ++---- .../IL/Transforms/SwitchOnStringTransform.cs | 14 +++++++++++++- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/Issue3421.cs b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/Issue3421.cs index d09c674fd..1acc63fac 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/Issue3421.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/ILPretty/Issue3421.cs @@ -5,10 +5,6 @@ internal class Issue3421 public virtual void SetValue(object value) { - if (name == null) - { - return; - } switch (name) { case "##Name##": @@ -19,6 +15,8 @@ internal class Issue3421 case "##InnerText##": this.value = value.ToString(); return; + case null: + return; } if (this.value == null) { diff --git a/ICSharpCode.Decompiler/IL/Transforms/SwitchOnStringTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/SwitchOnStringTransform.cs index 9f7752951..f39ff3c86 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/SwitchOnStringTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/SwitchOnStringTransform.cs @@ -675,6 +675,13 @@ namespace ICSharpCode.Decompiler.IL.Transforms return false; } } + else if (leaveContainer != null && !defaultBlockJump.MatchLeave(leaveContainer)) + { + if (!AddNullSection(sections, stringValues, (Leave)exitBlockJump)) + { + return false; + } + } context.Step(nameof(MatchLegacySwitchOnStringWithDict), instructions[i]); bool keepAssignmentBefore = false; if (switchValueVar.LoadCount > 2 || switchValue == null) @@ -741,6 +748,11 @@ namespace ICSharpCode.Decompiler.IL.Transforms } bool AddNullSection(List sections, List<(string Value, int Index)> stringValues, Block nullValueCaseBlock) + { + return AddNullSection(sections, stringValues, new Branch(nullValueCaseBlock)); + } + + bool AddNullSection(List sections, List<(string Value, int Index)> stringValues, ILInstruction body) { var label = new LongSet(stringValues.Max(item => item.Index) + 1); var possibleConflicts = sections.Where(sec => sec.Labels.Overlaps(label)).ToArray(); @@ -753,7 +765,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms possibleConflicts[0].Labels = possibleConflicts[0].Labels.ExceptWith(label); } stringValues.Add((null, (int)label.Values.First())); - sections.Add(new SwitchSection() { Labels = label, Body = new Branch(nullValueCaseBlock) }); + sections.Add(new SwitchSection() { Labels = label, Body = body }); return true; }