-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Do not ignore ClassLoader::ResolveTokenToTypeDefThrowing return values #121864
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
Conversation
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 improper handling of ClassLoader::ResolveTokenToTypeDefThrowing return values. The function can return FALSE when a token cannot be resolved, and in such cases, the out parameters are not initialized. The fix ensures that calling code checks the return value before using the out parameters.
- Added return value checks in three locations where
ResolveTokenToTypeDefThrowingwas previously called without checking its result - Updated logic to gracefully handle resolution failures with appropriate fallback behavior
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/coreclr/vm/methodtablebuilder.cpp | Added return value check at line 3875 with fallback to ELEMENT_TYPE_VALUETYPE, and wrapped code at line 3914 to only use resolved values when resolution succeeds |
| src/coreclr/vm/method.cpp | Added return value check at line 2318 to prevent use of uninitialized out parameters in IsTypeDefOrRefImplementedInSystemModule |
|
Tagging subscribers to this area: @mangod9 |
c5bb40c to
e56b25d
Compare
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
ResolveTokenToTypeDefThrowing can return FALSE in some situations when the token cannot be resolved. The calling code has to deal with the situation gracefully since the [out] arguments are not initialized in that case. Fixes dotnet#121791
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/backport to release/10.0 |
|
Started backporting to |
|
/ba-g osx infra is down. The change passed CI before. |
…ng return values (#121968) Backport of #121864 to release/10.0 /cc @jkotas ## Customer Impact - [x] Customer reported - [ ] Found internally Customer observed crash in NUnit project after upgrading to .NET 10. The bug is triggered by a static field that: - The type of the field must be a struct type - The struct type lives in external assembly. - The external assembly exists, but the struct type is missing in that assembly. The fix is the add missing error handling in a few places. ## Regression - [x] Yes - [ ] No Regressions introduced by #114675 (runtime async feature) and #118413 (type loading perf work) ## Testing Targeted repro, distilled to a test ## Risk Low. The fix is just an extra error handling. --------- Co-authored-by: Jan Kotas <[email protected]>
ResolveTokenToTypeDefThrowing can return FALSE in some situations when the token cannot be resolved. The calling code has to deal with the situation gracefully since the [out] arguments are not initialized in that case.
Regressions introduced by #114675 (method.cpp) and #118413 (methodtablebuilder.cpp)
Fixes #121791