Fix loading of R2R assemblies from byte arrays/streams

[VSadov]

Fixes:#71507

[ghost]

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

[ghost]

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

Issue Details

---

Fixes:#71507

Author:	VSadov
Assignees:	VSadov
Labels:	area-AssemblyLoader-coreclr
Milestone:	-

[janvorli]

I am not sure I understand this comment. Why would allocating executable memory not work on OSX?

[VSadov]

I think it would work if correct APIs are used. It is probably not as simple as just passing MAP_JIT to VirtualAlloc or just replacing VirtualAlloc with another call.
I want to backport this to 7.0, so I'd like to not do a lot of changes to how we convert layouts, which would carry extra risk to scenarios that are not being fixed.

The goal is to make sure that assemblies can be loaded from byte arrays even if they contain R2R code.
Whether the R2R is actually enabled is of less importance. It is a relatively rare case and a reflection scenario with unclear benefits from R2R.

Linux part was just tweaking the assert, but for OSX enabling R2R needs more changes. I think it is ok to do those changes in 8.0

[janvorli]

I agree that the change is fine as is.
Just FYI though, the VirtualAlloc sets MAP_JIT automatically for executable memory allocations. Either it gets memory from the g_executableMemoryAllocator.AllocateMemory or it sets the special flag that results in MAP_JIT addition during memory reservation:

runtime/src/coreclr/pal/src/map/virtual.cpp

Lines 923 to 927 in 6f69dde

	if ((flProtect & 0xff) == PAGE_EXECUTE_READWRITE)
	{
	flAllocationType |= MEM_RESERVE_EXECUTABLE;
	}
	pRetVal = ReserveVirtualMemory(pthrCurrent, (LPVOID)StartBoundary, MemSize, flAllocationType);

[VSadov]

Does it mean that I could just pass PAGE_EXECUTE_READWRITE to VirtualAlloc on OSX?

[janvorli]

Yes

[VSadov]

I tried going that path (setting PAGE_EXECUTE_READWRITE), but I think I also need to toggle PAL_JitWriteProtect when copying sections and then the change started making me worried about the complexity and possible bugs.

I think I will leave that for after-7.0, we do not have to do this now.

[janvorli]

Right (for both)

[VSadov]

I've logged #74203 to follow up with enabling R2R on OSX

[jkotas]

Loading from byte arrays/streams is expected to ignore R2R payload if there is one.

Respecting R2R payload in this scenario has a dubious value (you are giving up perf advantage of memory mapped files) and there are number of problems that would be have to be fixed to make it work end-to-end well (don't forget diagnostic tools).

[VSadov]

Loading from byte arrays/streams is expected to ignore R2R payload if there is one.

I was not sure about this primarily because it seems to work on Windows and on Linux (modulo an assert) and probably worked in past releases. "work" here is basically "code runs". I do not know what the situation with diagnostics etc.
It does seem to have dubious value, since loading from a byte array is hardly a startup scenario and avoiding JIT is unlikely a big win.

I guess this is enough controversy to just disable R2R in this scenario on all platforms.
I'll add a note to the follow-up issue to revisit later - whether there is any benefit at all.

[VSadov]

Thanks!!

[VSadov]

/backport to release/7.0-rc1

[github-actions]

Started backporting to release/7.0-rc1: https://github.com/dotnet/runtime/actions/runs/2893425031