[mono] Fix g_str_hash skipping the first byte and remove mono_metadata_str_hash (#119208)

Port of mono/mono#21844

`g_str_hash` was skipping the first byte because the `*p++` increment happens before the hash calculation, so when `*p` is referenced inside the loop, it's actually looking at the character after the one that was just tested.

We can also remove `mono_metadata_str_hash` which has similar logic (but isn't technically affected since it was already accounting for the first byte in  initializating `guint hash = *p`) since we no longer support linking to a different glib - we always link to eglib - and so the original reason for having a separate function no longer applies.

Author: akoeplinger

src/mono/mono/eglib/ghashtable.c

@@ -675,8 +675,8 @@ g_str_hash (gconstpointer v1)
 	guint hash = 0;
 	unsigned char *p = (unsigned char *) v1;
 
-	while (*p++)
-		hash = (hash << 5) - (hash + *p);
+	while (*p)
+		hash = (hash << 5) - (hash + *p++);
 
 	return hash;
 }

src/mono/mono/metadata/class-init.c

@@ -511,7 +511,7 @@ mono_class_create_from_typedef (MonoImage *image, guint32 type_token, MonoError
 	}
 
 	klass->name = name;
-	klass->name_hash = mono_metadata_str_hash (name);
+	klass->name_hash = g_str_hash (name);
 	klass->name_space = nspace;
 
 	MONO_PROFILER_RAISE (class_loading, (klass));
@@ -1176,7 +1176,7 @@ mono_class_create_bounded_array (MonoClass *eclass, guint32 rank, gboolean bound
 	name [nsize + maxrank + bounded] = ']';
 	name [nsize + maxrank + bounded + 1] = 0;
 	klass->name = mm ? mono_mem_manager_strdup (mm, name) : mono_image_strdup (image, name);
-	klass->name_hash = mono_metadata_str_hash (klass->name);
+	klass->name_hash = g_str_hash (klass->name);
 	g_free (name);
 
 	klass->type_token = 0;
@@ -1534,7 +1534,7 @@ mono_class_create_ptr (MonoType *type)
 	result->name_space = el_class->name_space;
 	name = g_strdup_printf ("%s*", el_class->name);
 	result->name = mm ? mono_mem_manager_strdup (mm, name) : mono_image_strdup (image, name);
-	result->name_hash = mono_metadata_str_hash (result->name);
+	result->name_hash = g_str_hash (result->name);
 	result->class_kind = MONO_CLASS_POINTER;
 	g_free (name);
 
@@ -1613,7 +1613,7 @@ mono_class_create_fnptr (MonoMethodSignature *sig)
 	result->parent = NULL; /* no parent for PTR types */
 	result->name_space = "System";
 	result->name = "MonoFNPtrFakeClass";
-	result->name_hash = mono_metadata_str_hash (result->name);
+	result->name_hash = g_str_hash (result->name);
 	result->class_kind = MONO_CLASS_POINTER;
 
 	result->image = mono_defaults.corlib; /* need to fix... */

src/mono/mono/metadata/metadata-internals.h

@@ -1070,8 +1070,6 @@ mono_metadata_get_corresponding_property_from_generic_type_definition (MonoPrope
 guint32
 mono_metadata_signature_size (MonoMethodSignature *sig);
 
-guint mono_metadata_str_hash (gconstpointer v1);
-
 gboolean mono_image_load_pe_data (MonoImage *image);
 
 gboolean mono_image_load_cli_data (MonoImage *image);

src/mono/mono/metadata/metadata.c

@@ -5598,29 +5598,6 @@ mono_metadata_generic_context_equal (const MonoGenericContext *g1, const MonoGen
 	return g1->class_inst == g2->class_inst && g1->method_inst == g2->method_inst;
 }
 
-/*
- * mono_metadata_str_hash:
- *
- *   This should be used instead of g_str_hash for computing hash codes visible
- * outside this module, since g_str_hash () is not guaranteed to be stable
- * (its not the same in eglib for example).
- */
-guint
-mono_metadata_str_hash (gconstpointer v1)
-{
-	/* Same as g_str_hash () in glib */
-	/* note: signed/unsigned char matters - we feed UTF-8 to this function, so the high bit will give diferent results if we don't match. */
-	unsigned char *p = (unsigned char *) v1;
-	guint hash = *p;
-
-	while (*p++) {
-		if (*p)
-			hash = (hash << 5) - hash + *p;
-	}
-
-	return hash;
-}
-
 /**
  * mono_metadata_type_hash:
  * \param t1 a type

src/mono/mono/metadata/object.c

@@ -1311,7 +1311,6 @@ field_is_special_static (MonoClass *fklass, MonoClassField *field)
  *   The IMT slot is embedded into AOTed code, so this must return the same value
  * for the same method across all executions. This means:
  * - pointers shouldn't be used as hash values.
- * - mono_metadata_str_hash () should be used for hashing strings.
  */
 guint32
 mono_method_get_imt_slot (MonoMethod *method)
@@ -1348,8 +1347,8 @@ mono_method_get_imt_slot (MonoMethod *method)
 
 	/* Initialize hashes */
 	hashes [0] = m_class_get_name_hash (method->klass);
-	hashes [1] = mono_metadata_str_hash (m_class_get_name_space (method->klass));
-	hashes [2] = mono_metadata_str_hash (method->name);
+	hashes [1] = g_str_hash (m_class_get_name_space (method->klass));
+	hashes [2] = g_str_hash (method->name);
 	hashes [3] = mono_metadata_type_hash (sig->ret);
 	for (i = 0; i < sig->param_count; i++) {
 		hashes [4 + i] = mono_metadata_type_hash (sig->params [i]);

src/mono/mono/metadata/sre.c

@@ -2558,7 +2558,7 @@ reflection_setup_internal_class_internal (MonoReflectionTypeBuilderHandle ref_tb
 	klass->inited = 1; /* we lie to the runtime */
 	klass->name = mono_string_to_utf8_image (klass->image, ref_name, error);
 	goto_if_nok (error, leave);
-	klass->name_hash = mono_metadata_str_hash (klass->name);
+	klass->name_hash = g_str_hash (klass->name);
 	klass->name_space = mono_string_to_utf8_image (klass->image, ref_nspace, error);
 	goto_if_nok (error, leave);
 	klass->type_token = MONO_TOKEN_TYPE_DEF | table_idx;

src/mono/mono/mini/aot-compiler.c

@@ -11284,18 +11284,18 @@ mono_aot_method_hash (MonoMethod *method)
 		else
 			full_name = mono_type_full_name (m_class_get_byval_arg (klass));
 
-		hashes [0] = mono_metadata_str_hash (full_name);
+		hashes [0] = g_str_hash (full_name);
 		hashes [1] = 0;
 		g_free (full_name);
 	} else {
 		hashes [0] = m_class_get_name_hash (klass);
-		hashes [1] = mono_metadata_str_hash (m_class_get_name_space (klass));
+		hashes [1] = g_str_hash (m_class_get_name_space (klass));
 	}
 	if (method->wrapper_type == MONO_WRAPPER_MANAGED_TO_NATIVE && mono_marshal_get_wrapper_info (method)->subtype == WRAPPER_SUBTYPE_ICALL_WRAPPER)
 		/* The name might not be set correctly if DISABLE_JIT is set */
 		hashes [2] = mono_marshal_get_wrapper_info (method)->d.icall.jit_icall_id;
 	else
-		hashes [2] = mono_metadata_str_hash (method->name);
+		hashes [2] = g_str_hash (method->name);
 
 	if (method->wrapper_type == MONO_WRAPPER_OTHER) {
 		if (info && (info->subtype == WRAPPER_SUBTYPE_GSHAREDVT_IN_SIG || info->subtype == WRAPPER_SUBTYPE_GSHAREDVT_OUT_SIG))
@@ -11710,7 +11710,7 @@ static uint32_t
 hash_for_class (MonoClass *klass)
 {
 	char *full_name = get_class_full_name_for_hash (klass);
-	uint32_t hash = mono_metadata_str_hash (full_name);
+	uint32_t hash = g_str_hash (full_name);
 	g_free (full_name);
 	return hash;
 }
@@ -12094,7 +12094,7 @@ emit_globals (MonoAotCompile *acfg)
 	for (guint i = 0; i < acfg->globals->len; ++i) {
 		char *name = (char *)g_ptr_array_index (acfg->globals, i);
 
-		hash = mono_metadata_str_hash (name) % table_size;
+		hash = g_str_hash (name) % table_size;
 
 		/* FIXME: Allocate from the mempool */
 		new_entry = g_new0 (GlobalsTableEntry, 1);

src/mono/mono/mini/aot-runtime.c

@@ -1545,7 +1545,7 @@ find_symbol (MonoDl *module, gpointer *globals, const char *name, gpointer *valu
 		table_size = table [0];
 		table ++;
 
-		hash = mono_metadata_str_hash (symbol) % table_size;
+		hash = g_str_hash (symbol) % table_size;
 
 		entry = &table [hash * 2];
 
@@ -2716,9 +2716,9 @@ mono_aot_get_class_from_name (MonoImage *image, const char *name_space, const ch
 	}
 #ifdef DEBUG_AOT_NAME_TABLE
 	debug_full_name = g_strdup (full_name);
-	debug_hash = mono_metadata_str_hash (full_name) % table_size;
+	debug_hash = g_str_hash (full_name) % table_size;
 #endif
-	hash = mono_metadata_str_hash (full_name) % table_size;
+	hash = g_str_hash (full_name) % table_size;
 	if (full_name != full_name_buf)
 		g_free (full_name);