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

[release/8.0] Put HasNativeCodeReJITAware into GetFunctionAddress #92665

Merged
merged 10 commits into from
Sep 27, 2023

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Sep 26, 2023

Backport of #90049 to release/8.0

/cc @mikelle-rogers

Customer Impact

R2R deoptimization while debugging doesn't happen properly with out this fix. If the customer were to step into R2R code without this fix, the debugger does not stop in the method like it should. This fix allows deoptimization of a method before it is jitted, which results in the debugger stopping in the R2R method when a customer steps into it.

We also audited the codepaths that deal with native code addresses and made sure they behaved appropriately when we have generics and/or deoptimized code.

Testing

It has been manually verified that these commits fix the error.

Risk

IMPORTANT: If this backport is for a servicing release, please verify that:

  • The PR target branch is release/X.0-staging, not release/X.0.

  • If the change touches code that ships in a NuGet package, you have added the necessary package authoring and gotten it explicitly reviewed.

@ghost
Copy link

ghost commented Sep 26, 2023

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #90049 to release/8.0

/cc @mikelle-rogers

Customer Impact

Testing

Risk

IMPORTANT: If this backport is for a servicing release, please verify that:

  • The PR target branch is release/X.0-staging, not release/X.0.

  • If the change touches code that ships in a NuGet package, you have added the necessary package authoring and gotten it explicitly reviewed.

Author: github-actions[bot]
Assignees: -
Labels:

area-Diagnostics-coreclr

Milestone: -

@tommcdon tommcdon added this to the 8.0.0 milestone Sep 26, 2023
@davmason
Copy link
Member

We need to add the changes from #90821 as well, and any other follow up work that happened if there is more.

@mikelle-rogers mikelle-rogers self-assigned this Sep 26, 2023
@mikelle-rogers
Copy link
Member

Just added changes from #90821 @davmason

Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approve. please get a code review. we will take for consideration in 8.0.x

@jeffschwMSFT jeffschwMSFT added the Servicing-consider Issue for next servicing release review label Sep 26, 2023
@carlossanlop carlossanlop added Servicing-approved Approved for servicing release Servicing-consider Issue for next servicing release review and removed Servicing-consider Issue for next servicing release review Servicing-approved Approved for servicing release labels Sep 27, 2023
@carlossanlop carlossanlop added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Sep 27, 2023
@carlossanlop
Copy link
Member

Approved by Tactics via email.

@mikelle-rogers @noahfalk is this good to merge? I see some feedback but don't know if I should wait for it to get addressed or not.

@hoyosjs
Copy link
Member

hoyosjs commented Sep 27, 2023

Nothing needs to get addressed, good to go

@carlossanlop carlossanlop merged commit 29cdcc1 into release/8.0 Sep 27, 2023
113 of 121 checks passed
@carlossanlop carlossanlop deleted the backport/pr-90049-to-release/8.0 branch September 27, 2023 22:38
@ghost ghost locked as resolved and limited conversation to collaborators Oct 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants