Skip to content

Fix RoslynAnalyzer crash when analyzing indexer properties in .NET 10.0#121603

Merged
sbomer merged 6 commits intomainfrom
copilot/fix-roslynanalyzer-crash
Nov 19, 2025
Merged

Fix RoslynAnalyzer crash when analyzing indexer properties in .NET 10.0#121603
sbomer merged 6 commits intomainfrom
copilot/fix-roslynanalyzer-crash

Conversation

Copy link
Contributor

Copilot AI commented Nov 13, 2025

Our analysis framework is designed for analyzing parameters of methods (since it's analyzing based on IL semantics), but the Roslyn operation visitor gives us an operation block for the parameter initializers of properties (indexers). These appear like any other operation block, and don't seem to provide a way to short-circuit the analysis early.

This fixes the issue at the leaf when we visit a parameter reference that's not owned by a method. In this case it doesn't make sense to look for the associated getter/setter because the assignment operation is on the property declaration and has no direct equivalent in IL.

Fixes #121546

Copilot AI self-assigned this Nov 13, 2025
Handle case where IParameterSymbol.ContainingSymbol is IPropertySymbol for indexer parameters.
When the parameter's containing symbol is a property, get the appropriate accessor method (getter or setter).
Use SymbolEqualityComparer for symbol comparison as required by analyzer rules.

Co-authored-by: sbomer <[email protected]>
@sbomer sbomer marked this pull request as ready for review November 14, 2025 18:36
@sbomer sbomer requested a review from marek-safar as a code owner November 14, 2025 18:36
@sbomer sbomer requested review from a team and Copilot November 14, 2025 18:36
Copy link
Contributor

Copilot AI left a comment

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 crash in the RoslynAnalyzer when analyzing indexer properties with default parameter values in .NET 10.0. The analyzer framework is designed for analyzing method parameters based on IL semantics, but Roslyn's operation visitor provides operation blocks for property parameter initializers (indexers), which don't have direct IL equivalents.

Key Changes:

  • Added a null check in GetParameterTargetValue to handle parameters not associated with methods, returning TopValue instead of crashing
  • Added comprehensive test coverage for indexer properties with default arguments

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimAnalysisVisitor.cs Adds null check for parameters not owned by methods (e.g., indexer default parameters) to prevent crash
src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/PropertyDataFlow.cs Adds test case IndexerDefaultArgument to verify the fix handles indexer default parameters correctly

Copy link
Member

@jtschuster jtschuster left a comment

Choose a reason for hiding this comment

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

This will fix the crash but we should file an issue for indexer parameters with DAM if we don't fix it here.

- Add clarifying comment
- Add more test coverage
@sbomer sbomer merged commit 4ab6b05 into main Nov 19, 2025
78 checks passed
@sbomer
Copy link
Member

sbomer commented Nov 20, 2025

/backport to release/10.0

@github-actions
Copy link
Contributor

Started backporting to release/10.0 (link to workflow run)

@github-actions
Copy link
Contributor

@sbomer backporting to release/10.0 failed, the patch most likely resulted in conflicts. Please backport manually!

git am output
$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch

Patch format detection failed.
Error: The process '/usr/bin/git' failed with exit code 128

Link to workflow output

Copilot AI added a commit that referenced this pull request Nov 20, 2025
@akoeplinger akoeplinger deleted the copilot/fix-roslynanalyzer-crash branch November 21, 2025 12:07
agocke pushed a commit that referenced this pull request Dec 5, 2025
#121861)

Backport of #121603 to
release/10.0

## Customer Impact

- [x] Customer reported
- [ ] Found internally

Reported by customer in #121546

## Regression

- [x] Yes
- [] No

Regressed in .NET 10 by analyzer changes that touched related code.

## Testing

Verified on the provided repro.

## Risk

Low. Adds an early return to the analysis in a place that doesn't have
downstream effects.


<hr />


Backports #121603 to release/10.0.
Fixes #121546.

The RoslynAnalyzer crashes when analyzing indexer property parameter
initializers because it assumes all parameters belong to methods, but
Roslyn provides operation blocks for property indexer parameters that
don't have a direct IL equivalent.

## Changes

- **TrimAnalysisVisitor.cs**: Guard against null `parameterMethod` in
`GetParameterTargetValue`. Returns `TopValue` for parameters not owned
by methods (e.g., indexer parameter initializers), preventing crashes
while preserving correct analysis for method parameters.

- **PropertyDataFlow.cs**: Add test coverage for annotated indexer
parameters and indexers with default arguments to verify the analyzer
handles these cases without crashing.

```csharp
// Previously crashed on this pattern:
public Type this[[DynamicallyAccessedMembers(...)] Type index]
{
    get { ... }
    set { ... }
}
```

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: sbomer <[email protected]>
@github-actions github-actions bot locked and limited conversation to collaborators Dec 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

linkable-framework Issues associated with delivering a linker friendly framework

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RoslynAnalyzer crashes after changing target framework to net10.0

4 participants