Repository navigation
[clr-interp] Preserve byref-like locals when an await does not suspend - #136172
radekdoulik merged 1 commit into
Conversation
Check for a continuation before zeroing byref and byref-like locals in the shared interpreter handler. Preserve existing zeroing for actual suspension and cover generic/nongeneric methods with ref-struct and byref locals. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b691535f-9dc8-49e3-95fe-2e83af8ef957
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @agocke |
|
Tagging subscribers to this area: @JulieLeeMSFT, @BrzVlad, @janvorli |
|
/ba-g failures are unrelated
The four red checks reflect these two failures—not four regressions. |
2 participants
Summary
Move the existing no-continuation fast path ahead of local zeroing in the shared interpreter continuation handler.
For a synchronously completed await, the method does not suspend and must preserve its initialized locals. The existing order clears those locals before taking the no-continuation fast path.
For actual suspension, retain the existing zeroing before handling the continuation. Both the ordinary and generic continuation opcodes share this handler.
Regression coverage
Expand the existing
Runtime_136000.TestEntryPointinto a four-case conditional theory: generic/nongeneric containing type crossed with ref-struct/byref local. Every case checks preservation acrossTask.CompletedTask, an incomplete result for a pending task, and defaulting after actual suspension.Use a no-inline ref identity helper for the ordinary byref, analogous to the original payload's opaque initialization. This keeps a byref local represented so the test exercises continuation-local clearing rather than recomputation of a local's address. Checked interpreter dumps confirm both ordinary and generic continuation handlers are exercised. Existing feature conditions are retained; no new files or skips are added.
A direct-address prototype exposed a separate native-JIT address-rematerialization issue after suspension. The opaque initializer isolates the interpreter-local clearing path; this PR does not change JIT code or disable the byref cases.
The test was added by #136085, which fixes a different native-JIT continuation-capture problem. The interpreter's clearing-before-check behavior predates that test.
Validation
Built actual CoreCLR/browser-Wasm Checked runtimes and Release managed libraries from
d260b5c598ab285046e217c012f6d7126947b06a, then rebuilt the corrected native Wasm runtime and refreshed Core_Root. Also built the same-source macOS/arm64 Checked runtime for a native-JIT control.The final positive runs contain 60 passing selected test-case executions, zero selected failures, and zero selected skips. Filtered-out tests are not counted as executed coverage.
Wasm runs used Node.js v26.11.0 and the four
(DOTNET_ReadyToRun, DOTNET_TieredCompilation)settings(0,1),(0,0),(1,1), and(1,0). The broader coverage comprises nine selected async cases per setting: shared generics, byref/out parameters, array updates, void results, generic virtual awaits and exception propagation, and emitted runtime-async methods.The native control disables R2R and interpretation. Eight FullOpts ARM64 emitted-method assembly listings verify actual native JIT compilation. The final native and Wasm regression DLLs are byte-identical:
74ae40379b4da41bfbc31c56e87ea515398a13a4053238b599ab4355f62fecf3.Commands ran from the repository root with
DOTNET_WASM_TOOL_CACHE_DIR="$PWD/.dotnet/wasm-tools".The initial clean baseline hit an MSB3883 compiler-lock exception before any source edits. The one identical retry passed without a source or build-setting workaround; all listed successful build commands completed with zero warnings and errors.
Representative final regression invocations, with
rootset to the repository root and each run executed in a separate output directory:The merged runner can return process exit 100 even for failed tests. Results above come from the selected xUnit XML entries and exact-count checks, not process exit status alone. Broader browser async tests were run through their generated
async.shwrapper for host-side subprocess orchestration.Scope
The product correction only changes ordering in the shared interpreter handler. Real-suspension zeroing and continuation capture remain unchanged. No native JIT source, Reflection.Emit support, R2R folding, pipeline coverage, or reporting automation is changed.
Validation uses the actual browser/Wasm target through Node, not a Chrome tab or the exact CI binaries. The ReadyToRun settings above are environment controls, not verified R2R-compiled callers or composite CI payloads. No full async suite, Mono, or other native architectures are claimed.
Resolves #136171
Note
This pull request description was generated with GitHub Copilot.