[mono][wasm] Refactor mono_fixup_symbol_name (#101681)

Refactor mono_fixup_symbol_name, apply @lambdageek's feedback

Author: mkhamoyan

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;
 }

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);
 }

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

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) {

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);
 	}

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))