Skip to content

Conversation

kg
Copy link
Member

@kg kg commented Aug 22, 2025

Currently if you pass a VT argument with a size that is not a multiple of 8, the interp->native transition will misalign its stack offset and start loading garbage into registers after it processes the VT. The native->interp transition is handled correctly because the Store_Ref_XXX helpers already align offsets. The Load_Ref_Reg helpers don't do alignment, unlike Load_Stack_Ref which does.

This PR tweaks the callstub generator so that when generating a list of load routines, it aligns up the size of any VTs to maintain the alignment of the stack offsets. This is unnecessary for stack refs but harmless there. Store routines are left alone.

This PR also adds a new DOTNET_InterpHaltOnCall environment var that lets you make the interpreter assert when compiling calls to a specific method. It was helpful to have this when investigating this issue.

This fixes an issue in JIT\SIMD\Vector3Interop_ro\Vector3Interop_ro (but it still doesn't pass, due to a GC hole much later in the test)

Add DOTNET_InterpHaltOnCall to make the interpreter assert when compiling a call to a specified method
@Copilot Copilot AI review requested due to automatic review settings August 22, 2025 19:01
@kg kg requested review from BrzVlad and janvorli as code owners August 22, 2025 19:01
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 a stack alignment issue in the CoreCLR interpreter when passing Vector3 arguments from interpreter to native code. The issue occurs when value types (VTs) with sizes not divisible by 8 bytes cause misaligned stack offsets, leading to garbage data being loaded into registers.

  • Aligns VT argument sizes to 8-byte boundaries for interpreter-to-native transitions
  • Adds debugging support via DOTNET_InterpHaltOnCall environment variable
  • Maintains correct behavior for native-to-interpreter transitions which already handle alignment

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/coreclr/vm/callstubgenerator.cpp Implements stack alignment fix for VT arguments in interpreter-to-native calls
src/coreclr/interpreter/interpconfigvalues.h Adds configuration for InterpHaltOnCall debugging feature
src/coreclr/interpreter/compiler.cpp Implements assertion trigger for InterpHaltOnCall debugging

Copy link
Contributor

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

Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@kg kg force-pushed the interp-vector3interop branch from 9b08e1d to 6973365 Compare August 22, 2025 20:49
@kg kg merged commit 6d9cd59 into dotnet:main Aug 22, 2025
96 of 98 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Sep 22, 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