Reject negative char index in the length-and-char string switch

[siegfriedpammer]

Problem

Follow-up to #3878, which fixed a negative-dictionary-capacity crash in the string-switch transform. The same class of bug — an unvalidated negative integer read from IL — exists in a different switch-on-string pattern: the Roslyn 4.6 "switch on length and char" reconstruction.

In MatchSwitchOnCharBlock, case 2 and default guard the character index with if (index < 0) return false;, but case 1 (a bare switch on get_Chars) does not. IL whose get_Chars/get_Item index is negative — e.g. s[-1], a value no compiler emits in this pattern but perfectly valid IL — reaches the reconstruction unchecked:

• length-1 group → the branch that runs never uses index; it rebuilds the string switch from the char labels alone, so IL that reads s[-1] is silently miscompiled into switch (s) (semantically wrong — the IL throws on any 1-char string, the output does not).
• longer strings → stringValue[index] throws IndexOutOfRangeException, aborting decompilation of the whole method.

Fix

Move the non-negative check into MatchGetChars (both the get_Chars and ReadOnlySpan/Span.get_Item branches), so all three call sites reject a negative index by construction, and drop the two now-redundant per-case guards.

Test

ILPretty/SwitchOnStringNegativeCharIndex — real Roslyn length-1 switch-on-string codegen with a single instruction edited (ldc.i4.0 → ldc.i4.m1). Without the fix the expected output diff shows the miscompiled return s switch { … }; with it, the decompiler correctly declines and preserves switch (s[-1]).

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR hardens the string-switch transform for the Roslyn “switch on length and char” pattern by rejecting negative get_Chars / Span.get_Item indices at match time, preventing both silent miscompilation (length==1 path) and decompilation failures (longer strings).

Changes:

• Enforce index >= 0 inside MatchGetChars for both System.String.get_Chars and ReadOnlySpan/Span.get_Item.
• Remove now-redundant negative-index guards from MatchSwitchOnCharBlock cases that call MatchGetChars.
• Add an ILPretty regression fixture + test that edits the get_Chars index to -1 and verifies the transform declines and preserves switch (s[-1]).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
ICSharpCode.Decompiler/IL/Transforms/SwitchOnStringTransform.cs	Centralizes non-negative index validation in MatchGetChars and removes redundant per-case checks.
ICSharpCode.Decompiler.Tests/TestCases/ILPretty/SwitchOnStringNegativeCharIndex.il	Adds a regression IL fixture that uses a negative get_Chars index to exercise the bug.
ICSharpCode.Decompiler.Tests/TestCases/ILPretty/SwitchOnStringNegativeCharIndex.cs	Adds the expected decompiler output, ensuring the transform declines and preserves switch (s[-1]).
ICSharpCode.Decompiler.Tests/ILPrettyTestRunner.cs	Registers the new ILPretty test case in the test runner.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.