Skip to content

[release/8.0-staging] [mono][interp] Fix incorrect stack type information #94966

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

Merged

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Nov 19, 2023

Backport of #94923 to release/8.0-staging

/cc @lewing @BrzVlad

Customer Impact

In certain situations, the interpreter would devirtualize to wrong method, leading usually to crashes. This seems to be more likely to happen in situations involving the ternary conditional operator (where the two paths produce different object types) followed by a virtual call. This is also a regression from .net 7.

Testing

Tested the fix on testcase reproducing the issue. CI tests for any potential regressions.

Risk

Low risk. Fix only removes type information in a certain situation, fix should have few side effects.

Assume we have a basic block that it is a forward branch destination, then its stack type information will be initialized at the moment of branching (let's say there is a value of type Obj1). If later in the code we reach this bblock by falling through (let's say the current stack contains a value of Obj2), the current stack state will be copied from the original state, with the type Obj1. If later on we do a virtual call, we will try to devirtualize it as if this is Obj1 which is incorrect, since the fallthrough path would produce an Obj2.

This commit adds missing checks for removing type information if we have different types on the execution types on incoming paths.
@ghost
Copy link

ghost commented Nov 19, 2023

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

Issue Details

Backport of #94923 to release/8.0-staging

/cc @lewing @BrzVlad

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-Codegen-Interpreter-mono

Milestone: -

@lewing lewing added the Servicing-consider Issue for next servicing release review label Nov 19, 2023
@lewing lewing added this to the 8.0.x milestone Nov 19, 2023
@leecow leecow added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Nov 21, 2023
@leecow leecow modified the milestones: 8.0.x, 8.0.1 Nov 21, 2023
@carlossanlop carlossanlop merged commit 3e1f672 into release/8.0-staging Nov 21, 2023
@carlossanlop carlossanlop deleted the backport/pr-94923-to-release/8.0-staging branch November 21, 2023 20:07
@github-actions github-actions bot locked and limited conversation to collaborators Dec 22, 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.

4 participants