Fix #2032: avoid propagating mutated SROA fields - #3929
Open
siegfriedpammer wants to merge 1 commit into
Open
Conversation
Aggressive scalar replacement propagated a display-class field to its source variable even when the field is mutated after initialization, aliasing two distinct source-level variables (Test9: thisField and this). Propagation is now cancelled when the field sees a second store or its address escapes, but only for propagation targets that cannot absorb the store: 'this' and variables that are themselves scalar-replaced display classes. Parameters continue to propagate, because their remaining uses are already restricted by ResolveVariableToPropagate and a captured parameter mutated inside a lambda (DelegateConstruction's Bug951) must keep mapping to the parameter. Checking CanPropagate first also keeps the guard away from Mono state-machine fields, whose VariableToDeclare is pre-bound to a state-machine variable that Propagate(null) would discard. Re-enables Test9 and adds Test10 covering the escaping-address variant (Interlocked.Exchange(ref displayClass.thisField, ...)). Assisted-by: OpenCode:openai/gpt-5.5:OpenCode Assisted-by: Claude:claude-fable-5:Claude Code
siegfriedpammer
force-pushed
the
fix-2032-tdcu-copy-propagation
branch
from
July 29, 2026 04:35
d4a1818 to
3e956e0
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens the correctness conditions for aggressive SROA in TransformDisplayClassUsage by preventing propagation of display-class fields back to certain source variables when the field is mutated again (additional stores or escaping address), and updates the Ugly SROA regression tests to cover the fixed behavior.
Changes:
- Detect post-initialization mutations/escapes of captured fields and disable propagation when it cannot be safely redirected (notably for
thisand scalar-replaced display-class variables). - Add a small helper (
IsReadOnlyOrInitializerUse) to distinguish plain reads / recorded initializer stores from later mutations. - Re-enable the prior
Test9regression case and addTest10covering an address-escape mutation viaInterlocked.Exchange.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| ICSharpCode.Decompiler/IL/Transforms/TransformDisplayClassUsage.cs | Adds a guard that disables unsafe propagation when a captured field is mutated after initialization, plus a helper to classify field uses. |
| ICSharpCode.Decompiler.Tests/TestCases/Ugly/AggressiveScalarReplacementOfAggregates.Expected.cs | Updates expected decompiler output: re-enables Test9 and adds Test10 (Interlocked-based mutation case). |
| ICSharpCode.Decompiler.Tests/TestCases/Ugly/AggressiveScalarReplacementOfAggregates.cs | Updates input test case source: re-enables Test9 and adds Test10 with Interlocked.Exchange. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #2032
Testing