extism / php-sdk

Extism PHP Host SDK - easily run WebAssembly modules / plugins from PHP applications
https://extism.org
BSD 3-Clause "New" or "Revised" License
20 stars 4 forks source link

Make Host Functions compatible with PHP 7 #22

Closed mhmd-azeez closed 4 months ago

mhmd-azeez commented 4 months ago

When running the tests with PHPUnit 9 and PHP 7.4, this is one of our community members got:

There were 3 errors:

1) PluginTest::testHostFunctions
Object of class FFI\CData could not be converted to int

/app/lib/vendor/extism/extism/src/LibExtism.php:167
/app/lib/vendor/extism/extism/src/LibExtism.php:140
/app/lib/vendor/extism/extism/src/HostFunction.php:110
/app/lib/vendor/extism/extism/tests/PluginTest.php:84
phpvfscomposer:///app/lib/vendor/extism/extism/vendor/phpunit/phpunit/phpunit:106

2) PluginTest::testHostFunctionManual
Object of class FFI\CData could not be converted to int

/app/lib/vendor/extism/extism/src/LibExtism.php:167
/app/lib/vendor/extism/extism/src/LibExtism.php:140
/app/lib/vendor/extism/extism/src/HostFunction.php:110
/app/lib/vendor/extism/extism/tests/PluginTest.php:107
phpvfscomposer:///app/lib/vendor/extism/extism/vendor/phpunit/phpunit/phpunit:106

3) PluginTest::testHostFunctionNamespace
Object of class FFI\CData could not be converted to int

/app/lib/vendor/extism/extism/src/LibExtism.php:167
/app/lib/vendor/extism/extism/src/LibExtism.php:140
/app/lib/vendor/extism/extism/src/HostFunction.php:110
/app/lib/vendor/extism/extism/tests/PluginTest.php:129
phpvfscomposer:///app/lib/vendor/extism/extism/vendor/phpunit/phpunit/phpunit:106
oatmael commented 4 months ago

So I've poked at this a little bit, from what I can tell the following function is the culprit here: https://github.com/extism/php-sdk/blob/main/src/LibExtism.php#L157

function toCArray(array $array, string $type): ?FFI\CData
{
    if (count($array) == 0) {
        return $this->ffi->new($type . "*");
    }

    $cArray = $this->ffi->new($type . "[" . count($array) . "]");
    for ($i = 0; $i < count($array); $i++) {            
        $cArray[$i] = $array[$i];
    }

    return $cArray;
}

But more specifically line 165, it's not exactly well documented from what I can tell, but PHP 7.4's FFI doesn't allow for assigning CData to structs or fields (see https://www.php.net/manual/en/class.ffi-cdata.php#Changelog).

Where this applies to host functions is when converting the handle to CData when mapping the inputs / outputs here: https://github.com/extism/php-sdk/blob/main/src/LibExtism.php#L136

function extism_function_new(string $name, array $inputTypes, array $outputTypes, callable $callback, $userData, $freeUserData) : \FFI\CData
{
    $inputs = $this->toCArray($inputTypes, "ExtismValType");
    $outputs = $this->toCArray($outputTypes, "ExtismValType");

    $handle = $this->ffi->extism_function_new($name, $inputs, count($inputTypes), $outputs, count($outputTypes), $callback, $userData, $freeUserData);

    return $handle;
}

Since the problematic line in toCArray is line 165, host functions actually do work if you define them as having no inputs or outputs:

$a = 0;
$increment_a = new HostFunction('increment_a', [], [], 
  function () use (&$a) {
    $a++;
  }
);

...
echo $a;

I'll do some more digging into if there's an effective workaround for the CData assignment issue, but hopefully this gives better visibility on where the issue actually lies :smile:

oatmael commented 4 months ago

I believe I have a fix. There seems to be an issue with specifically the ExtismValType enum casting that happens in the HostFunction.php.

Changing the following block from this:

$inputs = [];

for ($i = 0; $i < count($inputTypes); $i++) {
    $inputs[$i] = $this->lib->ffi->cast("ExtismValType", $inputTypes[$i]);
}

$outputs = [];

for ($i = 0; $i < count($outputTypes); $i++) {
    $outputs[$i] = $this->lib->ffi->cast("ExtismValType", $outputTypes[$i]);
}

To this:

public const VAL_TYPE_MAP = [
    0 => 'I32',
    1 => 'I64',
    2 => 'F32',
    3 => 'F64',
    4 => 'V128',
    5 => 'FUNC_REF',
    6 => 'EXTERN_REF',
];
$inputs = [];

for ($i = 0; $i < count($inputTypes); $i++) {
    $enum = ExtismValType::VAL_TYPE_MAP[$inputTypes[$i]];
    $inputs[$i] = $this->lib->ffi->$enum;
}

$outputs = [];

for ($i = 0; $i < count($outputTypes); $i++) {
    $enum = ExtismValType::VAL_TYPE_MAP[$outputTypes[$i]];
    $outputs[$i] = $this->lib->ffi->$enum;
}

Allows the host functions to work as expected :)

mhmd-azeez commented 4 months ago

This is fixed in #23