This will need #122988 (comment) to deal with the stack_pointer

Yes. I've implemented this for the calls into the interpreter stubs now, but I was holding off on finishing this up for FCalls until my next PR, as its not really needed for initial bring up, and the PR is already way too large.

GCPROTECT_BEGINCONSERVATIVE_ARRAY(args, ...) is protecting the local pointer variable args (i.e., &args) rather than the argument buffer it points to. This means the GC frame will conservatively scan unrelated stack memory adjacent to the pointer variable, not the actual argument slots, which can lead to incorrect root reporting and potential GC corruption/instability.

Instead, create a GCFrame over the actual buffer address (e.g., GCFrame gcFrame((OBJECTREF*)args, count, GC_CALL_INTERIOR | GC_CALL_PINNED)), or introduce a dedicated helper/macro for protecting a dynamic pointer+count buffer rather than using a macro that takes an array variable.

On TARGET_WASM, the loop condition dirPos < (dirSize - sizeof(IMAGE_BASE_RELOCATION)) can underflow when dirSize < sizeof(IMAGE_BASE_RELOCATION) (since COUNT_T is unsigned), leading to an out-of-bounds read of IMAGE_BASE_RELOCATION.

Consider rewriting the condition as dirPos + sizeof(IMAGE_BASE_RELOCATION) <= dirSize (or guarding with if (dirSize < sizeof(...)) return;) to avoid unsigned underflow.

SetPrefersInterpreterEntryPoint / ClearPrefersInterpreterEntryPoint update _flags via plain read/modify/write (_flags = _flags | ... / _flags = _flags & ...). Since _flags is also used for other state (e.g., UnmanagedCallersOnly flags) and can be accessed concurrently, this can drop updates or tear state.

Please switch these to interlocked bit operations (e.g., InterlockedOr / InterlockedAnd) similar to the existing SetFlags helper used elsewhere in this file.

Typo in comment: stack poitner should be stack pointer. (Also consider whether the comment should mention the portableEntryPointContext parameter name consistently.)

The comment says the type characters in these pregenerated signatures should match SignatureMapper.CharToNativeType, but the added signatures include a 'p' suffix, which is not a valid type character for SignatureMapper and is handled specially elsewhere.

Please update the comment to document 'p' as a portable-entrypoint marker (hidden parameter) so future additions don't follow the comment literally and break generation.

Minor style inconsistency: this file otherwise uses string[] but this new declaration uses string []. Consider switching to string[] to match the surrounding conventions.

The enum comment says WASM_MEMORY_ADDR_REL_LEB is a "varint32", but the relocation is encoded/decoded using ULEB128 (WritePaddedULEB128/ReadULEB128) and is described as varuint32 elsewhere.

Please fix the comment to avoid implying signed encoding.

The enum comment describes WASM_MEMORY_ADDR_REL_LEB as a "varint32", but the writer uses ULEB128 for this relocation type. Please update the comment to varuint32 (or clarify why it is signed) to avoid misleading JIT/objwriter implementers.

This is not reliable. CoreCLR is not compatible with general conservative reporting - it will cause dynamic methods and collectible assemblies to crash intermittently.

We can only conservatively double-report (I believe it is what the interpreter does). We cannot conservatively report random pointers.

@jkotas, I disagree. I'm pretty sure this IS safe, but it may not be sufficient. Notably, calls to LCG/collectible code may ALSO need to report a loaderallocator/LCG method. I'd prefer to add to that detail, than try to figure out how to report exactly here.

ALSO need to report a loaderallocator/LCG method

That's not the only problem. Conservative reporting like this causes dead objects to become alive. Collectible code is not robust against that. Sure, it can be fixed with more work but I am not sure whether we want to do it.

Example of sequence of events that causes problem:

1. The loader allocator is identified as dead and it is scheduled for cleanup
2. Some conservatively reported root resurrects an object from the loader allocator that is scheduled for cleanup
3. The loader allocator is cleaned up
4. The resurrected object is still on the GC heap, but points to a MethodTable that does not exist anymore
5. The GC crashes accessing the non-existing MethodTable when it tries to walk the heap

Also, conservative reporting has been a source of unpredictable/flaky behavior in Mono, sometimes even leading to OOMs, for example dotnet/aspnetcore#23451. It would be nice to avoid that on CoreCLR.

than try to figure out how to report exactly here.

I think it should be cheap and straightforward to do the proper reporting once the transition thunks are produced by crossgen.

I don't agree. The issues with conservative gc resurrecting dead objects were fixed some years ago. (It was one of Chris Ahna's more subtle finds.) The only issues remaining are keeping things method frames live, and the general problems of having a conservative gc which can lead to ooms, etc. given the current expectation around around codegen, I don't see it as useful to avoid conservative gc here at this time.

So what prevents the sequence of events in my comment above from crashing the GC?

It was one of Chris Ahna's more subtle finds.

That was in .NET Native that did not support collectible types (if you mean the same set of problems that I have in mind)

(I am fine with using conservative GC as a bring up aid, as long as it is wrapped with TODOs that link to issue.)

@jkotas That find, which Chris found on .NET Native, was also very interesting as my analysis at the time indicated that it actually addressed a number of issues that had been coming up in collectible assemblies for years but we had never managed to fix/explain to my satisfaction. Since then I haven't seen collectible assemblies fail in a way which would have been triggered by using conservative GC. Effectively, collectible assemblies had been relying on a property of garbage collection which should have always been true around marking dead objects as dead reliably, but had a very obscure bug in the GC, and when Chris fixed the issue in object liveness computation, this fundamentally fixed the problems that collectible assemblies had around walking over objects that had notionally been collected, but had not yet quite properly been considered free in the GC. The issue had been explained away for collectible assemblies as a detail that the GC sometimes walked over dead object memory that might not yet been marked as a Free region.

Effectively, Chris's fix was to the ability of the GC to determine if an object was live, and that is the fundamental
issue that collectible assemblies had had for years. It so happens that conservative GC has exactly the same requirements for correctness, but the failure mode with conservative GC was rather easier to understand (and frankly, Chris was never brought in to deal with the collectible types problems, and he is so intensely good at tracking these things down.)

Also, the current model for GC reporting on the interpreter in coreclr has a requirement on conservative GC, not just safe double-reporting. This is due to some rather awkward details around the CALL, and various call helpers instructions work. To fix this completely would require splitting the existing CALL/CALLVIRT/CALL_HELPER instructions into several separate instructions, and introducing a variety of different GC reporting rules at each. As noted when we put it together, it's a limited time with the conservative report on a given memory address, but it IS true conservative reporting where sometimes wild pointers are fed into the GC.

Regardless, we could use the PrestubMethodFrame here, which would be fairly reasonable. I'll look into it.

InterpreterToR2RSignatures

GCPROTECT_BEGINCONSERVATIVE_ARRAY(args, ...) is passing a pointer variable (int8_t* args) into a macro that takes the address of its argument. This ends up protecting the local pointer (int8_t**) rather than scanning/protecting the argument buffer contents, so GC refs in the args buffer can move during DoPrestub. Consider passing an lvalue representing the first element of the buffer (e.g., ((OBJECTREF*)args)[0]) or constructing a GCFrame directly over (OBJECTREF*)args (and also guard for args == nullptr / argsSize == 0).

The comment describing pregeneratedInterpreterToNativeSignatures says the type characters match SignatureMapper.CharToNativeType, but these signatures include the 'p' suffix which is handled as a portable-entrypoint marker (and is not recognized by CharToNativeType). Update the comment to reflect that 'p' is a special suffix, not a regular type code.

RhpNewArrayFast is declared via FCDECL2, which expands to a 2-arg function on non-WASM targets. This direct call with 4 arguments will not compile outside TARGET_WASM (and also makes the callsite inconsistent with the declaration on other platforms). Consider wrapping the extra stack_pointer / portableEntryPointContext arguments in #ifdef TARGET_WASM, or providing a small helper/macro that supplies the right argument list per target.

On WASM, the loop condition dirPos < (dirSize - sizeof(IMAGE_BASE_RELOCATION)) can underflow when dirSize < sizeof(IMAGE_BASE_RELOCATION) because dirSize is unsigned. That would turn the bound into a huge value and can lead to out-of-bounds reads. Use a form like dirPos + sizeof(IMAGE_BASE_RELOCATION) <= dirSize (and keep the existing padding handling) to avoid unsigned underflow.

type.GetNestedTypes() without binding flags returns only public nested types. This will skip InternalCall methods on non-public nested types. Use type.GetNestedTypes(BindingFlags.Public | BindingFlags.NonPublic) (and consider BindingFlags.DeclaredOnly if desired) to ensure the scan is complete.

The comment describing pregeneratedInterpreterToNativeSignatures says all characters match SignatureMapper.CharToNativeType, but these signatures now include the trailing 'p' portable-entrypoint marker (which is not a type char and isn't handled by CharToNativeType). Please update the comment to document the 'p' suffix convention (and ideally reference where it's interpreted) to avoid confusion.

ComputeCalliSigThunk still allows non-default calling conventions (C/stdcall/fastcall/unmanaged), but GetSignatureKey only appends the 'p' marker for IMAGE_CEE_CS_CALLCONV_DEFAULT. With the thunk table updates, several floating-point shapes now only exist in the ...p form (e.g., ddp, ffp), so a non-default calli with double/float args/returns would compute keys like dd/ff and fail lookup. Either restrict the supported calling conventions here (return NULL for non-default until thunks exist), or ensure the thunk table/keying scheme continues to cover those non-default shapes.

The thunk map no longer contains non-p floating-point signatures (e.g., dd, ddd, ff, etc.), but helpers.cpp can still compute and look up keys without the 'p' suffix for non-default calli calling conventions. If any such calli is exercised (C/stdcall/fastcall/unmanaged), it will now fail to find a thunk. Consider generating/keeping both the portable-entrypoint (...p) and non-portable variants for these float/double shapes, or tightening the runtime to reject non-default calling conventions until supported.

Call the actual _IMPL method? I think this is going to overwrite the stack pointer with zero otherwise.