Hello,
this is the new code, posted for review.
The reason for the rewrite is explained in the comment here:
https://bugzilla.novell.com/show_bug.cgi?id=319930
Now that monster piece of code (point [4] in the comment) is
gone, and the new code is hopefully easy to understand.
The patch still contains the old code, so that it can be
quickly enabled to compare the two behaviors if there are
further issues, but after we are confident that the new one
works it should go away.
Anyway, the patch passes everything here.
Ciao,
Massi
Index: mono/metadata/class.c
===================================================================
--- mono/metadata/class.c (revision 94247)
+++ mono/metadata/class.c (working copy)
@@ -1760,9 +1760,11 @@
printf ("Packed interface table for class %s has size %d\n", klass->name, klass->interface_offsets_count);
for (i = 0; i < klass->interface_offsets_count; i++)
- printf (" [%d][UUID %d][SLOT %d] interface %s\n", i,
+ printf (" [%03d][UUID %03d][SLOT %03d][SIZE %03d] interface %s.%s\n", i,
klass->interfaces_packed [i]->interface_id,
klass->interface_offsets_packed [i],
+ klass->interfaces_packed [i]->method.count,
+ klass->interfaces_packed [i]->name_space,
klass->interfaces_packed [i]->name );
printf ("Interface flags: ");
for (i = 0; i <= klass->max_interface_id; i++)
@@ -1782,6 +1784,12 @@
for (i = 0; i < ifaces->len; i++) {
MonoClass *ic = g_ptr_array_index (ifaces, i);
printf (" [UIID %d] interface %s\n", ic->interface_id, ic->name);
+ printf (" [%03d][UUID %03d][SLOT %03d][SIZE %03d] interface %s.%s\n", i,
+ ic->interface_id,
+ mono_class_interface_offset (klass, ic),
+ ic->method.count,
+ ic->name_space,
+ ic->name );
}
g_ptr_array_free (ifaces, TRUE);
}
@@ -2247,6 +2255,227 @@
}
}
+#define USE_NEW_INTERFACE_VTABLE_CODE 1
+
+#define DEBUG_INTERFACE_VTABLE_CODE 0
+#define TRACE_INTERFACE_VTABLE_CODE 0
+
+#if (TRACE_INTERFACE_VTABLE_CODE|DEBUG_INTERFACE_VTABLE_CODE)
+#define DEBUG_INTERFACE_VTABLE(stmt) do {\
+ stmt;\
+} while (0)
+#else
+#define DEBUG_INTERFACE_VTABLE(stmt)
+#endif
+
+#if TRACE_INTERFACE_VTABLE_CODE
+#define TRACE_INTERFACE_VTABLE(stmt) do {\
+ stmt;\
+} while (0)
+#else
+#define TRACE_INTERFACE_VTABLE(stmt)
+#endif
+
+
+#if (TRACE_INTERFACE_VTABLE_CODE|DEBUG_INTERFACE_VTABLE_CODE)
+static char*
+mono_signature_get_full_desc (MonoMethodSignature *sig, gboolean include_namespace)
+{
+ int i;
+ char *result;
+ GString *res = g_string_new ("");
+
+ g_string_append_c (res, '(');
+ for (i = 0; i < sig->param_count; ++i) {
+ if (i > 0)
+ g_string_append_c (res, ',');
+ mono_type_get_desc (res, sig->params [i], include_namespace);
+ }
+ g_string_append (res, ")=>");
+ if (sig->ret != NULL) {
+ mono_type_get_desc (res, sig->ret, include_namespace);
+ } else {
+ g_string_append (res, "NULL");
+ }
+ result = res->str;
+ g_string_free (res, FALSE);
+ return result;
+}
+static void
+print_method_signatures (MonoMethod *im, MonoMethod *cm) {
+ char *im_sig = mono_signature_get_full_desc (mono_method_signature (im), TRUE);
+ char *cm_sig = mono_signature_get_full_desc (mono_method_signature (cm), TRUE);
+ printf ("(IM \"%s\", CM \"%s\")", im_sig, cm_sig);
+ g_free (im_sig);
+ g_free (cm_sig);
+
+}
+
+#endif
+static gboolean
+check_interface_method_override (MonoClass *class, MonoMethod *im, MonoMethod *cm, gboolean require_newslot, gboolean interface_is_explicitly_implemented_by_class, gboolean slot_is_empty, gboolean security_enabled) {
+ if (strcmp (im->name, cm->name) == 0) {
+ if (! (cm->flags & METHOD_ATTRIBUTE_PUBLIC)) {
+ TRACE_INTERFACE_VTABLE (printf ("[PUBLIC CHECK FAILED]"));
+ return FALSE;
+ }
+ if (! slot_is_empty) {
+ if (require_newslot) {
+ if (! interface_is_explicitly_implemented_by_class) {
+ TRACE_INTERFACE_VTABLE (printf ("[NOT EXPLICIT IMPLEMENTATION IN FULL SLOT REFUSED]"));
+ return FALSE;
+ }
+ if (! (cm->flags & METHOD_ATTRIBUTE_NEW_SLOT)) {
+ TRACE_INTERFACE_VTABLE (printf ("[NEWSLOT CHECK FAILED]"));
+ return FALSE;
+ }
+ } else {
+ TRACE_INTERFACE_VTABLE (printf ("[FULL SLOT REFUSED]"));
+ }
+ }
+ if (! mono_metadata_signature_equal (mono_method_signature (cm), mono_method_signature (im))) {
+ TRACE_INTERFACE_VTABLE (printf ("[SIGNATURE CHECK FAILED "));
+ TRACE_INTERFACE_VTABLE (print_method_signatures (im, cm));
+ TRACE_INTERFACE_VTABLE (printf ("]"));
+ return FALSE;
+ }
+ TRACE_INTERFACE_VTABLE (printf ("[SECURITY CHECKS]"));
+ /* CAS - SecurityAction.InheritanceDemand on interface */
+ if (security_enabled && (im->flags & METHOD_ATTRIBUTE_HAS_SECURITY)) {
+ mono_secman_inheritancedemand_method (cm, im);
+ }
+
+ if (mono_security_get_mode () == MONO_SECURITY_MODE_CORE_CLR)
+ check_core_clr_override_method (class, cm, im);
+ TRACE_INTERFACE_VTABLE (printf ("[NAME CHECK OK]"));
+ return TRUE;
+ } else {
+ MonoClass *ic = im->klass;
+ const char *ic_name_space = ic->name_space;
+ const char *ic_name = ic->name;
+ char *subname;
+
+ if (! require_newslot) {
+ TRACE_INTERFACE_VTABLE (printf ("[INJECTED METHOD REFUSED]"));
+ return FALSE;
+ }
+ if (cm->klass->rank == 0) {
+ TRACE_INTERFACE_VTABLE (printf ("[RANK CHECK FAILED]"));
+ return FALSE;
+ }
+ if (! mono_metadata_signature_equal (mono_method_signature (cm), mono_method_signature (im))) {
+ TRACE_INTERFACE_VTABLE (printf ("[(INJECTED) SIGNATURE CHECK FAILED "));
+ TRACE_INTERFACE_VTABLE (print_method_signatures (im, cm));
+ TRACE_INTERFACE_VTABLE (printf ("]"));
+ return FALSE;
+ }
+ if (mono_class_get_image (ic) != mono_defaults.corlib) {
+ TRACE_INTERFACE_VTABLE (printf ("[INTERFACE CORLIB CHECK FAILED]"));
+ return FALSE;
+ }
+ if ((ic_name_space == NULL) || (strcmp (ic_name_space, "System.Collections.Generic") != 0)) {
+ TRACE_INTERFACE_VTABLE (printf ("[INTERFACE NAMESPACE CHECK FAILED]"));
+ return FALSE;
+ }
+ if ((ic_name == NULL) || ((strcmp (ic_name, "IEnumerable`1") != 0) && (strcmp (ic_name, "ICollection`1") != 0) && (strcmp (ic_name, "IList`1") != 0))) {
+ TRACE_INTERFACE_VTABLE (printf ("[INTERFACE NAME CHECK FAILED]"));
+ return FALSE;
+ }
+
+ subname = strstr (cm->name, ic_name_space);
+ if (subname != cm->name) {
+ TRACE_INTERFACE_VTABLE (printf ("[ACTUAL NAMESPACE CHECK FAILED]"));
+ return FALSE;
+ }
+ subname += strlen (ic_name_space);
+ if (subname [0] != '.') {
+ TRACE_INTERFACE_VTABLE (printf ("[FIRST DOT CHECK FAILED]"));
+ return FALSE;
+ }
+ subname ++;
+ if (strstr (subname, ic_name) != subname) {
+ TRACE_INTERFACE_VTABLE (printf ("[ACTUAL CLASS NAME CHECK FAILED]"));
+ return FALSE;
+ }
+ subname += strlen (ic_name);
+ if (subname [0] != '.') {
+ TRACE_INTERFACE_VTABLE (printf ("[SECOND DOT CHECK FAILED]"));
+ return FALSE;
+ }
+ subname ++;
+ if (strcmp (subname, im->name) != 0) {
+ TRACE_INTERFACE_VTABLE (printf ("[METHOD NAME CHECK FAILED]"));
+ return FALSE;
+ }
+
+ TRACE_INTERFACE_VTABLE (printf ("[SECURITY CHECKS (INJECTED CASE)]"));
+ /* CAS - SecurityAction.InheritanceDemand on interface */
+ if (security_enabled && (im->flags & METHOD_ATTRIBUTE_HAS_SECURITY)) {
+ mono_secman_inheritancedemand_method (cm, im);
+ }
+
+ if (mono_security_get_mode () == MONO_SECURITY_MODE_CORE_CLR)
+ check_core_clr_override_method (class, cm, im);
+
+ TRACE_INTERFACE_VTABLE (printf ("[INJECTED INTERFACE CHECK OK]"));
+ return TRUE;
+ }
+}
+
+#if (TRACE_INTERFACE_VTABLE_CODE|DEBUG_INTERFACE_VTABLE_CODE)
+static void
+foreach_override (gpointer key, gpointer value, gpointer user_data) {
+ MonoMethod *method = key;
+ MonoMethod *override = value;
+ MonoClass *method_class = mono_method_get_class (method);
+ MonoClass *override_class = mono_method_get_class (override);
+
+ printf (" Method '%s.%s:%s' has override '%s.%s:%s'\n",
+ mono_class_get_namespace (method_class), mono_class_get_name (method_class), mono_method_get_name (method),
+ mono_class_get_namespace (override_class), mono_class_get_name (override_class), mono_method_get_name (override));
+}
+static void
+print_overrides (GHashTable *override_map, const char *message) {
+ if (override_map) {
+ printf ("Override map \"%s\" START:\n", message);
+ g_hash_table_foreach (override_map, foreach_override, NULL);
+ printf ("Override map \"%s\" END.\n", message);
+ } else {
+ printf ("Override map \"%s\" EMPTY.\n", message);
+ }
+}
+static void
+print_vtable_full (MonoClass *class, MonoMethod** vtable, int size, int first_non_interface_slot, const char *message, gboolean print_interfaces) {
+ char *full_name = mono_type_full_name (&class->byval_arg);
+ int i;
+ int parent_size;
+
+ printf ("*** Vtable for class '%s' at \"%s\" (size %d)\n", full_name, message, size);
+
+ if (print_interfaces) {
+ print_implemented_interfaces (class);
+ printf ("* Interfaces for class '%s' done.\nStarting vtable (size %d):\n", full_name, size);
+ }
+
+ if (class->parent) {
+ parent_size = class->parent->vtable_size;
+ } else {
+ parent_size = 0;
+ }
+ for (i = 0; i < size; ++i) {
+ MonoMethod *cm = vtable [i];
+ if (cm) {
+ char *cm_name = mono_method_full_name (cm, TRUE);
+ char newness = (i < parent_size) ? 'O' : ((i < first_non_interface_slot) ? 'I' : 'N');
+ printf (" [%c][%03d][INDEX %03d] %s\n", newness, i, cm->slot, cm_name);
+ g_free (cm_name);
+ }
+ }
+
+ g_free (full_name);
+}
+#endif
+
/*
* LOCKING: this is supposed to be called with the loader lock held.
*/
@@ -2259,6 +2488,9 @@
GPtrArray *ifaces, *pifaces = NULL;
GHashTable *override_map = NULL;
gboolean security_enabled = mono_is_security_manager_active ();
+#if (DEBUG_INTERFACE_VTABLE_CODE|TRACE_INTERFACE_VTABLE_CODE)
+ int first_non_interface_slot;
+#endif
if (class->vtable)
return;
@@ -2270,6 +2502,7 @@
max_vtsize += ic->method.count;
}
g_ptr_array_free (ifaces, TRUE);
+ ifaces = NULL;
}
if (class->parent) {
@@ -2288,10 +2521,44 @@
cur_slot = setup_interface_offsets (class, cur_slot);
max_iid = class->max_interface_id;
+ DEBUG_INTERFACE_VTABLE (first_non_interface_slot = cur_slot);
+#if USE_NEW_INTERFACE_VTABLE_CODE
+ if (class->parent && class->parent->vtable_size) {
+ MonoClass *parent = class->parent;
+ int i;
+
+ memcpy (vtable, parent->vtable, sizeof (gpointer) * parent->vtable_size);
+
+ // Also inherit parent interface vtables, just as a starting point.
+ // This is needed otherwise bug-77127.exe fails when the property methods
+ // have different names in the iterface and the class, because for child
+ // classes the ".override" information is not used anymore.
+ for (i = 0; i < parent->interface_offsets_count; i++) {
+ MonoClass *parent_interface = parent->interfaces_packed [i];
+ int interface_offset = mono_class_interface_offset (class, parent_interface);
+
+ if (interface_offset >= parent->vtable_size) {
+ int parent_interface_offset = mono_class_interface_offset (parent, parent_interface);
+ int j;
+
+ mono_class_setup_methods (parent_interface);
+ TRACE_INTERFACE_VTABLE (printf (" +++ Inheriting interface %s.%s\n", parent_interface->name_space, parent_interface->name));
+ for (j = 0; j < parent_interface->method.count; j++) {
+ vtable [interface_offset + j] = parent->vtable [parent_interface_offset + j];
+ TRACE_INTERFACE_VTABLE (printf (" --- Inheriting: [%03d][(%03d)+(%03d)] => [%03d][(%03d)+(%03d)]\n",
+ parent_interface_offset + j, parent_interface_offset, j,
+ interface_offset + j, interface_offset, j));
+ }
+ }
+
+ }
+ }
+#else
if (class->parent && class->parent->vtable_size)
memcpy (vtable, class->parent->vtable, sizeof (gpointer) * class->parent->vtable_size);
-
+#endif
+ TRACE_INTERFACE_VTABLE (print_vtable_full (class, vtable, cur_slot, first_non_interface_slot, "AFTER INHERITING PARENT VTABLE", TRUE));
/* override interface methods */
for (i = 0; i < onum; i++) {
MonoMethod *decl = overrides [i*2];
@@ -2311,7 +2578,136 @@
check_core_clr_override_method (class, vtable [dslot], decl);
}
}
+ TRACE_INTERFACE_VTABLE (print_overrides (override_map, "AFTER OVERRIDING INTERFACE METHODS"));
+ TRACE_INTERFACE_VTABLE (print_vtable_full (class, vtable, cur_slot, first_non_interface_slot, "AFTER OVERRIDING INTERFACE METHODS", FALSE));
+#if USE_NEW_INTERFACE_VTABLE_CODE
+ // Loop on all implemented interfaces...
+ for (i = 0; i < class->interface_offsets_count; i++) {
+ MonoClass *parent = class->parent;
+ int ic_offset;
+ gboolean interface_is_explicitly_implemented_by_class;
+ int im_index;
+
+ ic = class->interfaces_packed [i];
+ ic_offset = mono_class_interface_offset (class, ic);
+
+ // Check if this interface is explicitly implemented (instead of just inherited)
+ if (parent != NULL) {
+ int implemented_interfaces_index;
+ interface_is_explicitly_implemented_by_class = FALSE;
+ for (implemented_interfaces_index = 0; implemented_interfaces_index < class->interface_count; implemented_interfaces_index++) {
+ if (ic == class->interfaces [implemented_interfaces_index]) {
+ interface_is_explicitly_implemented_by_class = TRUE;
+ break;
+ }
+ }
+ } else {
+ interface_is_explicitly_implemented_by_class = TRUE;
+ }
+
+ // Loop on all interface methods...
+ for (im_index = 0; im_index < ic->method.count; im_index++) {
+ MonoMethod *im = ic->methods [im_index];
+ int im_slot = ic_offset + im->slot;
+ MonoMethod *override_im = (override_map != NULL) ? g_hash_table_lookup (override_map, im) : NULL;
+
+ // If there is an explicit implementation, just use it right away,
+ // otherwise look for a matching method
+ if (override_im == NULL) {
+ int cm_index;
+
+ // First look for a suitable method among the class methods
+ for (cm_index = 0; cm_index < class->method.count; cm_index++) {
+ MonoMethod *cm = class->methods [cm_index];
+
+ TRACE_INTERFACE_VTABLE (printf (" For slot %d ('%s'.'%s':'%s'), trying method '%s'.'%s':'%s'... [EXPLICIT IMPLEMENTATION = %d][SLOT IS NULL = %d]", im_slot, ic->name_space, ic->name, im->name, cm->klass->name_space, cm->klass->name, cm->name, interface_is_explicitly_implemented_by_class, (vtable [im_slot] == NULL)));
+ if ((cm->flags & METHOD_ATTRIBUTE_VIRTUAL) && check_interface_method_override (class, im, cm, TRUE, interface_is_explicitly_implemented_by_class, (vtable [im_slot] == NULL), security_enabled)) {
+ TRACE_INTERFACE_VTABLE (printf ("[check ok]: ASSIGNING"));
+ vtable [im_slot] = cm;
+ /* Why do we need this? */
+ if (vtable [im_slot]->slot < 0) {
+ vtable [im_slot]->slot = im_slot;
+ }
+ }
+ TRACE_INTERFACE_VTABLE (printf ("\n"));
+ }
+
+ // If the slot is still empty, look in all the inherited virtual methods...
+ if ((vtable [im_slot] == NULL) && class->parent != NULL) {
+ MonoClass *parent = class->parent;
+ // Reverse order, so that last added methods are preferred
+ for (cm_index = parent->vtable_size - 1; cm_index >= 0; cm_index--) {
+ MonoMethod *cm = parent->vtable [cm_index];
+
+ TRACE_INTERFACE_VTABLE ((cm != NULL) && printf (" For slot %d ('%s'.'%s':'%s'), trying (ancestor) method '%s'.'%s':'%s'... ", im_slot, ic->name_space, ic->name, im->name, cm->klass->name_space, cm->klass->name, cm->name));
+ if ((cm != NULL) && check_interface_method_override (class, im, cm, FALSE, FALSE, TRUE, security_enabled)) {
+ TRACE_INTERFACE_VTABLE (printf ("[everything ok]: ASSIGNING"));
+ vtable [im_slot] = cm;
+ /* Why do we need this? */
+ if (vtable [im_slot]->slot < 0) {
+ vtable [im_slot]->slot = im_slot;
+ }
+ break;
+ }
+ TRACE_INTERFACE_VTABLE ((cm != NULL) && printf ("\n"));
+ }
+ }
+ } else {
+ g_assert (vtable [im_slot] == override_im);
+ }
+ }
+ }
+
+ // If the class is not abstract, check that all its interface slots are full.
+ // The check is done here and not directly at the end of the loop above because
+ // it can happen (for injected generic array interfaces) that the same slot is
+ // processed multiple times (those interfaces have overlapping slots), and it
+ // will not always be the first pass the one that fills the slot.
+ if (! (class->flags & TYPE_ATTRIBUTE_ABSTRACT)) {
+ for (i = 0; i < class->interface_offsets_count; i++) {
+ int ic_offset;
+ int im_index;
+
+ ic = class->interfaces_packed [i];
+ ic_offset = mono_class_interface_offset (class, ic);
+
+ for (im_index = 0; im_index < ic->method.count; im_index++) {
+ MonoMethod *im = ic->methods [im_index];
+ int im_slot = ic_offset + im->slot;
+
+ TRACE_INTERFACE_VTABLE (printf (" [class is not abstract, checking slot %d for interface '%s'.'%s', method %s, slot check is %d]\n",
+ im_slot, ic->name_space, ic->name, im->name, (vtable [im_slot] == NULL)));
+ if (vtable [im_slot] == NULL) {
+ int index;
+ char *method_signature;
+ for (index = 0; index < onum; ++index) {
+ g_print (" at slot %d: %s (%d) overrides %s (%d)\n", im_slot, overrides [index*2+1]->name,
+ overrides [index*2+1]->slot, overrides [index*2]->name, overrides [index*2]->slot);
+ }
+ method_signature = mono_signature_get_desc (mono_method_signature (im), FALSE);
+ printf ("no implementation for interface method %s::%s(%s) in class %s.%s\n",
+ mono_type_get_name (&ic->byval_arg), im->name, method_signature, class->name_space, class->name);
+ g_free (method_signature);
+ for (index = 0; index < class->method.count; ++index) {
+ MonoMethod *cm = class->methods [index];
+ method_signature = mono_signature_get_desc (mono_method_signature (cm), TRUE);
+
+ printf ("METHOD %s(%s)\n", cm->name, method_signature);
+ g_free (method_signature);
+ }
+
+ mono_class_set_failure (class, MONO_EXCEPTION_TYPE_LOAD, NULL);
+
+ if (override_map)
+ g_hash_table_destroy (override_map);
+
+ return;
+ }
+ }
+ }
+ }
+#else
for (k = class; k ; k = k->parent) {
int nifaces = 0;
@@ -2364,6 +2760,9 @@
g_assert (io + l <= max_vtsize);
vtable [io + l] = cm;
+ TRACE_INTERFACE_VTABLE (printf (" [NOA] Filling slot %d (%d+%d) with method '%s'.'%s':'%s' ", io + l, io, l, cm->klass->name_space, cm->klass->name, cm->name));
+ TRACE_INTERFACE_VTABLE (print_method_signatures (im, cm));
+ TRACE_INTERFACE_VTABLE (printf ("\n"));
}
}
}
@@ -2417,6 +2816,9 @@
g_assert (io + l <= max_vtsize);
vtable [io + l] = cm;
+ TRACE_INTERFACE_VTABLE (printf (" [FQN] Filling slot %d (%d+%d) with method '%s'.'%s':'%s' ", io + l, io, l, cm->klass->name_space, cm->klass->name, cm->name));
+ TRACE_INTERFACE_VTABLE (print_method_signatures (im, cm));
+ TRACE_INTERFACE_VTABLE (printf ("\n"));
break;
}
}
@@ -2457,6 +2859,9 @@
g_assert (io + l <= max_vtsize);
vtable [io + l] = cm;
+ TRACE_INTERFACE_VTABLE (printf (" [SQN] Filling slot %d (%d+%d) with method '%s'.'%s':'%s' ", io + l, io, l, cm->klass->name_space, cm->klass->name, cm->name));
+ TRACE_INTERFACE_VTABLE (print_method_signatures (im, cm));
+ TRACE_INTERFACE_VTABLE (printf ("\n"));
break;
}
@@ -2486,6 +2891,7 @@
if (MONO_CLASS_IMPLEMENTS_INTERFACE (parent, ic->interface_id) &&
parent->vtable) {
vtable [io + l] = parent->vtable [mono_class_interface_offset (parent, ic) + l];
+ TRACE_INTERFACE_VTABLE (printf (" [INH] Filling slot %d (%d+%d) with method '%s'.'%s':'%s'\n", io + l, io, l, vtable [io + l]->klass->name_space, vtable [io + l]->klass->name, vtable [io + l]->name));
}
}
}
@@ -2535,7 +2941,8 @@
if (ifaces)
g_ptr_array_free (ifaces, TRUE);
}
-
+#endif
+ TRACE_INTERFACE_VTABLE (print_vtable_full (class, vtable, cur_slot, first_non_interface_slot, "AFTER SETTING UP INTERFACE METHODS", FALSE));
for (i = 0; i < class->method.count; ++i) {
MonoMethod *cm;
@@ -2652,6 +3059,7 @@
memcpy (class->vtable, vtable, sizeof (gpointer) * class->vtable_size);
}
+ DEBUG_INTERFACE_VTABLE (print_vtable_full (class, class->vtable, class->vtable_size, first_non_interface_slot, "FINALLY", FALSE));
if (mono_print_vtable) {
int icount = 0;
_______________________________________________
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list
