[metadata] Generate type-3 GUIDs for interfaces. (dotnet#43677)

I'm opening this although it's still WIP, for discussion, as I'm not sure what else I should be adding. Tests probably, of course, but also I was wondering if it should be plugged to RuntimeType.GUID / Marshal.GenerateGuidForType. I'm not entirely sure if that's desired as I've seen clues in the reference source that the GUID generation has issues.

I've left class GUID generation aside, as it doesn't seem so useful for COM interop, and the algorithm uses some Assembly properties to seed the GUID, and retrieving that is currently beyond my understanding of the code.

Also I'm a bit lost regarding which RuntimeType.GUID it should be added to: there's at least two places that looked appropriate : in `mono/netcore/System.Private.CoreLib/src/RuntimeType.Mono.cs` and in `mono/mcs/class/corlib/ReferenceSources/RuntimeType.cs`.

Co-authored-by: rbernon <[email protected]>

Author: monojenkins

src/mono/mono/metadata/cominterop.c

@@ -150,7 +150,6 @@ GENERATE_GET_CLASS_WITH_CACHE (com_object, "System", "__ComObject")
 GENERATE_GET_CLASS_WITH_CACHE (variant,    "System", "Variant")
 
 static GENERATE_GET_CLASS_WITH_CACHE (interface_type_attribute, "System.Runtime.InteropServices", "InterfaceTypeAttribute")
-static GENERATE_GET_CLASS_WITH_CACHE (guid_attribute, "System.Runtime.InteropServices", "GuidAttribute")
 static GENERATE_GET_CLASS_WITH_CACHE (com_visible_attribute, "System.Runtime.InteropServices", "ComVisibleAttribute")
 static GENERATE_GET_CLASS_WITH_CACHE (com_default_interface_attribute, "System.Runtime.InteropServices", "ComDefaultInterfaceAttribute")
 static GENERATE_GET_CLASS_WITH_CACHE (class_interface_attribute, "System.Runtime.InteropServices", "ClassInterfaceAttribute")
@@ -487,30 +486,13 @@ cominterop_get_com_slot_for_method (MonoMethod* method, MonoError* error)
 	return slot + cominterop_get_com_slot_begin (ic);
 }
 
-static void
-cominterop_mono_string_to_guid (MonoString* string, guint8 *guid);
-
 static gboolean
 cominterop_class_guid (MonoClass* klass, guint8* guid)
 {
 	ERROR_DECL (error);
-	MonoCustomAttrInfo *cinfo;
-
-	cinfo = mono_custom_attrs_from_class_checked (klass, error);
-	mono_error_assert_ok (error);
-	if (cinfo) {
-		MonoReflectionGuidAttribute *attr = (MonoReflectionGuidAttribute*)mono_custom_attrs_get_attr_checked (cinfo, mono_class_get_guid_attribute_class (), error);
-		mono_error_assert_ok (error); /*FIXME proper error handling*/
-
-		if (!attr)
-			return FALSE;
-		if (!cinfo->cached)
-			mono_custom_attrs_free (cinfo);
-
-		cominterop_mono_string_to_guid (attr->guid, guid);
-		return TRUE;
-	}
-	return FALSE;
+	mono_metadata_get_class_guid (klass, guid, error);
+	mono_error_assert_ok (error); /*FIXME proper error handling*/
+	return TRUE;
 }
 
 static gboolean
@@ -552,8 +534,8 @@ cominterop_com_visible (MonoClass* klass)
 
 }
 
-static gboolean
-cominterop_method_com_visible (MonoMethod *method)
+gboolean
+mono_cominterop_method_com_visible (MonoMethod *method)
 {
 	ERROR_DECL (error);
 	MonoCustomAttrInfo *cinfo;
@@ -2157,7 +2139,7 @@ cominterop_class_method_is_visible (MonoMethod *method)
 	if (flags & METHOD_ATTRIBUTE_RT_SPECIAL_NAME)
 		return FALSE;
 
-	if (!cominterop_method_com_visible (method))
+	if (!mono_cominterop_method_com_visible (method))
 		return FALSE;
 
 	/* if the method is an override, ignore it and use the original definition */
@@ -2713,22 +2695,6 @@ cominterop_get_managed_wrapper_adjusted (MonoMethod *method)
 	return res;
 }
 
-/**
- * cominterop_mono_string_to_guid:
- *
- * Converts the standard string representation of a GUID 
- * to a 16 byte Microsoft GUID.
- */
-static void
-cominterop_mono_string_to_guid (MonoString* string, guint8 *guid) {
-	gunichar2 * chars = mono_string_chars_internal (string);
-	int i = 0;
-	static const guint8 indexes[16] = {7, 5, 3, 1, 12, 10, 17, 15, 20, 22, 25, 27, 29, 31, 33, 35};
-
-	for (i = 0; i < sizeof(indexes); i++)
-		guid [i] = g_unichar_xdigit_value (chars [indexes [i]]) + (g_unichar_xdigit_value (chars [indexes [i] - 1]) << 4);
-}
-
 static gboolean
 cominterop_class_guid_equal (const guint8* guid, MonoClass* klass)
 {

src/mono/mono/metadata/cominterop.h

@@ -73,4 +73,7 @@ mono_cominterop_get_com_interface (MonoObject* object, MonoClass* ic, MonoError
 gboolean
 mono_cominterop_is_interface (MonoClass* klass);
 
+gboolean
+mono_cominterop_method_com_visible (MonoMethod *method);
+
 #endif /* __MONO_COMINTEROP_H__ */

src/mono/mono/metadata/icall-def.h

@@ -872,6 +872,7 @@ HANDLES(RT_30, "GetCorrespondingInflatedConstructor", ves_icall_RuntimeType_GetC
 HANDLES_REUSE_WRAPPER(RT_31, "GetCorrespondingInflatedMethod", ves_icall_RuntimeType_GetCorrespondingInflatedMethod)
 HANDLES(RT_3, "GetEvents_native", ves_icall_RuntimeType_GetEvents_native, GPtrArray_ptr, 3, (MonoReflectionType, char_ptr, guint32))
 HANDLES(RT_5, "GetFields_native", ves_icall_RuntimeType_GetFields_native, GPtrArray_ptr, 4, (MonoReflectionType, char_ptr, guint32, guint32))
+HANDLES(RT_32, "GetGUID", ves_icall_RuntimeType_GetGUID, void, 2, (MonoReflectionType, MonoArray))
 HANDLES(RT_6, "GetGenericArgumentsInternal", ves_icall_RuntimeType_GetGenericArguments, MonoArray, 2, (MonoReflectionType, MonoBoolean))
 HANDLES(RT_9, "GetGenericParameterPosition", ves_icall_RuntimeType_GetGenericParameterPosition, gint32, 1, (MonoReflectionType))
 HANDLES(RT_10, "GetInterfaceMapData", ves_icall_RuntimeType_GetInterfaceMapData, void, 4, (MonoReflectionType, MonoReflectionType, MonoArrayOut, MonoArrayOut))

src/mono/mono/metadata/icall.c

@@ -3294,6 +3294,28 @@ set_type_object_in_array (MonoDomain *domain, MonoType *type, MonoArrayHandle de
 	HANDLE_FUNCTION_RETURN_VAL (is_ok (error));
 }
 
+#ifndef ENABLE_NETCORE
+void
+ves_icall_RuntimeType_GetGUID (MonoReflectionTypeHandle type_handle, MonoArrayHandle guid_handle, MonoError *error)
+{
+	error_init (error);
+
+	g_assert (mono_array_handle_length (guid_handle) == 16);
+	if (MONO_HANDLE_IS_NULL (type_handle)) {
+		mono_error_set_argument_null (error, "type", "");
+		return;
+	}
+
+	MonoType *type = MONO_HANDLE_GETVAL (type_handle, type);
+	MonoClass *klass = mono_class_from_mono_type_internal (type);
+	if (!mono_class_init_checked (klass, error))
+		return;
+
+	guint8 *data = (guint8*) mono_array_addr_with_size_internal (MONO_HANDLE_RAW (guid_handle), 1, 0);
+	mono_metadata_get_class_guid (klass, data, error);
+}
+#endif
+
 MonoArrayHandle
 ves_icall_RuntimeType_GetGenericArguments (MonoReflectionTypeHandle ref_type, MonoBoolean runtimeTypeArray, MonoError *error)
 {

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

@@ -1182,6 +1182,9 @@ mono_type_is_valid_generic_argument (MonoType *type);
 MonoAssemblyContextKind
 mono_asmctx_get_kind (const MonoAssemblyContext *ctx);
 
+void
+mono_metadata_get_class_guid (MonoClass* klass, uint8_t* guid, MonoError *error);
+
 #define MONO_CLASS_IS_INTERFACE_INTERNAL(c) ((mono_class_get_flags (c) & TYPE_ATTRIBUTE_INTERFACE) || mono_type_is_generic_parameter (m_class_get_byval_arg (c)))
 
 static inline gboolean