[cDAC] Use NativeCodeVersion entry for GetMethodVarInfo offset#128154
Merged
max-charlamb merged 4 commits intoMay 14, 2026
Merged
Conversation
Contributor
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a tiered-compilation bug in the managed cDAC's DebugInfo_2.GetMethodVarInfo: the IP-to-codeOffset calculation was based on MethodDesc.NativeCode (most recently compiled tier), causing wrong base offsets for older-tier frames still on the stack and producing empty variable location lists. The fix resolves the IP-specific NativeCodeVersion via the ICodeVersions contract, mirroring the legacy DAC's use of ExecutionManager::GetNativeCodeVersion(address).GetNativeCode().
Changes:
- Replace
RuntimeTypeSystem.GetNativeCode(methodDesc)withCodeVersions.GetNativeCodeVersionForIP(pCode)+CodeVersions.GetNativeCode(ncvh). - Throw
InvalidOperationExceptionwhen noNativeCodeVersionis found for the IP, matching native DAC'sE_INVALIDARGbehavior.
e9fcac7 to
6ef9e65
Compare
IExecutionManager.GetStartAddress and GetFuncletStartAddress were declared to return TargetCodePointer but the value stored in CodeBlock is the raw code-block start with no ARM32 thumb bit. That matches native (EECodeInfo::GetStartAddress and CodeHeader::GetCodeStartAddress return TADDR), but the cdac type signature was misleading and caused subtle bugs whenever a caller needed a true PCODE. On ARM32, CodeVersions_1.GetSpecificNativeCodeVersion compared MethodDesc.NativeCode (a PCODE with the thumb bit set) against the raw start address returned by GetStartAddress, so the equality check never matched and GetNativeCodeVersionForIP returned Invalid. Change the return types to TargetPointer (matching native TADDR semantics), drop the now-redundant AddressFromCodePointer conversions at the callers, and at the one site in CodeVersions_1 that genuinely needs a PCODE for comparison use CodePointerFromAddress (the cdac analogue of native PINSTRToPCODE). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
DebugInfo_2.GetMethodVarInfo was computing the IP->codeOffset relative to MethodDesc.NativeCode. That field tracks only the most recently-compiled tier, so under tiered compilation an older-tier frame still on the stack gets the wrong base, no varInfo ranges match, and cDAC returns an empty location list while the legacy DAC finds the entry. This trips the Debug.Assert in ClrDataValue.GetNumLocations. Resolve the IP-specific NativeCodeVersion via the ICodeVersions contract, mirroring the legacy DAC which uses ExecutionManager::GetNativeCodeVersion(address).GetNativeCode() (daccess.cpp:5591). Throw on an invalid NativeCodeVersion, matching the native DAC's E_INVALIDARG behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
6ef9e65 to
7e3d349
Compare
ReadyToRunJitManager.GetMethodInfo computed startAddress as 'imageBase + function.BeginAddress' using the raw RVA. On ARM32 thumb code, the RUNTIME_FUNCTION BeginAddress encodes the thumb bit (bit 0), so the resulting startAddress carried that bit. Native RUNTIME_FUNCTION__BeginAddress (clrnt.h, ARM32 branch) calls ThumbCodeToDataPointer to strip it before use. With the previous commit changing IExecutionManager.GetStartAddress to return TargetPointer (a raw TADDR), this mismatch became visible: GetExceptionClauses computed 'methodRVA = methodStart - rangeStart' with the thumb bit still set, so the R2R exception-info table lookup missed by one byte and IXCLRDataValueDumpTests.GetExceptionClauses_ContainsCatchAllClause failed on Debian.13.Arm32. Use CodePointerUtils.AddressFromCodePointer (the cdac analogue of native ThumbCodeToDataPointer) when forming the hot and cold start addresses from the R2R RVA, matching native semantics and producing a true raw TADDR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
noahfalk
approved these changes
May 13, 2026
noahfalk
approved these changes
May 13, 2026
noahfalk
approved these changes
May 13, 2026
Member
Author
|
/ba-g PR passed CI before docs only change |
1 similar comment
Member
Author
|
/ba-g PR passed CI before docs only change |
This was referenced May 14, 2026
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.
Note
This PR was authored with assistance from GitHub Copilot.
Problem
runtime-diagnosticsbuild 1418267 failed with:The asserting frame was the
thisparameter ofManualResetEventSlim.Waitduring!ClrStack -aagainst the WebApp3 dump.Root cause
DebugInfo_2.GetMethodVarInfocomputed the IP -> codeOffset relative toMethodDesc.NativeCode. That field tracks only the most recently-compiled tier. With tiered compilation, an older-tier frame still on the stack gets the wrong base, novarInforanges match, and cDAC returns an empty location list while the legacy DAC finds the entry.The legacy DAC (
src/coreclr/debug/daccess/daccess.cpp:5591) instead usesExecutionManager::GetNativeCodeVersion(address).GetNativeCode()-- the entry point of the specific NativeCodeVersion that owns the IP.This bug has been latent since #125463. dotnet/diagnostics#5767 (merged 2026-05-12) rebuilt the debuggees with the .NET 11 test SDK, which increased the chance of hot methods like
ManualResetEventSlim.Waitstraddling tier boundaries at dump-capture time and surfaced the existing bug.Fix
Resolve the IP-specific
NativeCodeVersionvia theICodeVersionscontract (already mirrorsExecutionManager::GetNativeCodeVersion+NativeCodeVersion::GetNativeCodefor versionable and non-versionable methods). Throw on an invalidNativeCodeVersion, matching the native DAC'sE_INVALIDARGbehavior.Type-correctness fix (uncovered while debugging an ARM32 regression)
While validating the fix above, an ARM32 regression in
IXCLRDataValueDumpTests.GetSize_ReturnsExpectedSizesrevealed a long-standing mistype:IExecutionManager.GetStartAddress/GetFuncletStartAddressdeclaredTargetCodePointer, but the value stored inCodeBlockis the raw code-block start with no ARM32 thumb bit. That matches native --EECodeInfo::GetStartAddressandCodeHeader::GetCodeStartAddressboth returnTADDR-- but the cdac type signature was lying about what the value represented.CodeVersions_1.GetSpecificNativeCodeVersioncomparedMethodDesc.NativeCode(aPCODEwith the thumb bit set) against the rawTADDRfromGetStartAddress, so the equality check never matched andGetNativeCodeVersionForIPreturnedInvalid-- which broke the newGetMethodVarInfopath on ARM32.The first commit in this PR changes the return types to
TargetPointer(matching nativeTADDRsemantics), drops the now-redundantAddressFromCodePointerconversions at the callers, and at the one site that genuinely needs aPCODEfor comparison usesCodePointerFromAddress(the cdac analogue of nativePINSTRToPCODE). The second commit is the originalGetMethodVarInfochange, now safe on ARM32.Validation
dotnet teston the full cdac suite: 2133/2133 passed (16 unrelated skips).