Skip to content

Conversation

janvorli
Copy link
Member

@janvorli janvorli commented Aug 7, 2025

This change fixes two issues:

  • INTOP_CALL_HELPER_P_S was taking arguments from wrong locations in the instruction
  • The total stack size computation in the call stub generator was incorrect in case arguments smaller than 8 bytes were passed on the stack on macOS ARM64 due to its special calling convention.

This change fixes two issues:
* INTOP_CALL_HELPER_P_S was taking arguments from wrong locations in the instruction
* The total stack size computation in the call stub generator was incorrect in case
  arguments smaller than 8 bytes were passed on the stack.
@janvorli janvorli added this to the 11.0.0 milestone Aug 7, 2025
@janvorli janvorli requested a review from davidwrighton August 7, 2025 08:04
@janvorli janvorli self-assigned this Aug 7, 2025
@Copilot Copilot AI review requested due to automatic review settings August 7, 2025 08:04
@janvorli janvorli requested review from BrzVlad and kg as code owners August 7, 2025 08:04
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 fixes two critical bugs in the CoreCLR interpreter and call stub generator that could cause incorrect execution behavior.

  • Fixes argument ordering bug in INTOP_CALL_HELPER_P_S instruction handling
  • Corrects stack size computation in call stub generator to avoid double counting

Reviewed Changes

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

File Description
src/coreclr/vm/interpexec.cpp Swaps ip[2] and ip[3] arguments in INTOP_CALL_HELPER_P_S to fix incorrect parameter ordering
src/coreclr/vm/callstubgenerator.cpp Removes duplicate stack size calculations and uses SizeOfArgStack() for accurate total

Copy link
Contributor

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

@janvorli
Copy link
Member Author

janvorli commented Aug 7, 2025

@davidwrighton this fixes the interpreter test issue that occurred in your PR.

@janvorli janvorli merged commit 0234029 into dotnet:main Aug 7, 2025
95 checks passed
@janvorli janvorli deleted the fix-interpreter-helper-and-stack-size branch August 7, 2025 11:48
@github-actions github-actions bot locked and limited conversation to collaborators Sep 7, 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.

2 participants