Skip to content

Fix interpreted synchronized methods#121020

Merged
janvorli merged 1 commit into
dotnet:mainfrom
janvorli:fix-interpreter-synchronized-methods
Oct 23, 2025
Merged

Fix interpreted synchronized methods#121020
janvorli merged 1 commit into
dotnet:mainfrom
janvorli:fix-interpreter-synchronized-methods

Conversation

@janvorli

Copy link
Copy Markdown
Member

The InterpCompiler::CreateBasicBlocks was comparing incorrect offset with m_ILCodeSizeFromILHeader when checking when to convert CEE_RET to CEE_LEAVE. The m_currentILOffset it was using was uninitialized, so sometimes the comparison passed, sometimes not based on the random value in the m_currentILOffset.

The InterpCompiler::CreateBasicBlocks was comparing incorrect offset
with m_ILCodeSizeFromILHeader when checking when to convert CEE_RET to
CEE_LEAVE. The m_currentILOffset it was using was uninitialized, so
sometimes the comparison passed, sometimes not based on the random value
in the m_currentILOffset.
@janvorli janvorli added this to the 11.0.0 milestone Oct 23, 2025
@janvorli janvorli self-assigned this Oct 23, 2025
@janvorli janvorli requested review from BrzVlad and kg as code owners October 23, 2025 14:00
Copilot AI review requested due to automatic review settings October 23, 2025 14:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 fixes a bug in the interpreter's basic block creation for synchronized methods where an uninitialized variable was being used to determine when to convert CEE_RET instructions to CEE_LEAVE instructions. The fix ensures the correct offset variable is used in the comparison.

  • Replaces use of uninitialized m_currentILOffset with the properly initialized insOffset variable in the condition check

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

Comment thread src/coreclr/interpreter/compiler.cpp
@davidwrighton

Copy link
Copy Markdown
Member

Oops. Sorry about that.

@janvorli janvorli merged commit c1384bb into dotnet:main Oct 23, 2025
98 checks passed
@janvorli janvorli deleted the fix-interpreter-synchronized-methods branch October 23, 2025 19:36
@github-actions github-actions Bot locked and limited conversation to collaborators Nov 23, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants