[ffi][riscv] CopyFromCompoundToStack and PopFromStackToTypedDataBase confused by struct packing

[rmacnak-google]

CopyFromCompoundToStack and PopFromStackToTypedDataBase compute a rolling byte offset based on the size of Representations seen so far. This doesn't correctly account for pieces whose Representation is widened compared to the piece's size (because our IL doesn't really support Representations smaller than 32-bit), nor would it account for varying alignments in pieces.

For example, in PassStruct9BytesPackedMixedx10DoubleInt32x2 passing a packed struct { uint8_t, double }

        ;; v49 <- LoadField(v3 T{*} . _typedDataBase@8050071 {final}) T{Object?}
0x7f8f8a2aaf6c    0072be03               ld t3, 7(t0)
        ;; v51 <- LoadUntagged(v49, 8) T{*?}
0x7f8f8a2aaf70    007e3283               ld t0, 7(t3)
        ;; v54 <- LoadIndexed(v51, v53) T{_Double}
0x7f8f8a2aaf74    0042b007               fld ft0, 4(t0)
...
        ;; ParallelMove
0x7f8f8a2ab0de    22000553               fmv.d fa0, ft0

The double is incorrectly loaded from offset 4 instead of offset 1.

It looks like the relevant information is gone by time CopyFromCompoundToStack runs. Even its caller FfiCallConvertCompoundArgumentToNative has only a MultipleNativeLocation that knows the eventual destination is a 8-byte cpu register and a 8-byte fpu register, but doesn't seem to have any description of the struct fields these values are meant to come from.

[sstrickl]

As expected re: Ryan's note in the gardening log for yesterday, we saw the following failures on dartkp-linux-release-riscv64-qemu:

ffi_2/function_callbacks_structs_by_value_generated_test/0 RuntimeError (expected Pass)
ffi_2/function_callbacks_structs_by_value_generated_test/1 RuntimeError (expected Pass)
ffi_2/function_callbacks_structs_by_value_generated_test/2 RuntimeError (expected Pass)
ffi_2/function_callbacks_structs_by_value_generated_test/3 RuntimeError (expected Pass)
ffi_2/function_structs_by_value_generated_leaf_test/0 RuntimeError (expected Pass)
ffi_2/function_structs_by_value_generated_leaf_test/1 RuntimeError (expected Pass)
ffi_2/function_structs_by_value_generated_leaf_test/2 RuntimeError (expected Pass)
ffi_2/function_structs_by_value_generated_leaf_test/3 RuntimeError (expected Pass)
ffi_2/function_structs_by_value_generated_test/0 RuntimeError (expected Pass)
ffi_2/function_structs_by_value_generated_test/1 RuntimeError (expected Pass)
ffi_2/function_structs_by_value_generated_test/2 RuntimeError (expected Pass)
ffi_2/function_structs_by_value_generated_test/3 RuntimeError (expected Pass)

[dcharkes]

RepsInFfiCall computes how the splitting is done. The assumptions are the following if I remember correctly:

• The bytes on the stack should be the same as in the typed data, it's the same struct.
• The TypedData is overapproximated to word-size. For 9 bytes, 12 or 16 bytes are allocated.

These two assumptions enable copying the chunks in the struct on a word-size basis.
The struct fields itself can be ignored while copying, we're just copying a multiple of of bytes.
Only when copying from the TypedData to the stack we must take care not to write the extra bytes, as we might override other arguments on the stack when tightly packed.

edit:

only a MultipleNativeLocation that knows the eventual destination is a 8-byte cpu register and a 8-byte fpu register

Ah, RISC-V might be the first calling convention which splits packed members out over multiple registers in a way that we can't just treat the bytes as bytes. (All other calling conventions pass structs as fully on stack or as a pointer to a memory location) when they have misaligned members.)

Then, this requires a bit more refactoring.

doesn't seem to have any description of the struct fields these values are meant to come from

MultipleNativeLocation has payload_type which is NativeCompoundType. For structs, this is the subtype NativeCompoundType which as the member member_offsets.