Skip to content

Fix cDAC stress comparison of stack byrefs - #135124

Closed
max-charlamb wants to merge 1 commit into
dotnet:mainfrom
max-charlamb:max-charlamb/fix-cdac-stress-stack-byrefs
Closed

max-charlamb wants to merge 1 commit into
dotnet:mainfrom
max-charlamb:max-charlamb/fix-cdac-stress-stack-byrefs

Conversation

@max-charlamb

Copy link
Copy Markdown
Member

Summary

Fix the cDAC stress oracle's asymmetric handling of interior references into the live stack.

PromoteCarefully normally excludes ordinary stack byrefs from native GC callbacks. Embedded byrefs in byref-like arguments can instead be reported directly. The stress harness filtered stack interiors only from the cDAC side, producing false PrestubMethodFrame mismatches for stack-backed Spans on Windows x86. The cDAC had reported the reference correctly.

Rather than filter both sides and lose coverage, retain raw stack-byref reporting:

  • Allow PromoteCarefully to report live-stack interiors only to the private stress collector callback.
  • Remove the comparison-side stack-interior filter.
  • Add ByRefArguments to both stress theories, covering stack/heap Spans and an ordinary out bool. Lazy signature resolution triggers allocations while the prestub is live; the receiver also allocates and runs a real GC.

Real GC callbacks keep their existing behavior. The hook is guarded by CDAC_STRESS and excluded from DAC builds; it does not change ScanContext or the GC/EE interface. This PR is independent of the transition-frame unwind fix in #135112.

Validation

  • Windows x86 Checked native runtime build and Release win-x86 NativeAOT cDAC publish succeeded.
  • With filtering removed but without the collector bypass, the new debuggee produced 3,824 failures / 4,642 GC-reference comparisons.
  • On this isolated PR branch, using cDAC rebuilt without [cDAC] Fix x86 transition-frame stack-pop handling #135112: 4,656 / 4,656 GC-reference comparisons passed and 320 argument-iterator checks passed, with zero failures, known issues, skips, or errors in their respective summaries. Both debuggee processes exited 100 and required the assembly-resolution callback to execute.
  • Earlier targeted GC-reference controls also passed: NotYetLoadedArgType (4,656), CallSignatures (5,220), and BasicAlloc (4,620).

The full stress suite and non-x86 execution have not been run locally.

Resolves #135086

Note

This PR description and code were prepared with GitHub Copilot.

Report live-stack interiors only to the diagnostic collector and remove asymmetric cDAC filtering. Add prestub Span and out-bool regression coverage while preserving real GC behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 593f81fe-227d-480b-9da0-38ddeea24fd9
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 5 pipeline(s).
11 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

@pcshrosbree

Copy link
Copy Markdown
Contributor

I traced this against main (44253fe) and agree with the root cause.

On x86, a PrestubMethodFrame is scanned with PromoteCallerStack, which goes to MetaSig::GcScanRoots. A by-value byref-like argument goes to ReportPointersFromValueTypeArg, then to ByRefPointerOffsetsReporter::Report. That calls fn directly with GC_CALL_INTERIOR (siginfo.cpp:5223), so the PromoteCarefully stack check never runs. The cDAC emits an Interior token for the same field (CallingConvention_1.cs:467). FilterInteriorStackRefs then removes it from the cDAC side only. On Windows x64 a 16-byte span is passed by reference, so both sides drop it and no mismatch appears.

The CI failures match this. I read the Helix consoles of builds 1629310, 1632709, 1630078 and 1630648. All 20 failing verifications are PrestubMethodFrame [ONLY(RT)] with Flags=0x1; they contain 10 distinct frame/slot/object address tuples, each reported at two different IPs. In every one, Addr is the frame + 0x84 (in the transition block's argument area) and Obj is the frame + 0x1dc. That puts Obj on the same thread's stack, above the frame. Both PromoteCarefully and the filter drop references in that range. The cDAC did scan the frame (known_nie=0). The stack is always NativeRuntimeEventSource..cctor → TypeNameParser.Parse(ReadOnlySpan<char>) → TypeNameResolver.GetTypeHelper(ReadOnlySpan<char>) → RuntimeCustomAttribute, which is CoreLib startup. That is why the failing debuggee varies from run to run (DynamicMethods, PInvoke, Generics, Comprehensive, InlineArrayByRefLike, DeepStack). It is not specific to NotYetLoadedArgType.

The fix looks right to me. The bypass compares against the file-static collector callback, is compiled only under CDAC_STRESS (debug and checked), and sits inside the existing non-DAC block, so real GC callbacks are unchanged. Removing the filter also makes the comparison stronger. Before, every live-stack interior on the cDAC side was discarded, including GcInfo and GCRefMap interiors. Now the collector retains GcInfo interiors through GcEnumObject → PromoteCarefully and GCRefMap interiors through the GCREFMAP_INTERIOR case → PromoteCarefully, so both sides compare them.

Two requests:

  • Could you show ByRefArguments failing on main with the filter still in place? The run reporting 3,824/4,642 had the filter removed and no bypass; main still has the filter. That run therefore does not show that the new debuggee catches [ci-scan] Test failure: GCRefStress_AllVerificationsPass misses a Windows x86 PrestubMethodFrame root #135086 on main.
  • Optional: could you note in the description that the change applies on every stress platform? The description correctly names Windows x86 as the platform where the failure was observed. From the code, by-value spans are also reported directly on arm64 and, through ReportPointersFromStructInRegisters, on Unix x64; the failure logs show only Windows x86.

A three-way merge check against main at dcc404c shows no conflicts; I have not built or tested the merged tree. Current main includes #135112 and #135890; #135890 touches ByRefPointerOffsetsReporter. The failures kept appearing after #135112 merged, so this PR is still needed for #135086.

The only CI run is from 2026-10-02. All six CdacStressTest legs passed, including windows-x86. Build Analysis never finished, but it matched the single test failure in the red runtime check to known issue #135001 (System.Diagnostics.Process.Tests hang on windows-x86 Checked). That failure is unrelated to this change. A rebase or rerun would refresh CI on current main.

This addresses #135086, which is still open and failing on PRs: on 2026-10-17 its Known Build Error report showed 38 hits in 7 days and 93 in a month.

@max-charlamb

Copy link
Copy Markdown
Member Author

closing in favor of #136089

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ci-scan] Test failure: GCRefStress_AllVerificationsPass misses a Windows x86 PrestubMethodFrame root

2 participants