-
Notifications
You must be signed in to change notification settings - Fork 5.3k
[clr-interp] Fix debugger issues in interpreter #123047
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[clr-interp] Fix debugger issues in interpreter #123047
Conversation
|
Tagging subscribers to this area: @BrzVlad, @janvorli, @kg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes debugger issues when working with interpreted code by ensuring that the EECodeInfo infrastructure can retrieve interpreter byte codes instead of interpreter stub addresses. The key addition is a new function GetInterpreterCodeFromInterpreterPrecodeIfPresent that unwraps interpreter precodes to get the actual bytecode addresses.
Key changes:
- New function
GetInterpreterCodeFromInterpreterPrecodeIfPresentto extract interpreter bytecode addresses from interpreter precodes - Helper method
GetCodeForInterpreterOrJittedin MethodDesc for easier access - Multiple DAC (Data Access Component) files updated to use the new function/helper when retrieving code addresses
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/vm/stubmgr.cpp | Adds null check for pPrecode to prevent dereferencing null pointers |
| src/coreclr/vm/precode.h | Declares new function GetInterpreterCodeFromInterpreterPrecodeIfPresent |
| src/coreclr/vm/precode.cpp | Implements GetInterpreterCodeFromInterpreterPrecodeIfPresent to unwrap interpreter precodes |
| src/coreclr/vm/method.hpp | Adds GetCodeForInterpreterOrJitted helper method for MethodDesc |
| src/coreclr/debug/daccess/request.cpp | Updates code address retrieval to use new helper/function in multiple locations |
| src/coreclr/debug/daccess/enummem.cpp | Uses GetCodeForInterpreterOrJitted for stack trace exception handling |
| src/coreclr/debug/daccess/dacdbiimpl.cpp | Updates GetMethodRegionInfo and GetNativeCodeInfoForAddr to use new function/helper |
| src/coreclr/debug/daccess/daccess.cpp | Updates multiple debugger functions to use GetCodeForInterpreterOrJitted and GetInterpreterCodeFromInterpreterPrecodeIfPresent |
| #ifdef FEATURE_INTERPRETER | ||
| PCODE GetCodeForInterpreterOrJitted() | ||
| { | ||
| WRAPPER_NO_CONTRACT; | ||
| return GetInterpreterCodeFromInterpreterPrecodeIfPresent(GetNativeCode()); | ||
| } | ||
| #endif |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GetCodeForInterpreterOrJitted method is only defined when FEATURE_INTERPRETER is enabled, but it is called unconditionally in multiple places (request.cpp, enummem.cpp, dacdbiimpl.cpp, daccess.cpp). This will cause compilation errors when FEATURE_INTERPRETER is not defined. Consider either: 1) removing the #ifdef guard around this method definition, or 2) adding #ifdef FEATURE_INTERPRETER guards around all call sites. Since GetInterpreterCodeFromInterpreterPrecodeIfPresent is already safe to call when FEATURE_INTERPRETER is not defined (it just returns the input unchanged), option 1 is recommended.
| #ifdef FEATURE_INTERPRETER | |
| PCODE GetCodeForInterpreterOrJitted() | |
| { | |
| WRAPPER_NO_CONTRACT; | |
| return GetInterpreterCodeFromInterpreterPrecodeIfPresent(GetNativeCode()); | |
| } | |
| #endif | |
| PCODE GetCodeForInterpreterOrJitted() | |
| { | |
| WRAPPER_NO_CONTRACT; | |
| return GetInterpreterCodeFromInterpreterPrecodeIfPresent(GetNativeCode()); | |
| } |
|
|
||
| #endif // !FEATURE_PORTABLE_ENTRYPOINTS | ||
|
|
||
| PCODE GetInterpreterCodeFromInterpreterPrecodeIfPresent(PCODE codePointerMaybeInterpreterStub) |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type mismatch between declaration and implementation. The function is declared as taking TADDR in precode.h line 867, but implemented as taking PCODE in precode.cpp. While PCODE and TADDR are likely compatible types, this inconsistency should be fixed to match the declaration. Change the parameter type to TADDR to match the header file.
| PCODE GetInterpreterCodeFromInterpreterPrecodeIfPresent(PCODE codePointerMaybeInterpreterStub) | |
| PCODE GetInterpreterCodeFromInterpreterPrecodeIfPresent(TADDR codePointerMaybeInterpreterStub) |
| { | ||
| pReJitData->rejitID = nativeCodeVersion.GetILCodeVersion().GetVersionId(); | ||
| pReJitData->NativeCodeAddr = nativeCodeVersion.GetNativeCode(); | ||
| pReJitData->NativeCodeAddr = GetInterpreterCodeFromInterpreterPrecodeIfPresent(nativeCodeVersion.GetNativeCode ()); |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is an extra space before the opening parenthesis in the function call. Change 'GetNativeCode ()' to 'GetNativeCode()'.
| pReJitData->NativeCodeAddr = GetInterpreterCodeFromInterpreterPrecodeIfPresent(nativeCodeVersion.GetNativeCode ()); | |
| pReJitData->NativeCodeAddr = GetInterpreterCodeFromInterpreterPrecodeIfPresent(nativeCodeVersion.GetNativeCode()); |
Fix a set of issues found when attempting to use the debugger on interpreted code. In general we need to get the interpreter byte codes if possible when attempting to use the EECodeInfo infrastructure. These changes helped me debug issues that appeared in interpmode=2