[cDAC] Fix NonVirtualEntry2MethodDesc to handle invalid precode addresses#124986
[cDAC] Fix NonVirtualEntry2MethodDesc to handle invalid precode addresses#124986max-charlamb wants to merge 1 commit intodotnet:mainfrom
Conversation
…sses NonVirtualEntry2MethodDesc throws InvalidOperationException when an address falls in a precode RangeSection but doesn't match any known precode type. The DAC's C++ implementation returns NULL in this case. This mismatch caused a DEBUG assertion crash in GetCodeHeaderData (cDAC returned 0x80131509, DAC returned 0x80070057), breaking SOS !clru on IL stubs in the cDAC_windows_x64_release CI job. The fix catches InvalidOperationException from GetMethodDescFromStubAddress and returns TargetPointer.Null, matching the DAC behavior. Adds a dump test that exercises GetCodeHeaderData with an invalid precode address derived from the VarargPInvoke debuggee's entry point. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
There was a problem hiding this comment.
Pull request overview
Aligns cDAC behavior with the native DAC when given an address that falls within a precode RangeSection but does not correspond to a valid precode, preventing exceptions and downstream assertion failures.
Changes:
- Added a regression dump test that calls
GetCodeHeaderDatawith an intentionally invalid precode address and assertsE_INVALIDARG. - Updated
NonVirtualEntry2MethodDescto returnTargetPointer.Null(instead of throwing) when precode bytes are invalid.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/native/managed/cdac/tests/DumpTests/VarargPInvokeDumpTests.cs | Adds a regression test for invalid precode addresses passed to GetCodeHeaderData. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManagerCore.cs | Matches native DAC behavior by catching invalid-precode exceptions and returning Null. |
....Diagnostics.DataContractReader.Contracts/Contracts/ExecutionManager/ExecutionManagerCore.cs
Show resolved
Hide resolved
| // Offset by 1 byte to create an address that is still in the same | ||
| // RangeList range but is not a valid precode. This is the same | ||
| // scenario that caused the original CI failure: GetCodeHeaderData | ||
| // calls NonVirtualEntry2MethodDesc, which calls GetMethodDescFromStubAddress, | ||
| // which throws InvalidOperationException for invalid precode bytes. | ||
| // The fix catches this and returns E_INVALIDARG (matching the DAC). | ||
| DacpCodeHeaderData codeHeaderData; | ||
| int hr = sosDac.GetCodeHeaderData(new ClrDataAddress(entryPoint.Value + 1), &codeHeaderData); |
There was a problem hiding this comment.
This test assumes entryPoint is a precode located in a precode RangeList section, but it doesn’t verify that precondition. If GetMethodEntryPointIfExists returns a non-precode entrypoint (e.g., a real code address), entryPoint.Value + 1 may still be valid and GetCodeHeaderData may succeed, making the test flaky or invalid. Consider adding a precondition check (and continue if it doesn’t hold), such as validating that GetCodeHeaderData(entryPoint) succeeds first, or that resolving the valid entryPoint maps back to the same MethodDesc before offsetting by 1.
| // Offset by 1 byte to create an address that is still in the same | |
| // RangeList range but is not a valid precode. This is the same | |
| // scenario that caused the original CI failure: GetCodeHeaderData | |
| // calls NonVirtualEntry2MethodDesc, which calls GetMethodDescFromStubAddress, | |
| // which throws InvalidOperationException for invalid precode bytes. | |
| // The fix catches this and returns E_INVALIDARG (matching the DAC). | |
| DacpCodeHeaderData codeHeaderData; | |
| int hr = sosDac.GetCodeHeaderData(new ClrDataAddress(entryPoint.Value + 1), &codeHeaderData); | |
| // First, validate that the entry point itself corresponds to a valid | |
| // code header. If it does not, this frame does not meet the test | |
| // preconditions, so skip it. | |
| DacpCodeHeaderData codeHeaderData; | |
| int hr = sosDac.GetCodeHeaderData(new ClrDataAddress(entryPoint.Value), &codeHeaderData); | |
| if (hr != 0) | |
| continue; | |
| // Offset by 1 byte to create an address that is still in the same | |
| // RangeList range but is not a valid precode. This is the same | |
| // scenario that caused the original CI failure: GetCodeHeaderData | |
| // calls NonVirtualEntry2MethodDesc, which calls GetMethodDescFromStubAddress, | |
| // which throws InvalidOperationException for invalid precode bytes. | |
| // The fix catches this and returns E_INVALIDARG (matching the DAC). | |
| DacpCodeHeaderData invalidCodeHeaderData; | |
| hr = sosDac.GetCodeHeaderData(new ClrDataAddress(entryPoint.Value + 1), &invalidCodeHeaderData); |
Summary
NonVirtualEntry2MethodDescthrowsInvalidOperationExceptionwhen an address falls in a precodeRangeSectionbut doesn't match any known precode type (e.g., a MethodDesc address that coincidentally shares the same memory range). The DAC's C++ implementation returnsNULLin this case.This mismatch caused a DEBUG assertion crash in
GetCodeHeaderData— the cDAC returned0x80131509(COR_E_INVALIDOPERATION) while the DAC returned0x80070057(E_INVALIDARG), triggeringDebug.Assert(hrLocal == hr)and crashing the process. This broke!clruon IL stubs in thecDAC_windows_x64_releaseCI job (build 1313397).Fix
Catch
InvalidOperationExceptionfromGetMethodDescFromStubAddressinExecutionManagerCore.NonVirtualEntry2MethodDescand returnTargetPointer.Null, matching the DAC's C++ behavior whereNonVirtualEntry2MethodDescreturnsNULLfor unrecognized addresses.Test
Adds
VarargPInvoke_GetCodeHeaderDataWithInvalidPrecodeAddressdump test that:GetCodeHeaderData(SOS-level API) and asserts it returnsE_INVALIDARGVerified the test fails without the fix (returns
COR_E_INVALIDOPERATION) and passes with the fix (returnsE_INVALIDARG).