JIT: Fix too wide loads on arm64 for small structs

[EgorBo]

Fixes #76194
using @SingleAccretion's snippet since mine was uglier.

We've had similar issues in the past e.g. #64683 (reproduces even on .NET Framework x64)
it's just that normally it's an extremely rare case, perhaps, still worth backporting to .NET 7.0

[ghost]

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

Issue Details

---

Fixes #76194
using @SingleAccretion's snippet since mine was uglier.

We've had similar issues in the past e.g. #64683 (reproduces even on .NET Framework)
it's just that normally it's an extremely rare case, perhaps, still worth backporting to .NET 7.0

Author:	EgorBo
Assignees:	-
Labels:	area-CodeGen-coreclr
Milestone:	-

[EgorBo]

PTAL @jakobbotsch @SingleAccretion

[SingleAccretion]

Suggested change

	LIR::Use retValUse(BlockRange(), &ret->gtOp1, ret);
	unsigned tmpNum = BAD_VAR_NUM;
	if (structCls != NO_CLASS_HANDLE)
	{
	tmpNum = comp->lvaGrabTemp(true DEBUGARG("mis-sized struct return"));
	comp->genReturnLocal = tmpNum;
	comp->lvaSetStruct(tmpNum, structCls, true);
	}
	LIR::Use retValUse(BlockRange(), &ret->gtOp1, ret);
	unsigned tmpNum = comp->lvaGrabTemp(true DEBUGARG("mis-sized struct return"));
	comp->lvaSetStruct(tmpNum, structCls, false);

1. genReturnLocal has a very specific purpose, no reason to set it here.
2. No unsafe value class checks needed (in principle, in practice it doesn't matter) since the address of this local won't escape.

[EgorBo]

genReturnLocal has a very specific purpose, no reason to set it here.

Tests assert after this patch: Assertion failed 'tmp == genReturnLocal'

[EgorBo]

I reverted some suggestions if you don't mind because we hit the path for non-structs as well (e.g. small type primitives) so now it's cleaner that we have a special case for that rare case with IND<struct>.

[SingleAccretion]

We still need to handle the case with handle-less BLK nodes though.

Reproduction:

struct Sx3
{
    public byte A, B, C;
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static Sx3 Problem(void* s)
{
    Sx3 a;
    Unsafe.CopyBlock(&a, s, 3);
    return a;
}

[EgorBo]

thanks, reverted the revert. @SingleAccretion does it look good to you? I assume it's just your change written with my hands 😄

[SingleAccretion]

Yep, looks good!

[EgorBo]

@jakobbotsch could you please take a look and sign it off?

CI failure is known: #78290

[jakobbotsch]

I assume it is guaranteed that structCls != NO_CLASS_HANDLE here or we would have hit some BADCODE previously? (e.g. returning struct in method declared to return int)

[EgorBo]

Not that the current behavior tries to be careful in such cases, I've just tried and got an AccessViolationException in Main for it when I changed return type to int while was trying to return a small struct.

[EgorBo]

LLVMAOT failure was known and already fixed in Main

[jakobbotsch]

This looks like it will result in a double free

[jakobbotsch]

(But I doubt that's the cause of #78758)

[jakobbotsch]

Ah, I see the reason now:

On success, mmap() returns a pointer to the mapped area.  On
       error, the value MAP_FAILED (that is, (void *) -1) is returned,
       and [errno](https://man7.org/linux/man-pages/man3/errno.3.html) is set to indicate the error.

So this does not correctly check that we failed to allocate the memory.