endianness issue with FFI (`ext/ffi/tests/gh11934.phpt`)

[NattyNarwhal]

Description

Caught this when provisioning a big endian box for CI.

diff output on test:

--
     --- Struct ---
     bool(true)
     --- Callback return type ---
044- int(42)
044+ int(0)
     --- Other FFI\CData assignment ---
     int(123)
     --- Array element ---
--

The relevant part of that test:

echo "--- Callback return type ---\n";

$ffi = FFI::cdef('
typedef uint32_t (*test_callback)();
typedef struct {
        test_callback call_me;
} my_struct;
');

$struct = $ffi->new('my_struct');
$struct->call_me = function () use ($ffi) {
        $int = $ffi->new('uint32_t');
        $int->cdata = 42;
        return $int;
};

var_dump(($struct->call_me)());

Not familiar with FFI, so haven't looked into it yet, but making a note.

PHP Version

PHP 8.4.0-DEV

Operating System

Gentoo/ppc64

[cmb69]

For instance, the following looks fishy:

https://github.com/php/php-src/blob/ba748e7bb555d65dc30763ae64958452fbdf57c4/ext/ffi/ffi.c#L787-L790

[NattyNarwhal]

I think reading and writing to the cdata is OK; if I do i.e.

<?php

echo "--- Callback return type ---\n";

$ffi = FFI::cdef('
typedef uint32_t (*test_callback)();
typedef struct {
        test_callback call_me;
} my_struct;
');

$struct = $ffi->new('my_struct');
$struct->call_me = function () use ($ffi) {
        $int = $ffi->new('uint32_t');
        $int->cdata = 42;
        var_dump($int->cdata);
        return $int;
};

var_dump(($struct->call_me)());

...the $int->data var_dump will return 42. What seems broken is function return types. I recall vaguely facing this kind of issue with return types years ago, which made me less interested in using FFI.

[NattyNarwhal]

Oh, found it. Still holds on Linux, which makes it easier to debug.

[NattyNarwhal]

Breakpoint 1, zend_ffi_cdata_to_zval (cdata=0x0, ptr=0x3ffff7a030f0, type=0x1008900a0 <zend_ffi_type_sint32>, read_type=0, rv=0x3ffff7a150d0, 
    flags=0, is_ret=true, debug_union=false) at /home/calvin/php-src/ext/ffi/ffi.c:549
549 fprintf(stderr, "Reading pointer %p type %d\n", ptr, kind);
[...]
(gdb) p/x ((uint32_t*)ptr)[0]
$5 = 0x0
(gdb) p/x ((uint32_t*)ptr)[1]
$6 = 0x11223344

the classic

[NattyNarwhal]

I think the problem is types smaller than machine register size are widened to the size of registers; ffi_trampoline is already doing this with ret = do_alloca(MAX(ret_type->size, sizeof(ffi_arg)), ret_use_heap);. On little endian, this doesn't matter, because of how it gets laid out in memory, but it's fatal for big endian.

Since you can i.e. be working with actual 32-bit stuff referenced by pointer for a cdata and that works fine, I'm suspecting the fix might be to change the return FFI type before calling zend_ffi_cdata_to_zval to one that matches the machine width. I think values are widened in arguments too, so it might need to be done there too - except it's really ABI dependent.

[nielsdos]

And most annoyingly is that almost none of the core developers have access to big endian machines. A similar problem already exists for AArch64, but less problematic than big endian platforms.

[NattyNarwhal]

I can provide access to a BE machine for developers. I'm also going to use it for CI, but it should have plenty of spare resources.