Skip to content

Handle negative dictionary capacity in string switch transform - #3878

Merged
siegfriedpammer merged 1 commit into
icsharpcode:masterfrom
ds5678:issue3877
Jul 12, 2026
Merged

Handle negative dictionary capacity in string switch transform#3878
siegfriedpammer merged 1 commit into
icsharpcode:masterfrom
ds5678:issue3877

Conversation

@ds5678

@ds5678 ds5678 commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Problem

Resolves #3877

Solution

  • Any comments on the approach taken, its consistency with surrounding code, etc.
  • Which part of this PR is most in need of attention/improvement?
  • At least one test covering the code changed

Added Issue3877 test to PrettyTestRunner and new test case source to verify dictionary initialization with negative capacity. Updated SwitchOnStringTransform to skip processing when a negative dictionary capacity is detected.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a decompiler crash in the string-switch dictionary-pattern transform when the dictionary constructor is given a negative capacity, by treating that pattern as non-matchable and adding a regression test to ensure decompilation completes successfully.

Changes:

  • Prevent SwitchOnStringTransform.ExtractStringValuesFromInitBlock from constructing a List with a negative capacity by rejecting negative valuesLength.
  • Add a new Pretty test case (Issue3877) that includes a negative dictionary capacity in the legacy string-switch pattern.
  • Register the new test in PrettyTestRunner so it runs across Roslyn 2+ (including net40 variants).

Reviewed changes

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

File Description
ICSharpCode.Decompiler/IL/Transforms/SwitchOnStringTransform.cs Adds a guard to avoid List<T>(capacity) throwing on negative capacities during pattern matching.
ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue3877.cs New regression test input/expected-output file reproducing the negative-capacity pattern.
ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs Adds the Issue3877 NUnit test to ensure coverage across relevant compiler option sets.

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

@siegfriedpammer
siegfriedpammer merged commit e3fbca9 into icsharpcode:master Jul 12, 2026
7 checks passed
@siegfriedpammer

Copy link
Copy Markdown
Member

Thank you very much for the contribution!

@ds5678
ds5678 deleted the issue3877 branch July 12, 2026 18:54
siegfriedpammer added a commit that referenced this pull request Jul 13, 2026
MatchSwitchOnCharBlock's case 2 and default paths guarded against a
negative character index, but case 1 (a bare switch on get_Chars) did
not. Crafted IL whose get_Chars/get_Item index is negative - a value no
compiler emits, but valid IL - therefore reached the pattern
reconstruction unchecked. For a length-1 group this silently miscompiled
the switch (it rebuilds the string switch from the char labels without
using the index), turning IL that reads s[-1] into `switch (s)`; for
longer strings it threw IndexOutOfRangeException and aborted the method.

Move the check into MatchGetChars so all three call sites reject a
negative index by construction, and drop the two now-redundant guards.

Same class of unvalidated-integer robustness issue as #3878, in a
different switch-on-string pattern.

Assisted-by: Claude:claude-opus-4-8:Claude Code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Negative length can make switch on string with dictionary transformation throw an exception

3 participants