You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Addresses #133193 by enabling aggressively trimmed browser CoreCLR library tests and adding separate LibraryTestsCoreCLR_R2R Chrome CI lanes. Uses the existing browser trimming configuration and descriptors; no Apple-mobile descriptor imports are added.
Introduce test-scoped TestWasmReadyToRun, avoiding global PublishReadyToRun=true reaching host-side build tools.
Pass identical R2R/trimming properties through build and Helix submission, with distinct test-run names. Existing interpreter lanes remain separate; the R2R lane selects the full supported inner-loop project set. R2R jobs have a 480-minute timeout; the interpreter default remains 240 minutes.
Set TEST_READY_TO_RUN_MODE=1 and use PlatformDetection.IsWasmReadyToRun for browser/WASI-scoped quarantines, preserving interpreter coverage.
Keep only TestUtilities.dll interpreted for the open platform-probe issue; CoreLib, libraries, and test assemblies remain R2R.
Remove stale .ilproj Link metadata from the TestILAssembly reference so its DLL is staged with the correct VFS name.
Preserve function-table/RuntimeFunctions ordering for WebAssembly virtual-dispatch thunks so async resume fixups resolve generated R2R resume stubs correctly.
Document the local workflow and retain WasmBuildNativeImplicitInReleaseConfiguration=false.
Integrated prerequisites and fixes
The original browser R2R publishing prerequisite #133378 has landed on main and is incorporated by the main merges in this branch. Other landed fixes incorporated here include:
#133146 introduced virtual-dispatch thunks into the Wasm function table before compiled methods and transition thunks. Existing resumption fixups intentionally encode relative function-table indices and correlate them with RuntimeFunctions ordinals. The inserted table-only thunk shifted those spaces apart.
The correct adjacent generated <Resume> stub had the expected four-argument signature. The fix moves WasmVirtualDispatchThunkNode after compiled method and transition-thunk nodes, restoring the invariant without changing dispatch codegen or adding steady-state overhead. New compiler regression coverage validates both ResumptionStubEntryPoint and StoreMultiCallableAddrOfCode targets in the presence of virtual dispatch.
R2RDump/Nesm maps async resume fixups to the generated <Resume> method with (i32,i32,i32,i32)->i32, proving native R2R resume execution rather than interpreter fallback.
Current full smoke after the revised fix has completed successfully for System.Runtime, HTTP, WebSockets, JavaScript interop, and Threading; the initial Tasks run reached only the known #133627 failure. After restoring that one exclusion, the standalone full Tasks project passed as reported above.
The full supported R2R library selection is still running at the time of this update. Results will be added before the draft is marked ready.
Earlier descriptor-ablation comparisons ran full UnmanagedMemoryStream, FileSystem, Cryptography, Data.Common, Globalization, and LINQ suites in both actual EAT and trimmed R2R with the original browser roots: 136,138 passed, 1,141 skipped, zero failed per configuration, with identical counts. No Apple-only roots or replacement descriptors were required.
ActiveIssue-filtered cases are omitted from totals rather than counted as skipped. No performance claim is made; the ordering fix changes deterministic table placement only and has no steady-state dispatch cost.
Note
This PR description and changes were prepared with GitHub Copilot assistance.
Azure Pipelines:
Successfully started running 6 pipeline(s).
10 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.
@AndyAyersMS are the AsyncProfilerTests expected to work on wasm, the session disabled them individually but I can make it blanket if preferred?
I'm not sure what surface these cover. If they rely on the ResumeInfo.DiagnosticIP then they're not going to work now and possibly for quite some time.
)
Fixes#133617.
## Problem
The Wasm portable-entrypoint slow path retains a `PrestubMethodFrame`
while invoking a resolved R2R body. Function-only stack walks can report
both the real managed activation and that transition frame.
`MethodBase.GetCurrentMethod` can therefore accept its own prestub frame
as the caller.
A Checked-runtime capture confirmed this: the stack marker was
`0x4fe71c`; the real `GetCurrentMethod` body at SP `0x4fe710` did not
pass the caller-marker comparison, but its explicit `PrestubMethodFrame`
at SP `0x4fe720` did. The result was `GetCurrentMethod` instead of the
calling method. This also reproduces without trimming.
#133610 addressed the duplicate in `GetStackFramesCallback`, but
caller-sensitive reflection uses a different callback and did not
benefit from that filter.
## Change
- Mark completed non-FCall Wasm prestub transitions as not representing
an additional managed activation.
- Filter those frames centrally for `FUNCTIONSONLY` walks, covering both
ordinary and skipped explicit frames.
- Retain the frame, MethodDesc, argument-root reporting, and physical
unwind/exception lifetime. Popping early or clearing the MethodDesc
would lose GC protection and the unwind bridge across interpreter-to-R2R
transitions.
- Replace #133610's StackTrace-specific adjacency filter. Filtering by
lifecycle rather than matching method identities preserves genuine
recursion.
- Extend the existing `github60486` regression with
direct/inlineable/generic caller identity, assembly lookup, recursive
frame counts, and compacting-GC object/byref/struct argument coverage.
Its original DIM frame-sequence and exception-stack checks remain
intact.
Native FCalls retain their existing representation. Non-Wasm filtering
behavior is unchanged.
## Validation
### CoreCLR runtime regression
Built browser-Wasm and macOS arm64 Checked runtimes with Release
libraries. The expanded
`Loader/classloader/DefaultInterfaceMethods/regressions/github60486`
passes with fixed Wasm R2R, the interpreter, and native arm64. The
identical final regression assembly fails against the saved original
Wasm runtime: expected `ValidateCurrentMethod`, actual
`GetCurrentMethod`.
These fixed runs include the original regression checks from #133610,
with its `debugdebugger.cpp` filter removed.
### Original browser library lane
Validated the exact patch in an isolated checkout of #133656 at
`282d12dfe220a1f34d0bb81a85239cab60c779ce`, which supplies the browser
R2R publishing/test prerequisites. Built matched original/fixed CoreCLR
Release products locally and ran Chromium 153. Temporarily enabled only
the nine #133617-quarantined methods, preserving every original
assertion and unrelated exclusion; all annotations were restored
afterward. This PR contains no library-test quarantine changes.
| Coverage | Original | Fixed |
|---|---:|---:|
| Cold trimmed direct GetCurrentMethod | 1 failed | 1 passed |
| Cold trimmed inlineable GetCurrentMethod | 1 failed | 1 passed |
| Trimmed targeted batch | 7 passed, 1 failed | 8 passed |
| Untrimmed R2R targeted batch | 8 passed, 1 failed | 9 passed |
| Full trimmed System.Runtime | 77,396 passed, 1 failed, 175 skipped |
77,397 passed, 0 failed, 175 skipped |
The affected-class fixed run also passed: 23 passed, 3 existing skips.
`CrossAssembly2` remains excluded when trimmed by dotnet/linker#2078 and
passes untrimmed; it is not counted as a trimmed pass.
Main test IL and main R2R images are byte-identical across each
original/fixed pair; skip sets are identical. Served assets were
verified against their respective rebuilt packs. This is a
matched-product A/B, not a native-module-only swap: CoreLib R2R image
bytes also differ.
### Existing reflection compatibility controls
`System.Reflection.Tests` caller-assembly theory, real cross-assembly
delegate invocation, static-constructor caller lookup,
executing-assembly lookup, and reflected first/second invocation all
pass: six cases on each original/fixed, trimmed/untrimmed browser R2R
configuration. Fresh-process delegate and reflective-invocation probes
also pass in all four configurations. All existing annotations remained
intact. These are compatibility controls, not failing-before
reproductions.
Full browser reflection suites have identical original/fixed results:
- Untrimmed: 1,750 passed, 1 failed, 23 skipped.
- Trimmed: 1,747 passed, 2 failed, 23 skipped.
Existing failures are `GetEntryAssembly` (browser host expectation
versus R2R expectation) and, trimmed only,
`AssemblyGetForwardedTypesLoadFailure` (underlying trimming cause not
isolated). Failure signatures and skip sets match before/after; neither
was suppressed.
Native arm64 Checked reflection control: six focused cases passed; full
suite 1,776 passed, 2 platform skips, zero failures.
## Remaining scope
Draft for review of the frame-lifecycle distinction and FCall exception.
Dedicated FCall cold/warm stack-identity coverage, a broader CoreCLR
GC-stress campaign, and other browsers have not been run. No performance
claim is made.
> [!NOTE]
> This change and pull request description were developed with GitHub
Copilot assistance.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
WASI R2R runs do not set TEST_READY_TO_RUN_MODE, so the new IsWasmReadyToRun gates are not activated there.
Review effort: Lite Findings: None
Previously missed (1)
In code that hasn't changed since last review
Set TEST_READY_TO_RUN_MODE for CoreCLR WASI R2R tests
eng/testing/tests.browser.targets:105
PlatformDetection.IsWasmReadyToRun covers both browser and WASI, but tests.wasi.targets imports tests.wasm.targets directly and never imports this browser-specific file. Consequently a CoreCLR WASI R2R test app does not set TEST_READY_TO_RUN_MODE=1, so all of the new IsWasmReadyToRun quarantines (and the GenericMathTestMemberData filtering) remain active instead of being applied. Add the equivalent conditional --engine-arg=--env setup to the WASI harness so the documented browser/WASI gate is accurate.
IsWasm includes both Browser and WASI (IsBrowser || IsWasi), but TEST_READY_TO_RUN_MODE is injected only by eng/testing/tests.browser.targets; the WASI targets import the common wasm targets and have no equivalent environment entry. A WASI CoreCLR R2R test would therefore report IsWasmReadyToRun == false and run the new WASI-scoped quarantines instead of suppressing them. Add the same marker to the WASI test targets, or narrow this predicate to Browser so its scope matches the setup.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
The new WASI-capable R2R predicate is backed only by browser environment propagation, so WASI R2R quarantines remain inactive.
Review effort: Lite Findings: None
Previously missed (1)
In code that hasn't changed since last review
WASI R2R tests miss TEST_READY_TO_RUN_MODE setup
eng/testing/tests.browser.targets:105
IsWasmReadyToRun includes IsWasi, but this is the only place that sets TEST_READY_TO_RUN_MODE, and tests.wasi.targets imports tests.wasm.targets directly without an equivalent environment export. A WASI CoreCLR R2R test process therefore reports IsReadyToRunCompiled == false, so the new ActiveIssue(... IsWasmReadyToRun) gates do not apply there. Move the setup to a shared WASM target or add the corresponding WASI --env path.
[!NOTE]
This review comment was generated with GitHub Copilot.
Enable WASI ReadyToRun mode in wasmtime test arguments
eng/testing/tests.wasi.targets:154
PlatformDetection.IsWasmReadyToRun includes WASI, but tests.wasi.targets never adds TEST_READY_TO_RUN_MODE to the wasmtime command line (the WasmEnvironmentVariable item used by the browser boot config is not consumed here). Any CoreCLR WASI R2R run therefore sees IsWasmReadyToRun == false, so the new WASI-scoped ActiveIssue conditions never activate and R2R-specific gating is incorrect. Add --engine-arg=--env --engine-arg=TEST_READY_TO_RUN_MODE=1 to _XHarnessArgs under the CoreCLR + PublishReadyToRun condition.
IsWasmReadyToRun is defined in terms of IsWasm (browser or WASI), but this is the only library-test path that sets TEST_READY_TO_RUN_MODE; tests.wasi.targets has no equivalent. A WASI R2R test app will therefore evaluate IsWasmReadyToRun as false and run the new shared quarantines instead of filtering them. Add the matching environment item to the WASI targets, or narrow the predicate/documentation to browser-only.
[!NOTE] This review comment was generated with GitHub Copilot.
<ItemGroup Condition="'$(TargetOS)' == 'browser' and '$(RuntimeFlavor)' == 'CoreCLR' and '$(TestWasmReadyToRun)' == 'true' and '$(PublishReadyToRun)' != 'false' and '$(RunDisabledWasmTests)' != 'true'">
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
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.
dotnet-policy-service
Bot
added
the
linkable-framework
Summary
Addresses #133193 by enabling aggressively trimmed browser CoreCLR library tests and adding separate
LibraryTestsCoreCLR_R2RChrome CI lanes. Uses the existing browser trimming configuration and descriptors; no Apple-mobile descriptor imports are added.TestWasmReadyToRun, avoiding globalPublishReadyToRun=truereaching host-side build tools.TEST_READY_TO_RUN_MODE=1and usePlatformDetection.IsWasmReadyToRunfor browser/WASI-scoped quarantines, preserving interpreter coverage.TestUtilities.dllinterpreted for the open platform-probe issue; CoreLib, libraries, and test assemblies remain R2R..ilprojLink metadata from the TestILAssembly reference so its DLL is staged with the correct VFS name.WasmBuildNativeImplicitInReleaseConfiguration=false.Integrated prerequisites and fixes
The original browser R2R publishing prerequisite #133378 has landed on main and is incorporated by the main merges in this branch. Other landed fixes incorporated here include:
IsDynamicCodeCompiledtrimming correction.Function-table ordering fix
#133146 introduced virtual-dispatch thunks into the Wasm function table before compiled methods and transition thunks. Existing resumption fixups intentionally encode relative function-table indices and correlate them with RuntimeFunctions ordinals. The inserted table-only thunk shifted those spaces apart.
Nesm decoded the live failure before the fix:
The correct adjacent generated
<Resume>stub had the expected four-argument signature. The fix movesWasmVirtualDispatchThunkNodeafter compiled method and transition-thunk nodes, restoring the invariant without changing dispatch codegen or adding steady-state overhead. New compiler regression coverage validates both ResumptionStubEntryPoint and StoreMultiCallableAddrOfCode targets in the presence of virtual dispatch.Remaining quarantines
TestUtilities.dllstays interpreted; product/test assemblies remain R2R. Fix PR #133773 is still open.RuntimeAsync_WhenAny_TracksAllBranchesremains excluded for its independent memory-OOB transition failure.Local validation
macOS arm64 host, browser-wasm CoreCLR Release, Chromium 154.
Rebuilt Release tasks and matched browser CoreCLR, crossgen2, CoreLib, libraries, host, and packs after merging main and applying the ordering fix.
Focused fix validation supplied and independently integrated:
RuntimeAsync_CallstackEmittedOnResume: 1/1 passed.RuntimeAsync_YieldAtEachLevel_CallstackShrinks: 1/1 passed.WasmVirtualDispatch+RuntimeAsyncWasmDiagnosticIPFixups: 2/2 passed.System.Threading.Tests: 549 passed, 87 skipped, zero failed.System.Threading.Tasks.Testswith only [wasm][R2R] StateMachineAsync profiler callstack identities do not resolve #134145 and [browser][CoreCLR][R2R] RuntimeAsync_WhenAny_TracksAllBranches traps in an R2R-to-interpreter transition #133627 retained: 375 passed, 430 skipped, zero failed.<Resume>method with(i32,i32,i32,i32)->i32, proving native R2R resume execution rather than interpreter fallback.Current full smoke after the revised fix has completed successfully for System.Runtime, HTTP, WebSockets, JavaScript interop, and Threading; the initial Tasks run reached only the known #133627 failure. After restoring that one exclusion, the standalone full Tasks project passed as reported above.
The full supported R2R library selection is still running at the time of this update. Results will be added before the draft is marked ready.
Earlier descriptor-ablation comparisons ran full UnmanagedMemoryStream, FileSystem, Cryptography, Data.Common, Globalization, and LINQ suites in both actual EAT and trimmed R2R with the original browser roots: 136,138 passed, 1,141 skipped, zero failed per configuration, with identical counts. No Apple-only roots or replacement descriptors were required.
ActiveIssue-filtered cases are omitted from totals rather than counted as skipped. No performance claim is made; the ordering fix changes deterministic table placement only and has no steady-state dispatch cost.
Note
This PR description and changes were prepared with GitHub Copilot assistance.