On x64 platform, "stelem.ref" ignores high bits of "native int" index

[kevinjwz]

IndexOutOfRangeException not thrown when index=0x100000000.
sharplab test

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[teo-tsirpanis]

Judging from the disassembly, the stelem.ref instruction calls System.Runtime.CompilerServices.CastHelpers.StelemRef helper method, which takes the index as a 32-bit integer. The index's high part is thrown away becoming zero, and the runtime happily stores your object to the array's zeroth position.

[GrabYourPitchforks]

Regression from 3.1, introduced in 5.0.

[GrabYourPitchforks]

Repathed to VM area, because that really seems like the closest thing to the true area owners.
/cc @VSadov

[SingleAccretion]

Judging from the disassembly, the stelem.ref instruction calls System.Runtime.CompilerServices.CastHelpers.StelemRef helper method, which takes the index as a 32-bit integer. The index's high part is thrown away becoming zero, and the runtime happily stores your object to the array's zeroth position.

That's correct. It looks like we just import a call to the helper, and it is the helper's responsibility to check out of range conditions, which it does. I am wondering what would be needed here beyond changing the helper's signature to take nint instead of int.

[GrabYourPitchforks]

I am wondering what would be needed here beyond changing the helper's signature to take nint instead of int.

That's why I pathed it under VM. Not sure if we require JIT changes to ensure 32-bit int indices (the 99.9999% case) are properly native word size extended before invoking the helper method. The VM area owners would know for sure.

[teo-tsirpanis]

I also noticed that the native-sized index is first conv.i8ed. Is that done on purpose? The documentation of stelem.ref says that the index is of type native int. Why first convert it to an int64?

[SingleAccretion]

Not sure if we require JIT changes to ensure 32-bit int indices (the 99.9999% case) are properly native word size extended before invoking the helper method.

I suspect changes will be needed, heh (looks like we don't sign extend right now), or an alternative version of the helper. Another note: seems like ldelema has the same problem.

[VSadov]

When I am running the Stelem_Ref repro with 3.1, I get the same results as on 5.0. I do not think this is a regression.

The previous implementation of the helper in x64 assembly truncated the index to 32bit as well - https://github.com/dotnet/runtime/pull/32722/files#r633023865

[am11]

It does not throw from Stelem_I4 either on 3.1 (linux-x64).

[davidwrighton]

I've looked into this. We can't just extend the helper to take a nint, we'll need to have a parallel helper, or force the index to be sign extended to nint. The abi of several of our platforms doesn't reliably set the high bits to 0 when passing 32bit values.