Fix most of the diagnostic EH tests with interpreter#125525
Fix most of the diagnostic EH tests with interpreter#125525janvorli merged 7 commits intodotnet:mainfrom
Conversation
Before this change, all of the exception handling diagnostic tests were failing. There were couple of reasons: * The debugger stack walk didn't work when it needed to extract the starting context from explicit frames and the InterpreterFrame was the first one. It got the context of the native caller of the interpreter instead of the interpreter context. * Exception interception was not supported for the interpreted code in the runtime yet * The IL to native offsets map generated by the interpreter compiler was always setting the source field to ICorDebugInfo::SOURCE_TYPE_INVALID. Exception interception looks for offset with ICorDebugInfo::STACK_EMPTY and thus it found none and has failed. * The System.Runtime.StackFrameIterator methods were not ignored by the DacDbiInterfaceImpl::UnwindStackWalkFrame and so the tests verifying the stack trace were failing * InterpreterFrame was not ignored when enumerating internal frames, generating extra unexpected stuff to the debugger stack trace. This change fixes these issues and now only three of the total 21 diagnostic EH tests are failing. It also fixes an issue introduced by recent refactoring of the debugging code error handling that was causing the tests to fail on Linux even without interpreter.
There was a problem hiding this comment.
Pull request overview
Fixes interpreter-related exception handling (EH) diagnostics by aligning debugger stack walking, interception, and IL↔native mapping behavior with expectations, and by addressing a Linux-only regression from recent debugging error-handling refactors.
Changes:
- Enable EH interception support for interpreted code (frame interception + code manager resume paths).
- Emit interpreter IL→native maps with
STACK_EMPTYwhere needed to allow interception to locate valid offsets. - Adjust DAC/DBI stack walking and internal-frame enumeration to ignore additional runtime/internal frames (including
InterpreterFrameandSystem.Runtime.StackFrameIterator.*).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/vm/frames.h | Adds interception reporting for InterpreterFrame based on faulting state. |
| src/coreclr/vm/exceptionhandling.cpp | Uses interpreter-or-JIT code address for interception and resumes using the code manager instead of restoring nonvolatile context directly. |
| src/coreclr/interpreter/compiler.h | Extends InterpInst with tracked evaluation stack depth. |
| src/coreclr/interpreter/compiler.cpp | Populates stackDepth and emits STACK_EMPTY in IL→native maps when stack depth is 0. |
| src/coreclr/debug/daccess/dacdbiimplstackwalk.cpp | Skips System.Runtime.StackFrameIterator.* methods and filters InterpreterFrame from internal frames. |
| src/coreclr/debug/daccess/dacdbiimpl.cpp | Improves context extraction when the first usable frame is an InterpreterFrame, and tweaks a success return path. |
You can also share your feedback on Copilot code review. Take the survey.
82ae044 to
2874951
Compare
There was a problem hiding this comment.
Pull request overview
This PR improves CoreCLR debugger/diagnostics support for interpreted exception handling paths so that most diagnostic EH tests can pass under the interpreter (and also fixes a Linux regression in debug error handling).
Changes:
- Extend interpreter frame/debugger integration (frame interception reporting, retrieving correct starting context when an
InterpreterFrameis present). - Update EH interception/resume logic to use interpreter-aware entry points and
CodeManager::ResumeAfterCatch. - Adjust interpreter-generated IL-to-native mapping/source typing and update DAC stack-walking to hide internal helper frames (including
System.Runtime.StackFrameIterator.*andInterpreterFrame).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/vm/frames.h | Adds InterpreterFrame::GetInterception_Impl to report interception state for debugger stackwalking. |
| src/coreclr/vm/exceptionhandling.cpp | Uses interpreter-aware code start for interception and restores context via ResumeAfterCatch. |
| src/coreclr/interpreter/compiler.h | Introduces a new instruction flag and initializes m_pLastNewIns. |
| src/coreclr/interpreter/compiler.cpp | Sets per-IL-offset flags and uses them to mark IL-to-native mapping source types. |
| src/coreclr/debug/daccess/dacdbiimplstackwalk.cpp | Skips System.Runtime.StackFrameIterator.* and InterpreterFrame in internal-frame enumeration. |
| src/coreclr/debug/daccess/dacdbiimpl.cpp | Special-cases InterpreterFrame when synthesizing a thread context from explicit frames and returns S_OK for that path. |
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
This PR targets CoreCLR debugging/EH support for the interpreter so that diagnostic exception-handling tests can run successfully under interpreted execution, and also fixes a Linux regression from recent debugging error-handling refactoring.
Changes:
- Adds interpreter-aware exception interception/resume behavior by using interpreter/jit code start addresses and code-manager resume APIs.
- Improves interpreter IL-to-native offset mapping by marking
STACK_EMPTYat the first instruction for an IL offset when the IL stack is empty. - Updates DAC/debugger stack walking to ignore
System.Runtime.StackFrameIterator.*and skipInterpreterFramewhere appropriate, plus adds interpreter-frame interception reporting.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/vm/frames.h | Adds InterpreterFrame::GetInterception_Impl() to report exception interception when faulting. |
| src/coreclr/vm/exceptionhandling.cpp | Adjusts interception handling to use interpreter/jit entrypoints and code-manager resume paths. |
| src/coreclr/interpreter/compiler.h | Introduces an instruction flag for “empty IL stack” and initializes m_pLastNewIns. |
| src/coreclr/interpreter/compiler.cpp | Sets STACK_EMPTY offset mappings when the IL stack is empty at an IL offset’s first emitted instruction. |
| src/coreclr/debug/daccess/dacdbiimplstackwalk.cpp | Skips StackFrameIterator methods and omits InterpreterFrame from internal-frame enumeration. |
| src/coreclr/debug/daccess/dacdbiimpl.cpp | Teaches DAC context extraction to seed context from interpreter frames when needed; fixes return value. |
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Improves CoreCLR debugger/EH behavior for interpreted code so that diagnostic exception-handling (EH) tests can run under the interpreter, and also fixes a Linux regression from recent debugging error-handling refactoring.
Changes:
- Add interpreter-aware interception and resumption paths in EH/debugger code (use
GetCodeForInterpreterOrJitted(), route resumption viaICodeManager::ResumeAfterCatch, and report interpreter frame interception state). - Teach the interpreter compiler to emit
STACK_EMPTYIL->native mappings (via a new instruction flag) to support exception interception resume-point lookup. - Adjust DAC/DBI stack walking to ignore
System.Runtime.StackFrameIterator.*andInterpreterFramein relevant internal-frame enumeration paths, and fixGetContextbehavior whenGetThreadContextis not implemented.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/vm/frames.h | Reports interpreter frame interception state based on faulting status. |
| src/coreclr/vm/exceptionhandling.cpp | Uses interpreter/jitted code start addresses and resumes via the code manager for interception scenarios. |
| src/coreclr/interpreter/compiler.h | Adds an instruction flag for “empty IL stack” and initializes m_pLastNewIns. |
| src/coreclr/interpreter/compiler.cpp | Sets IL->native map source to STACK_EMPTY based on the new flag; attempts to mark first instruction per IL offset. |
| src/coreclr/debug/daccess/dacdbiimplstackwalk.cpp | Skips StackFrameIterator methods and InterpreterFrame in certain debugger stackwalk/internal-frame paths. |
| src/coreclr/debug/daccess/dacdbiimpl.cpp | Fixes DAC GetContext fallback for interpreter frames and returns S_OK when context is synthesized. |
You can also share your feedback on Copilot code review. Take the survey.
|
/ba-g the failures are some iOS tests that have been failing in other PRs too and #125295 |
Before this change, all of the exception handling diagnostic tests were failing. There were couple of reasons:
This change fixes these issues and now only three of the total 21 diagnostic EH tests are failing.
It also fixes an issue introduced by recent refactoring of the debugging code error handling that was causing the tests to fail on Linux even without interpreter.