Arm64-SVE: Fixes for passing Vector<T> by reference in reflection calls

[snickolls-arm]

Some reflection calls are triggering segmentation faults in testing due to missing some areas of support in the ArgIteratorTemplate.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[dotnet-policy-service]

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

[snickolls-arm]

@dotnet/arm64-contrib Please could I have a review for this patch? There is a single test failure in some filesystem test. It seems unlikely this would trigger that, but not impossible.

[Copilot]

Pull request overview

This PR updates CoreCLR’s ARM64 calling convention argument/return classification to correctly treat certain value types (notably scalable Vector<T> under SVE) as passed/returned by reference when used via reflection, preventing mis-marshalling that can lead to crashes.

Changes:

• In ArgIteratorTemplate::GetNextOffset, uses IsArgPassedByRef() (instead of a raw size check) to decide when to treat non-HFA valuetypes as byref (pointer-sized) arguments.
• In ArgIteratorTemplate::ComputeReturnFlags, adds an ARM64-specific check based on IsArgPassedByRef(TypeHandle) to decide whether a valuetype return should use a return buffer (covers the scalable Vector<T> special-case).

[jkotas]

Let me know if you would like to make any additional changes based on my comments.

I am ok with merging the PR as is.

[Copilot]

Pull request overview

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

[snickolls-arm]

I think this is suggesting there is an existing bug with varargs calling convention. The block above should not identify parameters as HFA if they are varargs?

[jkotas]

Yes, there seems to be a discrepancy. It would be best to confirm by a test that hits the problematic case.

[snickolls-arm]

I have added a test that should fail on Windows-Arm64. N.B. I have used an LLM to produce this and am using the PR to validate that it reproduces the error. We should see the varargs function call attempt to pass a struct larger than 16 bytes by value as HFA, and therefore have some information loss on that value.

[snickolls-arm]

The fix should be to just add the varargs check to the block above the one I've changed, but I will wait for the CI to verify the error is actually reproducible.

[jkotas]

The repro for the problem can a lot simpler: trigger GC (via GC stress) during JITing of vararg method that's called with the problematic argument and some GC references.

[snickolls-arm]

Just for my understanding, why is this area reachable by GC code? Does the GC walk the transition block to look for GC references?

I've added this GC test now, and will wait for the repro again before pushing the fix. The previous test also seems to reproduce the bug, so I think it's worth keeping, unless you think otherwise?

[jkotas]

Does the GC walk the transition block to look for GC references?

Yes. It typically happens when GC is triggered while the method is being JITed. The method arguments are pushed as transition block by prestub and they need to be reported to the GC.

The previous test also seems to reproduce the bug, so I think it's worth keeping, unless you think otherwise?

The profiler tests are complicated and expensive to run. They are not the best way to build regressions for corner-case VM issues.

[snickolls-arm]

The GC test reproduces with
Assert failure(PID 9132 [0x000023ac], Thread: 2460 [0x099c]): !CREATE_CHECK_STRING(!"Detected use of a corrupted OBJECTREF. Possible GC hole.").

So I've reverted the profiler test, and pushed 01e6544 to try and fix this.