`Buffer.Memmove` has unaligned casts

[xtqqczze]

#26654 (comment):

It is invalid to reinterpret cast a ref byte to a ref ushort/uint/ulong without first checking the alignment of the input buffer. This could cause problems on future architectures or IOT devices where unaligned reads incur faults. This code should pin and check alignment or should use Unsafe.WriteUnaligned instead.

cc:@GrabYourPitchforks

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.

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

---

#26654 (comment):

It is invalid to reinterpret cast a ref byte to a ref ushort/uint/ulong without first checking the alignment of the input buffer. This could cause problems on future architectures or IOT devices where unaligned reads incur faults. This code should pin and check alignment or should use Unsafe.WriteUnaligned instead.

cc:@GrabYourPitchforks

Author:	xtqqczze
Assignees:	-
Labels:	area-System.Runtime, untriaged
Milestone:	-

[EgorBo]

For such integer reads/writes we assume that OS will be able to fix them for us via alignment fixups. On OSes without such fixups we have plenty of potential problems as is, Unsafe.Read/WriteUnaligned won't help (the way they're implemented now in JIT). See #80068 (comment)

[tannergooding]

We likely should be using Unsafe.Read/WriteUnaligned anyways as it improves the overall portability of the code (e.g. on other Runtimes) and has no real negative impact on architectures with fast unaligned reads/writes (e.g. Arm64 and x86/x64).

[xtqqczze]

We likely should be using Unsafe.Read/WriteUnaligned anyways as it improves the overall portability of the code (e.g. on other Runtimes) and has no real negative impact on architectures with fast unaligned reads/writes (e.g. Arm64 and x86/x64).

@tannergooding We would regress when using ReadUnaligned and WriteUnaligned with HAS_CUSTOM_BLOCKS structs, as the copy would then go through the stack. Could we just simplify the implementation and use CopyBlockUnaligned with bytes counts of 2, 4, 8, 16, 64, resulting in the same codegen? (Compiler Explorer)

internal static void CopyBlock64(ref byte dest, ref byte src)
{
  Unsafe.As<byte, Block64>(ref dest) = Unsafe.As<byte, Block64>(ref src);
}
internal static void CopyBlock64Unaligned(ref byte dest, ref byte src)
{
  Unsafe.WriteUnaligned<Block64>(ref dest, Unsafe.ReadUnaligned<Block64>(ref src));
}
internal static void CopyBlockUnaligned(ref byte dest, ref byte src)
{
  Unsafe.CopyBlockUnaligned(ref dest, ref src, 64);
}

[tannergooding]

We would regress when using ReadUnaligned and WriteUnaligned with HAS_CUSTOM_BLOCKS structs, as the copy would then go through the stack

Block16, Block32, and Block64 aren't integer reads. They are opaque sized structs that go through what is effectively a "block copy". Since they have no fields, the packing/natural alignment of them is 1 and they aren't subject to the issue.