FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

[release/9.0-staging] [android] Fix crash in method_to_ir by github-actions[bot] · Pull Request #109510 · dotnet/runtime · GitHub

Repository navigation

[release/9.0-staging] [android] Fix crash in method_to_ir - #109510

Merged
steveisok merged 2 commits into
release/9.0-stagingfrom
backport/pr-109381-to-release/9.0-staging
Nov 11, 2024
Merged

steveisok merged 2 commits into
release/9.0-stagingfrom
backport/pr-109381-to-release/9.0-staging

Conversation

github-actions Bot commented Nov 4, 2024 •
edited by steveisok
Loading

Copy link
Copy Markdown
Contributor

Backport of #109381 to release/9.0-staging

/cc @steveisok

Customer Impact

  • Customer reported
  • Found internally

A customer was experiencing intermittent crashes with their android app around mono_method_to_ir. After testing multiple iterations, we found there were times when calls to try_prepare_objaddr_callvirt_optimization contained a null reference to a MonoClass. As a result, the app would crash.

To fix, we made a call to mono_class_from_mono_type_internal to make sure we would get a legit MonoClass.

Regression

  • Yes
  • No

[If yes, specify when the regression was introduced. Provide the PR or commit if known.]

Testing

Manual before and after. After app did not crash.

Risk

Low

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.

There exists a possibility where the klass being passed to try_prepare_objaddr_callvirt_optimization is not legit. This can result
in unpredictable crashes.

To fix, we pass the MonoType and flush out the MonoClass by calling mono_class_from_mono_type_internal.

Fixes #109111

Copy link
Copy Markdown
Contributor

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

steveisok added the Servicing-consider Issue for next servicing release review label Nov 5, 2024

jeffschwMSFT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

lgtm. we will take for consideration in 9.0.x

Copy link
Copy Markdown
Member

/ba-g Known infra related errors

steveisok merged commit 29eae42 into release/9.0-staging Nov 11, 2024
steveisok deleted the backport/pr-109381-to-release/9.0-staging branch November 11, 2024 15:46
carlossanlop modified the milestones: 9.0.1, 9.0.2 Nov 21, 2024
github-actions Bot locked and limited conversation to collaborators Dec 22, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Codegen-JIT-mono Servicing-approved Approved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants


Back | FazBrowse Home | New Git URL