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

Fix import error when using wasm-c-api with WASI threads #2170

Conversation

eloparco
Copy link
Contributor

@eloparco eloparco commented May 3, 2023

Similar to what already in place for pthreads:

/* workaround about passing instantiate-linking information */
{
CApiFuncImport *c_api_func_imports;
uint32 import_func_count = 0;
uint32 size_in_bytes = 0;
#if WASM_ENABLE_INTERP != 0
if (module_inst->module_type == Wasm_Module_Bytecode) {
new_c_api_func_imports = &(
((WASMModuleInstance *)new_module_inst)->e->c_api_func_imports);
c_api_func_imports =
((WASMModuleInstance *)module_inst)->e->c_api_func_imports;
import_func_count = ((WASMModule *)module)->import_function_count;
}
#endif
#if WASM_ENABLE_AOT != 0
if (module_inst->module_type == Wasm_Module_AoT) {
AOTModuleInstanceExtra *e =
(AOTModuleInstanceExtra *)((AOTModuleInstance *)new_module_inst)
->e;
new_c_api_func_imports = &(e->c_api_func_imports);
e = (AOTModuleInstanceExtra *)((AOTModuleInstance *)module_inst)->e;
c_api_func_imports = e->c_api_func_imports;
import_func_count = ((AOTModule *)module)->import_func_count;
}
#endif
if (import_func_count != 0 && c_api_func_imports) {
size_in_bytes = sizeof(CApiFuncImport *) * import_func_count;
*new_c_api_func_imports = wasm_runtime_malloc(size_in_bytes);
if (!(*new_c_api_func_imports))
goto fail;
bh_memcpy_s(*new_c_api_func_imports, size_in_bytes,
c_api_func_imports, size_in_bytes);
}
}

#2149

TODO:

  • refactor common code
  • check missing free for new_c_api_func_imports in lib_pthread_wrapper.c

#endif

if (import_func_count != 0 && c_api_func_imports) {
size_in_bytes = sizeof(CApiFuncImport *) * import_func_count;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This star is wrong, it needs the size of the struct, not the size of one pointer. Corrected in main...cpetig:wasm-micro-runtime:main

Also there is more issues with threads which I will describe in a separate issue.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Note: I was the one introducing this error first)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -136,6 +177,7 @@ thread_spawn_wrapper(wasm_exec_env_t exec_env, uint32 start_arg)
deallocate_thread_id(thread_id);

thread_preparation_fail:
wasm_runtime_free(*new_c_api_func_imports);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly this is already implied by the next two lines - as deinstantiate frees c_api_func_imports in line 2265.

@eloparco
Copy link
Contributor Author

eloparco commented May 4, 2023

Closing since the same changes are being applied in #2173

@eloparco eloparco closed this May 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants