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/9.0-staging] JIT: Include more edges in BlockDominancePreds to avoid a JIT crash #110568

Open
wants to merge 5 commits into
base: release/9.0-staging
Choose a base branch
from

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Dec 10, 2024

Backport of #110531 to release/9.0-staging

/cc @jakobbotsch

Customer Impact

  • Customer reported
  • Found internally

Certain patterns of exception handling (involving nested filters, try-finally and try-catch) can cause the JIT to crash when the method is jitted with optimizations enabled. Reported by customer in #109981.
The problem occurs when:

  1. We have an outer try-filter
  2. Inside the try-filter we have a try-catch
  3. Inside the try-catch we have a try-finally
  4. The function has a loop in it
  5. The try-catch (2) is proven unreachable at a late point in the JIT's optimization phases

See the added test case for a C# example.

In this case the JIT computes a wrong dominator block for the catch-handler (2), which later causes a null pointer dereference.

Regression

  • Yes
  • No

The issue was introduced in #94672.

Testing

Added a test case that hits the crash without the fix. Also verified the fix on the customer's test case.

Risk

Low.

Because of spurious flow it is possible that the preds of the try-begin
block are not the only blocks that can dominate a handler. We handled
this possibility, but only for finally/fault blocks that can directly
have these edges. However, even other handler blocks can be reachable
through spurious paths that involves finally/fault blocks, and in these
cases returning the preds of the try-begin block is not enough to
compute the right dominator statically.
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Dec 10, 2024
Copy link
Contributor

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

@jakobbotsch jakobbotsch changed the title [release/9.0-staging] JIT: Include more edges in BlockDominancePreds [release/9.0-staging] JIT: Include more edges in BlockDominancePreds to avoid a JIT crash Dec 10, 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. please get a code review. we will take for consideration in 9.0.x

@jeffschwMSFT jeffschwMSFT added the Servicing-consider Issue for next servicing release review label Dec 10, 2024
@jeffschwMSFT jeffschwMSFT added this to the 9.0.x milestone Dec 10, 2024
@jeffschwMSFT jeffschwMSFT added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Dec 11, 2024
@jeffschwMSFT jeffschwMSFT modified the milestones: 9.0.x, 9.0.2 Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants