[mono][amd64] Fix calling-convention mismatch for small straddling vtypes in shared generics

[pavelsavara]

Summary

On amd64, a <= 16-byte value type whose nested field crosses the 8-byte SysV eightbyte boundary was passed on the stack as a byval struct, while a fully-flattened instantiation of the same layout is passed in two integer registers. These two classifications must agree, because partially-shared ("gshared") generic code calls the concrete method. Recent LLVM lowers small byval structs onto the stack instead of into registers, so the value is corrupted across the call. In practice this shows up only under Mono LLVM full-AOT on x64, e.g. boxing/unboxing a Nullable<struct-with-a-nullable-field> in shared generic code returns the wrong value.

Minimal repro

using System;
public struct GenQ<T> where T : struct { public T? Field; }   // 8 bytes; Nullable<GenQ<int>> is 12 bytes
public static class Repro {
    static bool Gen<T>(T? o) where T : struct {                // partially-shared generic
        var v = (GenQ<int>)(ValueType)(object)o;               // box T?, then unbox
        return !v.Field.HasValue;
    }
    static int Main() => Gen<GenQ<int>>(default(GenQ<int>)) ? 100 : 666;
}

• Mono JIT: returns 100.
• Mono LLVM full-AOT on x64 (recent LLVM): returns 666 — the boxed/unboxed value is corrupted.

Root cause

add_valuetype (mini-amd64.c) forces a value type onto the stack (ArgOnStack → LLVMArgVtypeByVal) when a nested field straddles the 8-byte eightbyte boundary. For Nullable<GenQ<int>> the value field is at offset 4, size 8, so it crosses byte 8.

The concrete, fully-flattened instantiation has no straddling field and is passed in two integer registers (ArgValuetypeInReg).

Partially-shared generic code calls the concrete method, so the conventions must match: the partially-shared caller sees the value type as an opaque type parameter (one straddling field, byval) while the concrete Nullable<T>:Box callee sees the flattened layout (registers). Older LLVM passed the small byval struct in registers so they matched; recent LLVM lowers byval onto the stack, so the caller writes the value to the stack while the callee reads it from registers → corruption.

Fix

For managed (non-P/Invoke) calls of a <= 16-byte value type, no longer force a straddling vtype onto the stack: let it fall through to the existing !sig->pinvoke classification, which splits the value into (at most) two integer eightbytes purely by total size. That split is layout-independent, so the opaque (gshared) view and the concrete (flattened) view produce the same ArgValuetypeInReg classification — and both the JIT and the LLVM backend honor it, so they agree regardless of how LLVM lowers byval.

P/Invoke, returns, and structs larger than 16 bytes keep the existing stack/byval ABI.

This replaces the earlier LLVM-only llvm_inreg_straddle approach (which fixed only the LLVM backend and left the JIT passing the value on the stack). The ArgInfo.llvm_inreg_straddle bitfield and its get_llvm_call_info special-case are removed, making the change a net simplification.

GC safety

Register-passing these values is safe for the GC: object references in a <= 16-byte managed value type are always 8-byte aligned (the type loader rejects misaligned or overlapped references), so an eightbyte never splits a managed pointer and conservative stack/register scanning still finds every reference. (A "pass reference-containing straddlers by-ref" variant was rejected — the concrete side passes <= 16-byte vtypes in registers, not by-ref, so by-ref would re-introduce the very mismatch this fixes.)

Scope — x64 only

• arm64 classifies by size (no straddle rule) and uses a register-array form (LLVMArgAsIArgs), so caller and callee already agree.
• x86 / arm / riscv have no straddle rule.
• wasm / llvmonly use a different, already-correct nullable-box path.

Validation

Built Mono + LLVM full-AOT on x64 and ran, in both JIT and full-AOT:

• the JIT/Directed/nullabletypes test sources (castclassvaluetype, covering every nullable type — int?, Guid?, the generic-struct cases, float?/double?/decimal?, GCHandle?, interface/explicit-layout structs): all pass;
• the minimal repro above (now 100 in both modes);
• a new JIT/Directed/StructABI/StraddlingVtypeAbi regression test (see below).

Negative check: with the pre-fix behavior restored (all straddlers on the stack), the new test and the repro fail under full-AOT (wrong unboxed value); with the fix they return 100. The JIT path now also passes these in registers, so LLVM↔non-LLVM calls agree.

New regression test

src/tests/JIT/Directed/StructABI/StraddlingVtypeAbi.cs covers the combinations the fix distinguishes: the gshared Nullable<GenQ<int>> box/unbox (the bug), an all-integer straddler, a float-field straddler, a direct by-value pass of a straddling Nullable through shared generic code, and a reference-containing <= 16-byte struct passed in registers through shared generic code across a GC.Collect (register GC-safety).

Alternatives considered

1. Patch the Mono LLVM fork to pass <= 16-byte INTEGER byval in registers (restore the old lowering). Uniform, but lives in dotnet/llvm-project and re-introduces reliance on non-standard byval register passing.
2. Route the shared nullable-box through a gsharedvt-out wrapper. Works for the fully-gsharedvt variant, but the hot path here is partially-shared (cfg->gsharedvt == 0), whose call passes the value byval (not by-ref), so the out-wrapper expects the wrong convention and faults.
3. LLVM-only llvm_inreg_straddle (the previous revision of this PR) — fixed the LLVM backend but left the JIT passing the value on the stack, so an LLVM↔JIT call could still disagree.
4. This change — fix the classification at its source so the JIT and LLVM backends both pass these in registers; smallest, no new state, and makes both sides agree regardless of LLVM's byval lowering.

Note

This pull request was prepared with the assistance of GitHub Copilot.

---

Part of the MonoAOT LLVM 23 full-AOT regression set tracked by #129508. Built and tested together with the Emscripten 5.0.6 / LLVM 23 bump on #129396, where the re-enabled nullabletypes tests (boxunboxvaluetype / castclassvaluetype) pass on the runtime-llvm AllSubsets_Mono_LLVMFULLAOT_RuntimeTests leg.

Contributes to #129508.

[dotnet-policy-service]

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

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

[BrzVlad]

I don't believe this is technically the right approach. We have inconsistency in the cconv with the JIT/non-llvm aot so this could still lead to problems, so the proper fix would be also make the non-llvm path pass these in regs as well.

As a low effort fix I believe this is fine, but I would double check that we don't regress any other tests.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

[pavelsavara]

We have inconsistency in the cconv with the JIT/non-llvm aot

@BrzVlad This helped me to realize that it would also create GC hole, thanks.
I gave it another try, differently.
And added more tests. We will see how it goes.

[pavelsavara]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.