Fix issues in Function.FromCallback() and Linker.DefineFunction(): (b…

…ytecodealliance#161)

- Instead of invoking callback.Method (that may have a different parameter count than the callback), invoke the callback's Invoke() method.
- Handle a ITuple return value only when GetFunctionType determined that the function returns a tuple, to correctly return a boxed ValueTuple as externref.
- Support nested levels of ValueTuples, so that it's possible e.g. to return a ValueTulple<..., ValueTuple<..., ValueTuple<...>>> (15 values).

Fixes bytecodealliance#158
Fixes bytecodealliance#159

src/Function.cs

@@ -2239,7 +2239,8 @@ internal Function(IStore store, Delegate callback, bool hasReturn)
             }
             this.store = store;
 
-            using var funcType = GetFunctionType(callback.GetType(), hasReturn, this.parameters, this.results, out var hasCaller);
+            using var funcType = GetFunctionType(callback.GetType(), hasReturn, this.parameters, this.results, out var hasCaller, out var returnsTuple);
+            var callbackInvokeMethod = callback.GetType().GetMethod(nameof(Action.Invoke))!;
 
             unsafe
             {
@@ -2249,15 +2250,15 @@ internal Function(IStore store, Delegate callback, bool hasReturn)
                     func = (env, callerPtr, args, nargs, results, nresults) =>
                     {
                         using var caller = new Caller(callerPtr);
-                        return InvokeCallback(callback, caller, true, args, (int)nargs, results, (int)nresults, Results);
+                        return InvokeCallback(callback, callbackInvokeMethod, caller, true, args, (int)nargs, results, (int)nresults, Results, returnsTuple);
                     };
                 }
                 else
                 {
                     func = (env, callerPtr, args, nargs, results, nresults) =>
                     {
                         using var caller = new Caller(callerPtr);
-                        return InvokeCallback(callback, caller, false, args, (int)nargs, results, (int)nresults, Results);
+                        return InvokeCallback(callback, callbackInvokeMethod, caller, false, args, (int)nargs, results, (int)nresults, Results, returnsTuple);
                     };
                 }
 
@@ -2301,32 +2302,42 @@ internal Function(IStore store, ExternFunc func)
             }
         }
 
-        private static IEnumerable<Type> EnumerateReturnTypes(Type? returnType)
+        private static IEnumerable<Type> EnumerateReturnTypes(Type? returnType, out bool isTuple)
         {
+            isTuple = false;
+
             if (returnType is null)
             {
-                yield break;
+                return Array.Empty<Type>();
             }
 
             if (IsTuple(returnType))
             {
-                foreach (var type in returnType
-                    .GetGenericArguments()
-                    .SelectMany(type =>
+                isTuple = true;
+                return EnumerateTupleTypes(returnType);
+
+                static IEnumerable<Type> EnumerateTupleTypes(Type tupleType)
+                {
+                    foreach (var (typeArgument, idx) in tupleType.GenericTypeArguments.Select((e, idx) => (e, idx)))
+                    {
+                        if (idx is 7 && IsTuple(typeArgument))
                         {
-                            if (type.IsConstructedGenericType)
+                            // Recursively enumerate the nested tuple's type arguments.
+                            foreach (var type in EnumerateTupleTypes(typeArgument))
                             {
-                                return type.GenericTypeArguments;
+                                yield return type;
                             }
-                            return Enumerable.Repeat(type, 1);
-                        }))
-                {
-                    yield return type;
+                        }
+                        else
+                        {
+                            yield return typeArgument;
+                        }
+                    }
                 }
             }
             else
             {
-                yield return returnType;
+                return new Type[] { returnType };
             }
         }
 
@@ -2350,7 +2361,7 @@ private static bool IsTuple(Type type)
                    definition == typeof(ValueTuple<,,,,,,,>);
         }
 
-        internal static TypeHandle GetFunctionType(Type type, bool hasReturn, List<ValueKind> parameters, List<ValueKind> results, out bool hasCaller)
+        internal static TypeHandle GetFunctionType(Type type, bool hasReturn, List<ValueKind> parameters, List<ValueKind> results, out bool hasCaller, out bool returnsTuple)
         {
             Span<Type> parameterTypes = null;
             Type? returnType = null;
@@ -2388,7 +2399,7 @@ internal static TypeHandle GetFunctionType(Type type, bool hasReturn, List<Value
                 parameters.Add(kind);
             }
 
-            results.AddRange(EnumerateReturnTypes(returnType).Select(t =>
+            results.AddRange(EnumerateReturnTypes(returnType, out returnsTuple).Select(t =>
             {
                 if (!Value.TryGetKind(t, out var kind))
                 {
@@ -2400,7 +2411,7 @@ internal static TypeHandle GetFunctionType(Type type, bool hasReturn, List<Value
             return new Function.TypeHandle(Function.Native.wasm_functype_new(new ValueTypeArray(parameters), new ValueTypeArray(results)));
         }
 
-        internal unsafe static IntPtr InvokeCallback(Delegate callback, Caller caller, bool passCaller, Value* args, int nargs, Value* results, int nresults, IReadOnlyList<ValueKind> resultKinds)
+        internal unsafe static IntPtr InvokeCallback(Delegate callback, MethodInfo callbackInvokeMethod, Caller caller, bool passCaller, Value* args, int nargs, Value* results, int nresults, IReadOnlyList<ValueKind> resultKinds, bool returnsTuple)
         {
             try
             {
@@ -2420,23 +2431,21 @@ internal unsafe static IntPtr InvokeCallback(Delegate callback, Caller caller, b
 
                 // 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);
+                var result = callbackInvokeMethod.Invoke(callback, BindingFlags.DoNotWrapExceptions, null, invokeArgs, null);
 
-                if (resultKinds.Count > 0)
+                if (returnsTuple)
                 {
-                    var tuple = result as ITuple;
-                    if (tuple is null)
-                    {
-                        results[0] = Value.FromObject(result, resultKinds[0]);
-                    }
-                    else
+                    var tuple = (ITuple)result!;
+
+                    for (int i = 0; i < tuple.Length; ++i)
                     {
-                        for (int i = 0; i < tuple.Length; ++i)
-                        {
-                            results[i] = Value.FromObject(tuple[i], resultKinds[i]);
-                        }
+                        results[i] = Value.FromObject(tuple[i], resultKinds[i]);
                     }
                 }
+                else if (resultKinds.Count == 1)
+                {
+                    results[0] = Value.FromObject(result, resultKinds[0]);
+                }
                 return IntPtr.Zero;
             }
             catch (Exception ex)

src/Linker.cs

@@ -756,14 +756,15 @@ private void DefineFunction(string module, string name, Delegate callback, bool
             var parameterKinds = new List<ValueKind>();
             var resultKinds = new List<ValueKind>();
 
-            using var funcType = Function.GetFunctionType(callback.GetType(), hasReturn, parameterKinds, resultKinds, out var hasCaller);
+            using var funcType = Function.GetFunctionType(callback.GetType(), hasReturn, parameterKinds, resultKinds, out var hasCaller, out var returnsTuple);
+            var callbackInvokeMethod = callback.GetType().GetMethod(nameof(Action.Invoke))!;
 
             unsafe
             {
                 Function.Native.WasmtimeFuncCallback func = (env, callerPtr, args, nargs, results, nresults) =>
                 {
                     using var caller = new Caller(callerPtr);
-                    return Function.InvokeCallback(callback, caller, hasCaller, args, (int)nargs, results, (int)nresults, resultKinds);
+                    return Function.InvokeCallback(callback, callbackInvokeMethod, caller, hasCaller, args, (int)nargs, results, (int)nresults, resultKinds, returnsTuple);
                 };
 
                 var moduleBytes = Encoding.UTF8.GetBytes(module);

tests/ExternRefTests.cs

@@ -53,6 +53,19 @@ public void ItHandlesNullReferences()
             (nullref.Invoke()).Should().BeNull();
         }
 
+        [Fact]
+        public void ItReturnsBoxedValueTupleAsExternRef()
+        {
+            // Test for issue #158
+            var instance = Linker.Instantiate(Store, Fixture.Module);
+
+            var inout = instance.GetFunction<object, object>("inout");
+            inout.Should().NotBeNull();
+
+            var input = (object)(1, 2, 3);
+            inout(input).Should().BeSameAs(input);
+        }
+
         unsafe class Value
         {
             internal Value(int* counter)

tests/FunctionTests.cs

@@ -31,6 +31,36 @@ public FunctionTests(FunctionsFixture fixture)
             {
                 caller.GetMemory("mem").ReadString(address, length).Should().Be("Hello World");
             }));
+
+            Linker.Define("env", "return_i32", Function.FromCallback(Store, GetBoundFuncIntDelegate()));
+
+            Linker.Define("env", "return_15_values", Function.FromCallback(Store, () =>
+            {
+                return (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15);
+            }));
+
+            Linker.Define("env", "accept_15_values", Function.FromCallback(Store,
+                (int i1, int i2, int i3, int i4, int i5, int i6, int i7, int i8, int i9, int i10, int i11, int i12, int i13, int i14, int i15) =>
+                {
+                    (i1, i2, i3, i4, i5, i6, i7, i8, i9, i10, i11, i12, i13, i14, i15)
+                        .Should().Be((1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15));
+                }));
+
+            Func<int> GetBoundFuncIntDelegate()
+            {
+                // Get a delegate that is bound over an argument.
+                // See #159
+                var getLengthDelegate = GetLength;
+                var getLengthMethod = getLengthDelegate.Method;
+
+                string str = "abc";
+                return (Func<int>)Delegate.CreateDelegate(typeof(Func<int>), str, getLengthMethod);
+
+                int GetLength(string s)
+                {
+                    return s.Length;
+                }
+            }
         }
 
         private FunctionsFixture Fixture { get; }
@@ -316,6 +346,31 @@ public void ItReturnsNullForVeryLongTuples()
                 .BeNull();
         }
 
+        [Fact]
+        public void ItReturnsInt32WithBoundDelegate()
+        {
+            // Test for issue #159: It should be possible to defined a Delegate that is bound with
+            // a first argument.
+            var instance = Linker.Instantiate(Store, Fixture.Module);
+            var echo = instance.GetFunction<int>("return_i32");
+            echo.Should().NotBeNull();
+
+            var result = echo.Invoke();
+            result.Should().Be(3);
+        }
+
+        [Fact]
+        public void ItReturnsAndAccepts15Values()
+        {
+            // Verify that nested levels of ValueTuple are handled correctly. Returning 15
+            // values means that a ValueTuple<..., ValueTuple<..., ValueTuple<...>>> is used.
+            var instance = Linker.Instantiate(Store, Fixture.Module);
+            var action = instance.GetAction("get_and_pass_15_values");
+            action.Should().NotBeNull();
+
+            action.Invoke();
+        }
+
         public void Dispose()
         {
             Store.Dispose();

tests/LinkerFunctionsTests.cs

@@ -30,6 +30,36 @@ public LinkerFunctionTests(LinkerFunctionsFixture fixture)
             {
                 caller.GetMemory("mem").ReadString(address, length).Should().Be("Hello World");
             });
+
+            Linker.Define("env", "return_i32", Function.FromCallback(Store, GetBoundFuncIntDelegate()));
+
+            Linker.Define("env", "return_15_values", Function.FromCallback(Store, () =>
+            {
+                return (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15);
+            }));
+
+            Linker.Define("env", "accept_15_values", Function.FromCallback(Store,
+                (int i1, int i2, int i3, int i4, int i5, int i6, int i7, int i8, int i9, int i10, int i11, int i12, int i13, int i14, int i15) =>
+                {
+                    (i1, i2, i3, i4, i5, i6, i7, i8, i9, i10, i11, i12, i13, i14, i15)
+                        .Should().Be((1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15));
+                }));
+
+            Func<int> GetBoundFuncIntDelegate()
+            {
+                // Get a delegate that is bound over an argument.
+                // See #159
+                var getLengthDelegate = GetLength;
+                var getLengthMethod = getLengthDelegate.Method;
+
+                string str = "abc";
+                return (Func<int>)Delegate.CreateDelegate(typeof(Func<int>), str, getLengthMethod);
+
+                int GetLength(string s)
+                {
+                    return s.Length;
+                }
+            }
         }
 
         private LinkerFunctionsFixture Fixture { get; }
@@ -40,12 +70,15 @@ public void ItBindsImportMethodsAndCallsThemCorrectly()
             var instance = Linker.Instantiate(Store, Fixture.Module);
             var add = instance.GetFunction("add");
             var swap = instance.GetFunction("swap");
-            var check = instance.GetFunction("check_string");
+            var check = instance.GetFunction("check_string"); ;
+            var getInt32 = instance.GetFunction<int>("return_i32");
 
             int x = (int)add.Invoke(40, 2);
             x.Should().Be(42);
             x = (int)add.Invoke(22, 5);
             x.Should().Be(27);
+            x = getInt32.Invoke();
+            x.Should().Be(3);
 
             object[] results = (object[])swap.Invoke(10, 100);
             results.Should().Equal(new object[] { 100, 10 });

tests/Modules/Functions.wat

@@ -3,11 +3,16 @@
  (import "env" "swap" (func $env.swap (param i32 i32) (result i32 i32)))
  (import "env" "do_throw" (func $env.do_throw))
  (import "env" "check_string" (func $env.check_string (param i32 i32)))
+ (import "env" "return_i32" (func $env.return_i32 (result i32)))
+ (import "env" "return_15_values" (func $env.return_15_values (result i32 i32 i32 i32 i32 i32 i32 i32 i32 i32 i32 i32 i32 i32 i32)))
+ (import "env" "accept_15_values" (func $env.accept_15_values (param i32 i32 i32 i32 i32 i32 i32 i32 i32 i32 i32 i32 i32 i32 i32)))
  (memory (export "mem") 1)
  (export "add" (func $add))
  (export "swap" (func $swap))
  (export "do_throw" (func $do_throw))
  (export "check_string" (func $check_string))
+ (export "return_i32" (func $env.return_i32))
+ (export "get_and_pass_15_values" (func $get_and_pass_15_values))
  (func $add (param i32 i32) (result i32)
   (call $env.add (local.get 0) (local.get 1))
  )
@@ -20,6 +25,10 @@
  (func $check_string
   (call $env.check_string (i32.const 0) (i32.const 11))
  )
+ (func $get_and_pass_15_values
+	call $env.return_15_values
+	call $env.accept_15_values
+ )
  (data (i32.const 0) "Hello World")
 
  (func (export "noop"))