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-staging] [android] Fix crash in method_to_ir #109511

Merged

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Nov 4, 2024

Backport of #109381 to release/8.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
Contributor

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

@steveisok steveisok added the Servicing-consider Issue for next servicing release review label Nov 5, 2024
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.

lgtm. we will take for consideration 8.0.x

@jeffschwMSFT jeffschwMSFT added this to the 8.0.x milestone Nov 5, 2024
@rbhanda rbhanda added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Nov 5, 2024
@rbhanda rbhanda modified the milestones: 8.0.x, 8.0.12 Nov 5, 2024
@steveisok steveisok merged commit c489a8c into release/8.0-staging Nov 8, 2024
105 of 112 checks passed
@steveisok steveisok deleted the backport/pr-109381-to-release/8.0-staging branch November 8, 2024 13:50
@github-actions github-actions bot locked and limited conversation to collaborators Dec 9, 2024
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.

4 participants