Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error using double or float values with WebAssembly #67970

Closed
mark-cb opened this issue Apr 13, 2022 · 6 comments · Fixed by #69620
Closed

Error using double or float values with WebAssembly #67970

mark-cb opened this issue Apr 13, 2022 · 6 comments · Fixed by #69620
Assignees
Labels
arch-wasm WebAssembly architecture area-Codegen-AOT-mono
Milestone

Comments

@mark-cb
Copy link

mark-cb commented Apr 13, 2022

Description

Seeing a bug when using SQLitePCL.Raw preview and also see the same bug in Steve Sandersons Blaze Orbital sample when using double/float in entity classes. Works fine with decimals! Tried with EF Core and SQLite-net.

Reproduction Steps

https://github.com/mark-cb/BlazorSQLiteWasm

Expected behavior

Double and floats should work the same as decimals.

Actual behavior

Error:
/__w/1/s/src/mono/mono/mini/aot-runtime-wasm.c:113 /__w/1/s/src/mono/mono/mini/aot-runtime-wasm.c:113
Uncaught ExitStatus

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

ericsink/SQLitePCL.raw#480

@tannergooding
Copy link
Member

This may be the same error I'm seeing on #67939

@tannergooding
Copy link
Member

Not quite the same actually, I'm seeing Error: [MONO] * Assertion at /__w/1/s/src/mono/mono/mini/aot-runtime.c:5976, condition ' not met` but its also float/double related

@radical radical added the arch-wasm WebAssembly architecture label Apr 18, 2022
@ghost
Copy link

ghost commented Apr 18, 2022

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

Seeing a bug when using SQLitePCL.Raw preview and also see the same bug in Steve Sandersons Blaze Orbital sample when using double/float in entity classes. Works fine with decimals! Tried with EF Core and SQLite-net.

Reproduction Steps

https://github.com/mark-cb/BlazorSQLiteWasm

Expected behavior

Double and floats should work the same as decimals.

Actual behavior

Error:
/__w/1/s/src/mono/mono/mini/aot-runtime-wasm.c:113 /__w/1/s/src/mono/mono/mini/aot-runtime-wasm.c:113
Uncaught ExitStatus

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

ericsink/SQLitePCL.raw#480

Author: mark-cb
Assignees: -
Labels:

arch-wasm, untriaged, area-VM-meta-mono

Milestone: -

@lewing lewing added this to the 7.0.0 milestone Apr 29, 2022
@lewing lewing removed the untriaged New issue has not been triaged by the area owner label Apr 29, 2022
@kg
Copy link
Member

kg commented Apr 29, 2022

Is it possible to get a full stack trace here? Usually the JS exceptions you're reporting will include a stack below the message + line number

@lewing lewing assigned maraf and unassigned vargaz Apr 30, 2022
@lewing
Copy link
Member

lewing commented Apr 30, 2022

 void *p = bsearch (cookie, interp_to_native_signatures, G_N_ELEMENTS (interp_to_native_signatures), sizeof (gpointer), compare_icall_tramp);
	if (!p)
		g_error ("CANNOT HANDLE INTERP ICALL SIG %s\n", cookie);

@lambdageek
Copy link
Member

lambdageek commented May 9, 2022

We need to generate wasm_m2n_invoke using an msbuild task based on teh actual dll imports, instead of hardcoding a list in the runtime.

so basically we need to change aot-runtime-wasm.c to make three static variables in aot-runtime-wasm.c:

static void** interp_to_native_invokes;
static const char **interp_to_native_signatures;
static unsigned int interp_to_native_count;

and expose a public API that assigns to those variables:

MONO_API void
mono_wasm_install_interp_to_native_invokes (void **invokes, const char **sigs, unsingned int count);

so then instead of including wasm_m2n_invoke.g.h, aot-runtime-wasm can just assume the wasm runtime called mono_wasm_install_interp_to_native_invokes already and populated those static variables, and it can use them pretty much the same way as it does now. (except using the count variable instead of G_N_ELEMENTS (interp_to_native_signatures)).

then in src/wasm/runtime or src/wasm/build/ or some place we can create a .c and .o file for wasm_m2n_invoke using an MSBuild task.

One complication is that InterpMethodArguments is a private struct.
So we should also add a mono_wasm_interp_method_args_get_iargs, get_fargs, get_retval etc.
to the mono wasm headers (src/mono/wasm/runtime/pinvoke.h is a good candidate):

MONO_API gpointer*
mono_wasm_interp_method_args_get_iargs (InterpMethodArguments *margs); // returns margs->iargs

MONO_API 
gpointer *
mono_wasm_interp_method_args_get_retval  (InterpMethodArguments *margs); // returns margs->retval

and the generated functions like

static void
wasm_invoke_iifi (void *target_func, InterpMethodArguments *margs);

will use those methods to get at the elements of the InterpMethodArguments struct

static void
wasm_invoke_iifi (void *target_func, InterpMethodArguments *margs)
{
	typedef int (*T)(int arg_0, float arg_1, int arg_2);
	T func = (T)target_func;
        gpointer* iargs = mono_wasm_interp_method_args_get_iargs (margs);
        double *fargs = mono_wasm_interp_method_args_get_fargs (margs);
	int res = func ((int)(gssize)iargs [0], *(float*)&fargs [0], (int)(gssize)iargs [1]);
        gpointer *retval = mono_wasm_interp_method_args_get_retval (margs);
	*(int*)retval = res;
}

One complication is that the implementation of mono_wasm_interp_method_args_get_iargs needs to live in src/mono/mono/mini/aot-runtime-wasm.c so we also need to add the API declaration to src/mono/mono/mini/interp/interp.h.

In the future we should figure out some way for src/mono/wasm/runtime and src/mono/mono/mini to share some headers.


(there is also some utility junk that should probably just be duplicated. Things like the interp_pair struct and get_long_arg helper funciton that are used for long arguments,)

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label May 20, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label May 25, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jun 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-Codegen-AOT-mono
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants