From ce2364c7c3ac0c238a4eea38b36ecc50d67c9964 Mon Sep 17 00:00:00 2001 From: Meri Khamoyan <96171496+mkhamoyan@users.noreply.github.com> Date: Thu, 9 May 2024 13:30:12 +0200 Subject: [PATCH] [mono][wasm] Refactor mono_fixup_symbol_name (#101681) Refactor mono_fixup_symbol_name, apply @lambdageek's feedback --- src/mono/browser/runtime/runtime.c | 5 +- src/mono/mono/metadata/native-library.c | 71 ++++++++++++++------ src/mono/mono/metadata/native-library.h | 2 +- src/mono/mono/mini/aot-compiler.c | 28 ++------ src/mono/mono/mini/mini-llvm.c | 18 +---- src/tasks/AotCompilerTask/MonoAOTCompiler.cs | 2 +- 6 files changed, 64 insertions(+), 62 deletions(-) diff --git a/src/mono/browser/runtime/runtime.c b/src/mono/browser/runtime/runtime.c index 8ddcdbce13863..436c3a2958632 100644 --- a/src/mono/browser/runtime/runtime.c +++ b/src/mono/browser/runtime/runtime.c @@ -65,7 +65,7 @@ int mono_wasm_enable_gc = 1; /* Missing from public headers */ -char *mono_fixup_symbol_name (char *key); +char *mono_fixup_symbol_name (const char *prefix, const char *key, const char *suffix); void mono_icall_table_init (void); void mono_wasm_enable_debugging (int); void mono_ee_interp_init (const char *opts); @@ -214,8 +214,9 @@ get_native_to_interp (MonoMethod *method, void *extra_arg) assert (strlen (name) < 100); snprintf (key, sizeof(key), "%s_%s_%s", name, class_name, method_name); - char* fixedName = mono_fixup_symbol_name(key); + char *fixedName = mono_fixup_symbol_name ("", key, ""); addr = wasm_dl_get_native_to_interp (fixedName, extra_arg); + free (fixedName); MONO_EXIT_GC_UNSAFE; return addr; } diff --git a/src/mono/mono/metadata/native-library.c b/src/mono/mono/metadata/native-library.c index 142c467fdd0af..6adb70bfda4d2 100644 --- a/src/mono/mono/metadata/native-library.c +++ b/src/mono/mono/metadata/native-library.c @@ -1223,31 +1223,62 @@ mono_loader_install_pinvoke_override (PInvokeOverrideFn override_fn) pinvoke_override = override_fn; } -// Keep synced with FixupSymbolName from src/tasks/Common/Utils.cs -char* mono_fixup_symbol_name (char *key) { - char* fixedName = malloc(256); - int sb_index = 0; - int len = (int)strlen (key); +static gboolean +is_symbol_char_verbatim (unsigned char b) +{ + return ((b >= '0' && b <= '9') || (b >= 'a' && b <= 'z') || (b >= 'A' && b <= 'Z')); +} - for (int i = 0; i < len; ++i) { +static gboolean +is_symbol_char_underscore (unsigned char c) +{ + switch (c) { + case '_': + case '.': + case '-': + case '+': + case '<': + case '>': + return TRUE; + default: + return FALSE; + } +} + +static size_t mono_precompute_size (const char *key) +{ + size_t size = 1; // Null terminator + size_t len = (int)strlen (key); + for (size_t i = 0; i < len; ++i) { unsigned char b = key[i]; - if ((b >= '0' && b <= '9') || - (b >= 'a' && b <= 'z') || - (b >= 'A' && b <= 'Z') || - (b == '_')) { - fixedName[sb_index++] = b; - } - else if (b == '.' || b == '-' || b == '+' || b == '<' || b == '>') { - fixedName[sb_index++] = '_'; + if (is_symbol_char_verbatim (b) || is_symbol_char_underscore (b)) { + size++; } else { - // Append the hexadecimal representation of b between underscores - sprintf(&fixedName[sb_index], "_%X_", b); - sb_index += 4; // Move the index after the appended hexadecimal characters + size += 4; } } + return size; +} - // Null-terminate the fixedName string - fixedName[sb_index] = '\0'; - return fixedName; +// Keep synced with FixupSymbolName from src/tasks/Common/Utils.cs +char* mono_fixup_symbol_name (const char *prefix, const char *key, const char *suffix) { + size_t size = mono_precompute_size (key) + strlen (prefix) + strlen (suffix); + GString *str = g_string_sized_new (size); + size_t len = (int)strlen (key); + g_string_append_printf (str, "%s", prefix); + + for (size_t i = 0; i < len; ++i) { + unsigned char b = key[i]; + if (is_symbol_char_verbatim (b)) { + g_string_append_c (str, b); + } else if (is_symbol_char_underscore (b)) { + g_string_append_c (str, '_'); + } else { + // Append the hex representation of b between underscores + g_string_append_printf (str, "_%X_", b); + } + } + g_string_append_printf (str, "%s", suffix); + return g_string_free (str, FALSE); } diff --git a/src/mono/mono/metadata/native-library.h b/src/mono/mono/metadata/native-library.h index 8a0b0e20b9ec6..9a794a5bb8af7 100644 --- a/src/mono/mono/metadata/native-library.h +++ b/src/mono/mono/metadata/native-library.h @@ -36,6 +36,6 @@ void mono_loader_install_pinvoke_override (PInvokeOverrideFn override_fn); char * -mono_fixup_symbol_name (char *key); +mono_fixup_symbol_name (const char *prefix, const char *key, const char *suffix); #endif diff --git a/src/mono/mono/mini/aot-compiler.c b/src/mono/mono/mini/aot-compiler.c index cf71da8e5329c..cb525d12e20d7 100644 --- a/src/mono/mono/mini/aot-compiler.c +++ b/src/mono/mono/mini/aot-compiler.c @@ -12414,18 +12414,8 @@ emit_file_info (MonoAotCompile *acfg) * mono_aot_register_module (). The symbol points to a pointer to the file info * structure. */ - sprintf (symbol, "%smono_aot_module_%s_info", acfg->user_symbol_prefix, acfg->image->assembly->aname.name); -#ifdef TARGET_WASM - acfg->static_linking_symbol = g_strdup (mono_fixup_symbol_name(symbol)); -#else - /* Get rid of characters which cannot occur in symbols */ - char *p = symbol; - for (p = symbol; *p; ++p) { - if (!(isalnum (*p) || *p == '_')) - *p = '_'; - } - acfg->static_linking_symbol = g_strdup (symbol); -#endif + snprintf (symbol, MAX_SYMBOL_SIZE, "%smono_aot_module_%s", acfg->user_symbol_prefix, acfg->image->assembly->aname.name); + acfg->static_linking_symbol = mono_fixup_symbol_name ("", symbol, "_info"); } if (acfg->llvm) @@ -14294,6 +14284,8 @@ acfg_free (MonoAotCompile *acfg) g_free (acfg->static_linking_symbol); g_free (acfg->got_symbol); g_free (acfg->plt_symbol); + g_free (acfg->global_prefix); + g_free (acfg->assembly_name_sym); g_ptr_array_free (acfg->methods, TRUE); g_ptr_array_free (acfg->image_table, TRUE); g_ptr_array_free (acfg->globals, TRUE); @@ -15121,18 +15113,8 @@ aot_assembly (MonoAssembly *ass, guint32 jit_opts, MonoAotOptions *aot_options) if (acfg->aot_opts.llvm_only) acfg->flags = (MonoAotFileFlags)(acfg->flags | MONO_AOT_FILE_FLAG_LLVM_ONLY); - acfg->assembly_name_sym = g_strdup (get_assembly_prefix (acfg->image)); -#ifdef TARGET_WASM - acfg->global_prefix = g_strdup_printf ("mono_aot_%s", g_strdup(mono_fixup_symbol_name (acfg->assembly_name_sym))); -#else - char *p; - /* Get rid of characters which cannot occur in symbols */ - for (p = acfg->assembly_name_sym; *p; ++p) { - if (!(isalnum (*p) || *p == '_')) - *p = '_'; - } + acfg->assembly_name_sym = mono_fixup_symbol_name ("", get_assembly_prefix (acfg->image), ""); acfg->global_prefix = g_strdup_printf ("mono_aot_%s", acfg->assembly_name_sym); -#endif acfg->plt_symbol = g_strdup_printf ("%s_plt", acfg->global_prefix); acfg->got_symbol = g_strdup_printf ("%s_got", acfg->global_prefix); if (acfg->llvm) { diff --git a/src/mono/mono/mini/mini-llvm.c b/src/mono/mono/mini/mini-llvm.c index 0c0f4e0cb1f2e..650a5a775e670 100644 --- a/src/mono/mono/mini/mini-llvm.c +++ b/src/mono/mono/mini/mini-llvm.c @@ -14542,22 +14542,10 @@ emit_aot_file_info (MonoLLVMModule *module) LLVMSetInitializer (info_var, LLVMConstNamedStruct (module->info_var_type, fields, nfields)); if (module->static_link) { - char *s; LLVMValueRef var; - - s = g_strdup_printf ("mono_aot_module_%s_info", module->assembly->aname.name); -#ifdef TARGET_WASM - var = LLVMAddGlobal (module->lmodule, pointer_type (LLVMInt8Type ()), g_strdup (mono_fixup_symbol_name(s))); -#else - /* Get rid of characters which cannot occur in symbols */ - char *p = s; - for (p = s; *p; ++p) { - if (!(isalnum (*p) || *p == '_')) - *p = '_'; - } - var = LLVMAddGlobal (module->lmodule, pointer_type (LLVMInt8Type ()), s); -#endif - g_free (s); + char *fixedName = mono_fixup_symbol_name ("mono_aot_module_", module->assembly->aname.name, "_info"); + var = LLVMAddGlobal (module->lmodule, pointer_type (LLVMInt8Type ()), fixedName); + free (fixedName); LLVMSetInitializer (var, LLVMConstBitCast (LLVMGetNamedGlobal (module->lmodule, "mono_aot_file_info"), pointer_type (LLVMInt8Type ()))); LLVMSetLinkage (var, LLVMExternalLinkage); } diff --git a/src/tasks/AotCompilerTask/MonoAOTCompiler.cs b/src/tasks/AotCompilerTask/MonoAOTCompiler.cs index 4e98e9271490b..2dc46b0c65a73 100644 --- a/src/tasks/AotCompilerTask/MonoAOTCompiler.cs +++ b/src/tasks/AotCompilerTask/MonoAOTCompiler.cs @@ -752,7 +752,7 @@ private PrecompileArguments GetPrecompileArgumentsFor(ITaskItem assemblyItem, st if (CollectTrimmingEligibleMethods) { - string assemblyName = assemblyFilename.Replace(".", "_"); + string assemblyName = FixupSymbolName(assemblyFilename); string outputFileName = assemblyName + "_compiled_methods.txt"; string outputFilePath; if (string.IsNullOrEmpty(TrimmingEligibleMethodsOutputDirectory))