-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Fix ExecutionManager::GetCodeMethodDesc for interpreter #117625
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
Fix ExecutionManager::GetCodeMethodDesc for interpreter #117625
Conversation
The ExecutionManager::GetCodeMethodDesc expects that the address passed to it is an address of actual JIT/AOT or interpreter generated code. However, it is sometimes passed an address of the interpreter precode, e.g. in the case when called by the MethodTable::GetMethodDescForSlotAddress. The VTable slots need to point to the interpreter stub so that these methods can be called by virtual calls from native code. This change adds a check for the interpreter precode to the ExecutionManager::GetCodeMethodDesc and extracts the MethodDesc from the interpreter data in case it is passed the precode address.
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 enhances the runtime’s handling of interpreter precodes by enabling ExecutionManager::GetCodeMethodDesc
and Precode::GetMethodDesc
to resolve a MethodDesc
when given an interpreter precode entry point. Key changes include:
- Added
GetMethodDesc
inInterpreterPrecode
and wiring it intoPrecode::GetMethodDesc
- Introduced
Precode::AsInterpreterPrecode
helper for casting toInterpreterPrecode
- Updated
ExecutionManager::GetCodeMethodDesc
to detect interpreter precodes and return the associatedMethodDesc
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
src/coreclr/vm/precode.h | Declare InterpreterPrecode::GetMethodDesc , PTR_InterpreterPrecode , and AsInterpreterPrecode |
src/coreclr/vm/precode.cpp | Include interpreter header, implement InterpreterPrecode::GetMethodDesc , update Precode::GetMethodDesc |
src/coreclr/vm/codeman.cpp | Add interpreter-precode check in ExecutionManager::GetCodeMethodDesc |
Comments suppressed due to low confidence (2)
src/coreclr/vm/precode.h:339
- [nitpick] Consider adding a brief XML doc comment above
GetMethodDesc()
to explain that it returns the raw MethodDesc pointer stored in the interpreter precode data.
TADDR GetMethodDesc();
src/coreclr/vm/codeman.cpp:5094
- It would be helpful to add a unit or integration test that drives
ExecutionManager::GetCodeMethodDesc
with an interpreter precode address to ensure this new branch correctly returns the expected MethodDesc.
PTR_Precode pPrecode = Precode::GetPrecodeFromEntryPoint(currentPC);
Would it be more appropriate for the fix to be in |
ExecutionManager::GetCodeMethodDesc is meant to be used on arbitrary IP in the middle of a method. I think it is fragile to depend on the asm code match in Precode::GetPrecodeFromEntryPoint to be able to reject code from arbitrary IPs. |
Hmm, ok, I didn't know that. In that case, I'll move it to the |
@jkotas, ok, the funny part is that with the changes I've added in precode.cpp and precode.h to get MethodDesc for InterpreterPrecode, neither the MethodTable::GetMethodDescForSlotAddress nor ExecutionManager::GetCodeMethodDesc needs to be modified. The MethodTable::GetMethodDescForSlotAddress actually gets the MethodDesc from the precode as the last step if other ones don't work. |
/ba-g the android build seems to be timing out on multiple CI runs |
The
ExecutionManager::GetCodeMethodDesc
expects that the address passed to it is an address of actual JIT/AOT or interpreter generated code. However, it is sometimes passed an address of the interpreter precode, e.g. in the case when called by theMethodTable::GetMethodDescForSlotAddress
. The VTable slots need to point to the interpreter stub so that these methods can be called by virtual calls from native code.This issue occurs in several methods on the .NET runtime startup path when running interpreted.
This change adds a check for the interpreter precode to the
ExecutionManager::GetCodeMethodDesc
and extracts theMethodDesc
from the interpreter data in case it is passed the precode address.