Skip to content

Function.InvokeCallback() should handle ITuple return value only when Function.IsTuple() was true #158

@kpreisser

Description

@kpreisser

Hi, I noticed the an issue in Function.InvokeCallback() using Wasmtime 1.0.0. Consider the following C# program (.NET 6.0):

using Wasmtime;

using var engine = new Engine();

using var module = Module.FromText(
    engine,
    "hello",
    @"
(module 
    (func $getExternref (import """" ""getExternref"") (result externref)) 
    (func (export ""run"")
        call $getExternref
        drop
    )
)");

using var linker = new Linker(engine);
using var store = new Store(engine);

linker.Define(
    "",
    "getExternref",
    Function.FromCallback(store, () => (object)(1, 2, 3)));

var instance = linker.Instantiate(store, module);

var run = instance.GetAction("run")!;
run();

Expected behavior: Program runs successfully (the boxed ValueTuple<int, int, int> should be returned as externref to WASM, as the Delegate returns an object and the return type's value kind is ValueKind.ExternRef).

Actual behavior: An ArgumentOutOfRangeException occurs in Function.InvokeCallback():

System.ArgumentOutOfRangeException
  HResult=0x80131502
  Message=Index was out of range. Must be non-negative and less than the size of the collection.
  Source=System.Private.CoreLib
  StackTrace:
   at System.ThrowHelper.ThrowArgumentOutOfRange_IndexMustBeLessException()
   at System.Collections.Generic.List`1.get_Item(Int32 index)
   at Wasmtime.Function.InvokeCallback(Delegate callback, Caller caller, Boolean passCaller, Value* args, Int32 nargs, Value* results, Int32 nresults, IReadOnlyList`1 resultKinds) in C:\Users\VPC\Desktop\WebAssembly-Test\wasmtime-dotnet\src\Function.cs:line 2436

This is because Function.InvokeCallback() unconditionally checks whether the returned value is a ITuple, regardless of whether Function.IsTuple() previously returned true when determining the function type:

// NOTE: reflection is extremely slow for invoking methods. in the future, perhaps this could be replaced with
// source generators, system.linq.expressions, or generate IL with DynamicMethods or something
var result = callback.Method.Invoke(callback.Target, BindingFlags.DoNotWrapExceptions, null, invokeArgs, null);
if (resultKinds.Count > 0)
{
var tuple = result as ITuple;
if (tuple is null)
{
results[0] = Value.FromObject(result, resultKinds[0]);
}
else
{
for (int i = 0; i < tuple.Length; ++i)
{
results[i] = Value.FromObject(tuple[i], resultKinds[i]);
}
}
}

I think it should probably cast the value as ITuple only when Function.IsTuple() returned true previously, so that in the above case, the ValueTuple<int, int, int> is returned as externref.

Additionally, I was thinking to try to implement an approch using ILGenerator and DynamicMethod to generate IL code that can directly call the method without reflection, as it is also noted in the comment that calling the callback using reflection is very slow. Would you accept such a change as PR?

Thank you!

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions