[clr-interp] Add initial CoreCLR interpreter func-eval support#126576
[clr-interp] Add initial CoreCLR interpreter func-eval support#126576kotlarmilos merged 12 commits intodotnet:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds initial debugger func-eval support when the target thread is stopped in CoreCLR interpreter code, by avoiding native-context hijacking and instead running queued evals from the interpreter breakpoint path.
Changes:
- Adds a pending func-eval slot to the interpreter thread context and triggers execution after the breakpoint debugger callback returns.
- Extends
DebugInterface/Debuggerwith an interpreter-specific hook to execute pending evals. - Adjusts
FuncEvalSetup/DebuggerEvalinitialization to skip native-only setup (e.g., executable breakpoint segment allocation, SP alignment checks) for interpreter evals.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/vm/interpexec.h | Adds InterpThreadContext::m_pPendingFuncEval storage for interpreter-thread func-evals. |
| src/coreclr/vm/interpexec.cpp | Executes pending interpreter func-evals from the INTOP_BREAKPOINT handler using a synthetic filter context. |
| src/coreclr/vm/dbginterface.h | Adds ExecutePendingInterpreterFuncEval(Thread*) to the debug interface under FEATURE_INTERPRETER. |
| src/coreclr/debug/ee/debugger.h | Declares the Debugger implementation of ExecutePendingInterpreterFuncEval. |
| src/coreclr/debug/ee/debugger.cpp | Implements pending-eval execution and updates FuncEvalSetup/DebuggerEval to support interpreter eval flow. |
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
…ogic for clarity in DebuggerEval handling
…r-funceval # Conflicts: # src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/Frames/DebuggerEval.cs
…ec.h include - Restore m_bypassAddress/m_bypassOpcode in EX_CATCH before EX_RETHROW so bypass state is not corrupted if func-eval throws - Remove unused interpexec.h include from debugger.cpp (GetInterpThreadContext is declared on Thread, not in interpexec.h) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…reterFuncEval method
noahfalk
left a comment
There was a problem hiding this comment.
Looks pretty good. I think there are a few places where a little more refactoring can simplify even further and potentially an issue about SetIP interacting with breakpoint skipping.
Includes the func-eval support from dotnet#126576 plus follow-up fixes: - Skip CALL_FINALLY instructions during stepping (internal EH machinery) - Filter CallEntryPoint frames from interpreter debugger stack walks (in-process and DAC) - Add IsCallFinally() to InterpreterWalker Co-authored-by: Matous Kozak <55735845+matouskozak@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Are these local tests? Asking as I'd like to run them against #126809 (your and mine PR combined can be tested too; it has wasm changes in |
@am11 These are the private diagnostic tests that I was running with your PR |
When debugger breakpoint is hit and then debugger resumes the execution, the context at which the breakpoint was hit can be modified. For example, one of the exception interception tests does that to continue from a different location after an exception is hit. The interpreter ignored changes in the context and resumed execution at the original breakpoint location instead of the modified context. This change fixes it by throwing the `ResumeAfterCatchException` if the context was modified. That ensures that we resume the exception correctly even if both the stack frame and the IP was changed. The `ResumeAfterCatchException` became a bit of a misnomer, but I don't want to pollute this PR by renaming the `ResumeAfterCatchException` to `ResumeException`, which would be a better name now. I'd prefer doing it in a separate PR. Follow-up to #126576 This fixes the following interpreter debugger test failure: * Exceptions.InterceptTest
Merged latest main into the branch and resolved the conflicts. I verified locally that on osx-arm64 under --clrinterpreter: FuncEval.NestedFuncEvalTest, FuncEval.EscapeSecondPass, and Exceptions.InterceptTest all pass. |
There was a problem hiding this comment.
Copilot's findings
Comments suppressed due to low confidence (1)
src/coreclr/debug/ee/debugger.cpp:14266
- If
bpInfoSegmentRXwas allocated for a hijacked eval andnew DebuggerEval(...)returns NULL, this returnsE_OUTOFMEMORYwithout freeingbpInfoSegmentRX, leaking executable-heap memory. Consider keeping the heap pointer and freeingbpInfoSegmentRXon this failure path (and any other early-return paths beforepDEtakes ownership).
DebuggerEval *pDE = new (interopsafe, nothrow) DebuggerEval(filterContext, pEvalInfo, bpInfoSegmentRX);
if (pDE == NULL)
{
return E_OUTOFMEMORY;
- Files reviewed: 9/9 changed files
- Comments generated: 2
- Skip INTOP_CALL_FINALLY during stepping (internal EH machinery) - Add IsCallFinally() helper to InterpreterWalker - Filter Environment.CallEntryPoint frames from debugger stack walks (frameinfo.cpp) Follow-up to dotnet#126576. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Skip INTOP_CALL_FINALLY during stepping (internal EH machinery) - Add IsCallFinally() helper to InterpreterWalker - Filter Environment.CallEntryPoint frames from debugger stack walks (frameinfo.cpp) Follow-up to dotnet#126576. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Description
This adds initial func-eval support for threads stopped in CoreCLR interpreter code.
Testing
FuncEval.InfiniteLoopASyncAbortTest is Windows-only and FuncEval.AssemblyLoadFuncEval is a Mono configuration.
Fixes #125959