Skip to content

Disable IL offset mapping validation #117809

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
merged 1 commit into from
Jul 21, 2025
Merged

Conversation

jakobbotsch
Copy link
Member

@davidwrighton suggested just disabling this in #117561 (comment).

Makes #117561 non blocking.

Some additional context from the case in #117561 (comment):

I looked briefly and it looks like the old logic considers the last range of the IL mappings to be [184, 220) while the new logic considers the last range to be [184,261). Hence the old logic finds no mapping while the new logic does.

It's quite convoluted, but I get the impression that the intention of the old code is to ignore the CALL_INSTRUCTION mapping completely and that the new code is actually implementing the intention of the old code more properly.

I will do what @davidwrighton suggested and disable the validation for now since this is failing all our jitstress runs.

@Copilot Copilot AI review requested due to automatic review settings July 18, 2025 11:48
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jul 18, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR disables IL offset mapping validation in the CoreCLR VM to address failures in jitstress runs. The change is a temporary workaround to unblock testing while the underlying IL offset mapping logic is investigated and fixed.

Key changes:

  • Temporarily disables IL offset mapping validation in debug builds
  • Removes a duplicate assignment that was causing incorrect offset tracking
  • Adds reference to the GitHub issue for context

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/coreclr/vm/jitinterface.cpp Comments out the ValidateILOffsets call with explanatory comment
src/coreclr/vm/debugdebugger.cpp Removes duplicate assignment of dwCurrentNativeOffset

@jakobbotsch jakobbotsch added area-Diagnostics-coreclr and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jul 18, 2025
Copy link
Contributor

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

@jakobbotsch
Copy link
Member Author

cc @dotnet/dotnet-diag, @davidwrighton, @noahfalk can you review since you looked at the original PR?

Copy link
Member

@hoyosjs hoyosjs left a comment

Choose a reason for hiding this comment

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

It it's blocking - this sounds OK. But I see only one hit?

@jakobbotsch
Copy link
Member Author

It it's blocking - this sounds OK. But I see only one hit?

This blocks optional pipelines that aren't tracked by build analysis. E.g. https://dev.azure.com/dnceng-public/public/_build?definitionId=109 has widespread failures from the validation asserting.

@jakobbotsch
Copy link
Member Author

/ba-g Helix timeout

@jakobbotsch jakobbotsch merged commit 68cb956 into dotnet:main Jul 21, 2025
98 of 100 checks passed
@jakobbotsch jakobbotsch deleted the fix-117561 branch July 21, 2025 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants