From 607ecff7baaab65d589446ba86a45bc03daa024d Mon Sep 17 00:00:00 2001 From: Zoltan Varga Date: Sat, 8 May 2021 16:15:20 -0400 Subject: [PATCH] [mono] Fix invoking string ctors on wasm in AOT mode. (#52505) * [mono] Avoid passing a dummy string to string ctors from runtime invokes, pass NULL instead. The call will go to a wrapper method which will ignore the 'this' argument anyway. * [mono] Fix invoking string ctors on wasm in AOT mode. --- src/mono/mono/metadata/marshal-ilgen.c | 28 +++------------------------- src/mono/mono/mini/aot-compiler.c | 14 ++++++++++++++ src/mono/mono/mini/mini-runtime.c | 15 +++++++++------ 3 files changed, 26 insertions(+), 31 deletions(-) diff --git a/src/mono/mono/metadata/marshal-ilgen.c b/src/mono/mono/metadata/marshal-ilgen.c index a253af0..151e05a 100644 --- a/src/mono/mono/metadata/marshal-ilgen.c +++ b/src/mono/mono/metadata/marshal-ilgen.c @@ -1507,30 +1507,11 @@ emit_invoke_call (MonoMethodBuilder *mb, MonoMethod *method, int loc_res, gboolean virtual_, gboolean need_direct_wrapper) { - static MonoString *string_dummy = NULL; int i; int *tmp_nullable_locals; gboolean void_ret = FALSE; gboolean string_ctor = method && method->string_ctor; - /* to make it work with our special string constructors */ - if (!string_dummy) { - ERROR_DECL (error); - - // FIXME Allow for static construction of MonoString. - - SETUP_ICALL_FUNCTION; - SETUP_ICALL_FRAME; - - MONO_GC_REGISTER_ROOT_SINGLE (string_dummy, MONO_ROOT_SOURCE_MARSHAL, NULL, "Marshal Dummy String"); - - MonoStringHandle string_dummy_handle = mono_string_new_utf8_len ("dummy", 5, error); - string_dummy = MONO_HANDLE_RAW (string_dummy_handle); - mono_error_assert_ok (error); - - CLEAR_ICALL_FRAME; - } - if (virtual_) { g_assert (sig->hasthis); g_assert (method->flags & METHOD_ATTRIBUTE_VIRTUAL); @@ -1538,12 +1519,9 @@ emit_invoke_call (MonoMethodBuilder *mb, MonoMethod *method, if (sig->hasthis) { if (string_ctor) { - if (mono_gc_is_moving ()) { - mono_mb_emit_ptr (mb, &string_dummy); - mono_mb_emit_byte (mb, CEE_LDIND_REF); - } else { - mono_mb_emit_ptr (mb, string_dummy); - } + /* This will call the code emitted by mono_marshal_get_native_wrapper () which ignores it */ + mono_mb_emit_icon (mb, 0); + mono_mb_emit_byte (mb, CEE_CONV_I); } else { mono_mb_emit_ldarg (mb, 0); } diff --git a/src/mono/mono/mini/aot-compiler.c b/src/mono/mono/mini/aot-compiler.c index 4cc99ae..6bb97b2 100644 --- a/src/mono/mono/mini/aot-compiler.c +++ b/src/mono/mono/mini/aot-compiler.c @@ -4731,6 +4731,20 @@ add_wrappers (MonoAotCompile *acfg) /* FIXME: locking - this is "safe" as full-AOT threads don't mutate the icall data */ for (int i = 0; i < MONO_JIT_ICALL_count; ++i) add_jit_icall_wrapper (acfg, mono_find_jit_icall_info ((MonoJitICallId)i)); + + if (acfg->aot_opts.llvm_only) { + /* String ctors are called directly on llvmonly */ + for (int i = 0; i < rows; ++i) { + ERROR_DECL (error); + + token = MONO_TOKEN_METHOD_DEF | (i + 1); + method = mono_get_method_checked (acfg->image, token, NULL, NULL, error); + if (method && method->string_ctor) { + MonoMethod *w = get_runtime_invoke (acfg, method, FALSE); + add_method (acfg, w); + } + } + } } /* diff --git a/src/mono/mono/mini/mini-runtime.c b/src/mono/mono/mini/mini-runtime.c index 6c2e6c4..e450f92 100644 --- a/src/mono/mono/mini/mini-runtime.c +++ b/src/mono/mono/mini/mini-runtime.c @@ -2921,10 +2921,7 @@ create_runtime_invoke_info (MonoMethod *method, gpointer compiled_method, gboole info = g_new0 (RuntimeInvokeInfo, 1); info->compiled_method = compiled_method; info->use_interp = use_interp; - if (mono_llvm_only && method->string_ctor) - info->sig = mono_marshal_get_string_ctor_signature (method); - else - info->sig = mono_method_signature_internal (method); + info->sig = mono_method_signature_internal (method); invoke = mono_marshal_get_runtime_invoke (method, FALSE); (void)invoke; @@ -3016,7 +3013,13 @@ create_runtime_invoke_info (MonoMethod *method, gpointer compiled_method, gboole } if (!info->dyn_call_info) { - if (mono_llvm_only) { + /* + * Can't use the normal llvmonly code for string ctors since the gsharedvt out wrapper passes + * an extra arg, which the string ctor methods don't have, which causes signature mismatches + * on wasm. Instead, call string ctors normally using a direct runtime invoke wrapper + * which is AOTed for each ctor. + */ + if (mono_llvm_only && !method->string_ctor) { #ifndef MONO_ARCH_GSHAREDVT_SUPPORTED g_assert_not_reached (); #endif @@ -3331,7 +3334,7 @@ mono_jit_runtime_invoke (MonoMethod *method, void *obj, void **params, MonoObjec if (info->use_interp) { result = mini_get_interp_callbacks ()->runtime_invoke (method, obj, params, exc, error); return_val_if_nok (error, NULL); - } else if (mono_llvm_only) { + } else if (mono_llvm_only && !method->string_ctor) { result = mono_llvmonly_runtime_invoke (method, info, obj, params, exc, error); if (!is_ok (error)) return NULL; -- 2.7.4