Skip to content

Fix #3320: detect dynamic await in pre-Roslyn state machines - #4132

Open
siegfriedpammer wants to merge 1 commit into
masterfrom
fix/3320-legacy-dynamic-await
Open

Fix #3320: detect dynamic await in pre-Roslyn state machines#4132
siegfriedpammer wants to merge 1 commit into
masterfrom
fix/3320-legacy-dynamic-await

Conversation

@siegfriedpammer

Copy link
Copy Markdown
Member

An await on a dynamic value in an assembly built by the C# 5 compiler was
left undecompiled: the state machine struct and its call-site container leaked
into the output as <GetAsync>d__d<T> and <GetAsync>o__SiteContainer6<T>.

The C# 5 compiler loads the awaiter into a fresh local before every dynamic call
site and before the ICriticalNotifyCompletion type test, and the early
non-aggressive inlining refuses to fold those copies. Every await matcher
identifies an await by the identity of its awaiter variable, so each copy hid
the awaiter from the matcher.

Each such copy has one store and one load, and once the dynamic call sites are
collapsed that load sits in the next instruction - the case
ILInlining.InlineOne already handles, including the check that the source is
not overwritten first. Folding is restricted to the IsCompleted / GetResult
call sites and the completion-interface type tests, so dynamic calls in user
code keep their locals; a blanket fold retypes unrelated locals of the
surrounding method (an enum local decompiled as int plus a cast).

Two smaller divergences from the Roslyn shape sat behind that one:

  • the merge block of the AwaitOnCompleted/AwaitUnsafeOnCompleted diamond
    clears doFinallyBodies before its leave, where Roslyn's is a bare leave;
  • the awaiter is restored from its object-typed field with unbox.any rather
    than castclass.

The engine change is +60/-13 in AsyncAwaitDecompiler.cs; the rest of the diff
is the ildasm fixture.

Verification

  • The regression test is the state machine from the issue's assembly with its
    external types stubbed out, so it runs where the legacy compiler does not.
  • Checked against real legacy csc output (/o- and /o+): dynamic await in
    plain code, loops, try/catch, try/finally and using all decompile.
  • Async/await test surface green (127 tests).
  • Both InlineAwaiterCopies call sites were tested for redundancy by removing
    each in turn; either removal leaves the state machine undecompiled. The first
    has to run before detection in AnalyzeStateMachine, the second only becomes
    possible once coalescing has put the call site and its awaiter copy in one
    block.

Known coverage gap

The doFinallyBodies merge-block handling is required for dynamic await inside
try/finally or using - verified against real legacy csc - but the committed
fixture does not exercise that path, so there is no red test behind those lines.
Reviewers may want a second fixture for it.


This pull request was prepared by an AI agent (Claude Opus 5 via Claude Code)
on behalf of the repository owner.

{
for (int i = block.Instructions.Count - 1; i >= 0; i--)
{
if (block.Instructions[i] is StLoc { Variable: { Kind: VariableKind.Local, IsSingleDefinition: true, LoadCount: 1, AddressCount: 0 } copy, Value: LdLoc } store

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Suggested change
if (block.Instructions[i] is StLoc { Variable: { Kind: VariableKind.Local, IsSingleDefinition: true, LoadCount: 1, AddressCount: 0 } copy, Value: LdLoc } store
if (block.Instructions[i] is StLoc { Variable: { Kind: VariableKind.Local, IsSingleDefinition: true, LoadCount: 1 } copy, Value: LdLoc } store

IsSingleDefinition implies AddressCount == 0

Reviewed with Stampeded!

The C# 5 compiler loads the awaiter into a fresh local before every
dynamic call site and before the ICriticalNotifyCompletion type test, and
the early non-aggressive inlining refuses to fold those copies. Every
await matcher identifies an await by the identity of its awaiter
variable, so each copy hid the awaiter from the matcher and the whole
await stayed undetected.

Each such copy has one store and one load, and after the dynamic call
sites are collapsed that load sits in the next instruction, which is the
case ILInlining.InlineOne already handles, including the check that the
source is not overwritten first. Folding is restricted to the IsCompleted
and GetResult call sites and the completion-interface type tests, so
dynamic calls in user code keep their locals: a blanket fold retypes
unrelated locals of the surrounding method (an enum local decompiled as
int plus a cast).

Two smaller divergences from the Roslyn shape sat behind that one: the
merge block of the AwaitOnCompleted/AwaitUnsafeOnCompleted diamond
clears doFinallyBodies before its leave, and the awaiter is restored
from its object-typed field with unbox.any rather than castclass.

The regression test is the state machine from the issue's assembly with
its external types stubbed out, so it runs where the legacy compiler
does not. Checked against real legacy csc output (/o- and /o+): dynamic
await in plain code, loops, try/catch, try/finally and using.

Assisted-by: Claude:claude-opus-5[1m]:Claude Code
@siegfriedpammer
siegfriedpammer force-pushed the fix/3320-legacy-dynamic-await branch from 821923b to 96456e9 Compare September 14, 2026 04:21
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.

1 participant