-
Notifications
You must be signed in to change notification settings - Fork 5.3k
[clr-interp] Fix Runtime_74635_1 test #122194
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
This is an interesting test which touches on a poorly design/documented portion of the ECMA spec. Notably, what can be done with the this pointer of a structure during its construction. Notably, if the this pointer is stored somewhere else, and then that pointer is used then there is a possibility of mutating a value on the IL evaluation stack in the current interpreter implementation. As that is not considered a thing which should be possible, RyuJIT has the policy of creating function local temporaries for the newobj instruction, and then copying the result to a location on the evaluation stack. This change matches that behavior which is the current reading of the correct behavior according to the ECMA standard.
|
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 the Runtime_74635_1 test by ensuring ECMA spec compliance for value type construction in the interpreter. The issue occurs when a value type constructor exposes its this pointer (e.g., by storing it in a closure), which could allow external mutation of values on the evaluation stack. The fix matches RyuJIT behavior by allocating value type instances in stable memory during construction, then copying the result to the evaluation stack afterward.
Key Changes
- Introduces a flag
resultIsNewObjToVTWithCopyto track when value typenewobjrequires a copy operation - Allocates
newobjvalue type instances as global variables (stable memory) during construction - Adds post-constructor logic to copy the initialized value type from the temporary to a new stack location
janvorli
left a comment
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.
LGTM, thank you!
This is an interesting test which touches on a poorly design/documented portion of the ECMA spec. Notably, what can be done with the this pointer of a structure during its construction. Notably, if the this pointer is stored somewhere else, and then that pointer is used then there is a possibility of mutating a value on the IL evaluation stack in the current interpreter implementation. As that is not considered a thing which should be possible, RyuJIT has the policy of creating function local temporaries for the newobj instruction, and then copying the result to a location on the evaluation stack. This change matches that behavior which is the current reading of the correct behavior according to the ECMA standard.