cdac: use debugger-conventional HRESULTs in the data-target adapters
- Dominant language
- C#
- Stars
- 18.3k
- Forks
- 5.6k
- PR merge metrics
- PR metrics pending
Description
The cdac legacy data-target adapters in `mscordaccore_universal/Entrypoints.cs` return generic HRESULTs where the CoreCLR debugger stack uses more specific ones. The same patterns exist across the `ICorDebugDataTarget` path (added in #130856) and the `ICLRDataTarget`/`CLRDataCreateInstance` path (added in #114812), and were re-flagged in review of #131008. Consolidating the fixes here so they land in one change.
Items to address across both data-target adapter paths (`CreateTargetFromCorDebugDataTarget` and `CLRDataCreateInstanceCore`):
1. **Partial `ReadVirtual`.** When `ReadVirtual` returns success but reads fewer bytes than requested, return `CORDBG_E_READVIRTUAL_FAILURE` instead of `E_FAIL`. The `CLRDataCreateInstance` path currently does not check `bytesRead` at all - a partial read there can leave a fixed-size header buffer partly uninitialized and cause nondeterministic descriptor parsing. Apply the check consistently.
2. **Write to a read-only target.** When the target does not implement `ICorDebugMutableDataTarget`, the `WriteVirtual` and `SetThreadContext` callbacks return `E_NOTIMPL`, which is ambiguous (method-missing vs. intentionally read-only). Return `CORDBG_E_TARGET_READONLY`, matching `ReadOnlyDataTargetFacade`.
3. **`ContractDescriptorTarget.TryCreate` failure.** When `TryCreate` returns `false`, the entrypoint surfaces `COR_E_INVALIDOPERATION` via `InvalidOperationException`. Return a conventional HRESULT (`E_FAIL`) instead.
Out of scope: the `shimdatatarget.cpp` `E_INVALIDARG` -> `E_UNEXPECTED` suggestions from #131008 review are moot; those methods were removed.
> [!NOTE]
> This issue was authored by GitHub Copilot on behalf of @hoyosjs.
Contributor guide
Research direction
Start in mscordaccore_universal/Entrypoints.cs at CreateTargetFromCorDebugDataTarget and CLRDataCreateInstanceCore, then compare the read-only behavior with ReadOnlyDataTargetFacade. Trace both adapter paths and verify that partial reads, read-only writes or context updates, and failed ContractDescriptorTarget.TryCreate calls return the specified conventional HRESULTs consistently.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- devtools
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 58/100