Skip to content
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

Change ICorProfilerInfo::SetILFunctionBody to use codeversions #33197

Open
davmason opened this issue Mar 5, 2020 · 0 comments
Open

Change ICorProfilerInfo::SetILFunctionBody to use codeversions #33197

davmason opened this issue Mar 5, 2020 · 0 comments
Assignees
Labels
area-Diagnostics-coreclr enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@davmason
Copy link
Member

davmason commented Mar 5, 2020

#32969 is introducing on stack replacement (OSR) for jitted methods, and this means there is a new sharp edge for profilers to find. Currently we do not prevent (but do have documentation that states you shouldn't) profilers from calling ICorProfilerInfo::SetILFunctionBody after a method has already been jitted. Previous to tiered compilation this would be effectively a no-op, unless rejit was called, since the method was already compiled.

Once tiered compilation was introduced, if a profiler sets the IL after a tier 0 compilation has occurred but before a tier 1 has, then the tier 1 will use different IL and have different behavior than tier 0. While this might be unexpected by the profiler author, it won't cause fatal errors.

With OSR, if a profiler calls SetILFunctionBody after tier 0 the new function will potentially be replaced mid method, and if the IL varies significantly then there is plenty of potential for misbehavior or crashes.

As mentioned in #32969 (comment), we could solve this by using the existing code versioning scheme that we use for rejit and tiered compilation. Some extra care would be necessary to preserve the existing behavior of SetILFunctionBody. We want it to be viewed as the new default IL version, so any code that requests the default version should be redirected to the new version provided to SetILFunctionBody, including RequestRevert.

@davmason davmason added this to the 5.0 milestone Mar 5, 2020
@davmason davmason self-assigned this Mar 5, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Mar 5, 2020
@tommcdon tommcdon removed the untriaged New issue has not been triaged by the area owner label Mar 27, 2020
@tommcdon tommcdon modified the milestones: 5.0.0, 6.0.0 Jul 27, 2020
@tommcdon tommcdon added the enhancement Product code improvement that does NOT require public API changes/additions label Dec 10, 2020
@davmason davmason modified the milestones: 6.0.0, Future Jan 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Diagnostics-coreclr enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

3 participants