Convert IDacDbiInterface to return HRESULT to fix cross-DSO exception handling on mobile platforms#124836
Merged
steveisok merged 6 commits intodotnet:mainfrom Mar 2, 2026
Merged
Conversation
Change all 119 non-HRESULT virtual methods in IDacDbiInterface to return HRESULT. Methods that previously returned values (BOOL, VMPTR_*, ULONG, etc.) now take an additional OUT parameter. void methods simply change their return type to HRESULT. This is the interface declaration change for Approach E: eliminating C++ exceptions crossing the DAC/DBI DSO boundary on Android. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Convert all 122 IDacDbiInterface method implementations to catch exceptions and return HRESULT. Value-returning methods now write results through OUT parameters. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the CoreCLR DBI↔DAC boundary to avoid C++ exceptions crossing DSOs on mobile platforms by converting IDacDbiInterface to an HRESULT-returning API (with previous return values moved to OUT parameters) and updating both DAC implementations and DBI call sites accordingly.
Changes:
- Convert many
IDacDbiInterfacemethods fromvoid/value-returning toHRESULT+ OUT parameter(s). - Update DBI (right-side) call sites to handle
HRESULT(typically viaIfFailThrow). - Update DAC implementations to catch/translate exceptions to
HRESULTand return values via OUT parameters.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/debug/inc/dacdbiinterface.h | Converts interface methods to HRESULT + OUT params (API contract change). |
| src/coreclr/debug/daccess/dacdbiimpl.h | Updates DAC-side class declarations to match new HRESULT signatures. |
| src/coreclr/debug/daccess/dacdbiimplstackwalk.cpp | Updates stackwalk-related DAC implementations to HRESULT and adds try/catch-to-HRESULT wrappers. |
| src/coreclr/debug/daccess/dacdbiimpllocks.cpp | Updates lock-test DAC helpers to HRESULT with exception-to-HRESULT translation. |
| src/coreclr/debug/di/valuehome.cpp | Updates handle-address fetch calls to use new OUT-parameter signature. |
| src/coreclr/debug/di/rstype.cpp | Updates type-query DAC calls to HRESULT + OUT params with IfFailThrow. |
| src/coreclr/debug/di/rsthread.cpp | Updates thread-related DAC calls to HRESULT + OUT params with IfFailThrow. |
| src/coreclr/debug/di/rsstackwalk.cpp | Updates stackwalk DAC calls to HRESULT + OUT params with IfFailThrow. |
| src/coreclr/debug/di/rsfunction.cpp | Updates function/IL/native-info DAC calls to HRESULT + OUT params with IfFailThrow. |
| src/coreclr/debug/di/rsclass.cpp | Updates class/type/field-related DAC calls to HRESULT + OUT params with IfFailThrow. |
| src/coreclr/debug/di/rsassembly.cpp | Updates assembly path/trust DAC calls to HRESULT + OUT params with IfFailThrow. |
| src/coreclr/debug/di/rsappdomain.cpp | Updates AppDomain/object/module lookup DAC calls to HRESULT + OUT params with IfFailThrow. |
| src/coreclr/debug/di/process.cpp | Updates process-level DAC calls (init/heap/object/type/etc.) to HRESULT + OUT params with IfFailThrow. |
| src/coreclr/debug/di/module.cpp | Updates module metadata/symbol/name-related DAC calls to HRESULT + OUT params with IfFailThrow. |
| src/coreclr/debug/di/divalue.cpp | Updates value/object inspection DAC calls to HRESULT + OUT params with IfFailThrow (one call site still missing). |
This was referenced Feb 25, 2026
noahfalk
approved these changes
Feb 25, 2026
Member
noahfalk
left a comment
There was a problem hiding this comment.
This change is a monster and if it subtlely didn't follow the pattern a few thousand lines in I doubt any human reviewer is going to notice. That said it seems like a fine pattern to go for and copilot seems to be finding the occasional mis-applications.
The cross-module exception handling I believe was also problematic even in desktop scenarios though perhaps not to the same degree. I consider this goodness across the board, not just for mobile.
mdh1418
reviewed
Feb 25, 2026
added 2 commits
February 26, 2026 16:53
Member
Author
|
@noahfalk can you give this another pass? |
noahfalk
approved these changes
Mar 1, 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.
On mobile platforms, C++ exceptions thrown from DAC (libmscordaccore.so) are never caught by DBI (libmscordbi.so). This causes all ICorDebug operations that involve DAC calls to fail with E_FAIL.
Root cause:
Our mobile configurations statically link libc++ into each DSO. Combined with -fvisibility=hidden and -Bsymbolic, each DSO gets private copies of all C++ typeinfo. libc++'s RTTI uses pointer-only comparison (no string
fallback), so catch(Exception*) in DBI never matches exceptions thrown from DAC — the typeinfo pointers are different.
Solution
Convert all 119 non-HRESULT methods in IDacDbiInterface to return HRESULT, with former return values moved to OUT parameters. Each DAC implementation body is wrapped with EX_TRY/EX_CATCH_HRESULT so exceptions are caught inside the DAC and converted to HRESULT
before crossing the DSO boundary. DBI callers are updated to use IfFailThrow() to convert the HRESULT back to an exception within the DBI's own DSO, preserving existing error handling behavior.
This eliminates C++ exceptions crossing the DAC→DBI boundary entirely.
Changes