[mono] Add basic support for static virtual methods on interfaces (#5…

…3465) * [mono] Add basic support for abstract static methods. * Enable some static virtual methods tests on mono. * Disable the tests they still fail on wasm. * Fix interpreter support. * Reenable some tests.
dotnet · Jun 1, 2021 · 50639b8 · 50639b8
1 parent 22640e1
commit 50639b8
Show file tree

Hide file tree

Showing 5 changed files with 103 additions and 48 deletions.
diff --git a/src/mono/mono/metadata/class-inlines.h b/src/mono/mono/metadata/class-inlines.h
@@ -205,6 +205,18 @@ m_class_is_private (MonoClass *klass)
  return (mono_class_get_flags (klass) & TYPE_ATTRIBUTE_VISIBILITY_MASK) == TYPE_ATTRIBUTE_NOT_PUBLIC;
 }
 
+static inline gboolean
+m_method_is_static (MonoMethod *method)
+{
+ return (method->flags & METHOD_ATTRIBUTE_STATIC) != 0;
+}
+
+static inline gboolean
+m_method_is_virtual (MonoMethod *method)
+{
+ return (method->flags & METHOD_ATTRIBUTE_VIRTUAL) != 0;
+}
+
 static inline gboolean
 m_method_is_icall (MonoMethod *method)
 {

diff --git a/src/mono/mono/metadata/class-setup-vtable.c b/src/mono/mono/metadata/class-setup-vtable.c
@@ -710,19 +710,18 @@ verify_class_overrides (MonoClass *klass, MonoMethod **overrides, int onum)
  return FALSE;
  }
 
- if (!(body->flags & METHOD_ATTRIBUTE_VIRTUAL) || (body->flags & METHOD_ATTRIBUTE_STATIC)) {
- if (body->flags & METHOD_ATTRIBUTE_STATIC)
- mono_class_set_type_load_failure (klass, "Method must not be static to override a base type");
- else
- mono_class_set_type_load_failure (klass, "Method must be virtual to override a base type");
+ if (m_method_is_static (decl) != m_method_is_static (body)) {
+ mono_class_set_type_load_failure (klass, "Static method can't override a non-static method and vice versa.");
  return FALSE;
  }
 
- if (!(decl->flags & METHOD_ATTRIBUTE_VIRTUAL) || (decl->flags & METHOD_ATTRIBUTE_STATIC)) {
- if (body->flags & METHOD_ATTRIBUTE_STATIC)
- mono_class_set_type_load_failure (klass, "Cannot override a static method in a base type");
- else
- mono_class_set_type_load_failure (klass, "Cannot override a non virtual method in a base type");
+ if (!m_method_is_virtual (body) && !m_method_is_static (body)) {
+ mono_class_set_type_load_failure (klass, "Method must be virtual to override a base type");
+ return FALSE;
+ }
+
+ if (!m_method_is_virtual (decl)) {
+ mono_class_set_type_load_failure (klass, "Cannot override a non virtual method in a base type");
  return FALSE;
  }
 
@@ -1773,7 +1772,7 @@ mono_class_setup_vtable_general (MonoClass *klass, MonoMethod **overrides, int o
  int im_slot = ic_offset + im->slot;
  MonoMethod *override_im = (override_map != NULL) ? (MonoMethod *)g_hash_table_lookup (override_map, im) : NULL;
 
- if (im->flags & METHOD_ATTRIBUTE_STATIC)
+ if (!m_method_is_virtual (im))
  continue;
 
  TRACE_INTERFACE_VTABLE (printf ("\tchecking iface method %s\n", mono_method_full_name (im,1)));
@@ -1856,7 +1855,7 @@ mono_class_setup_vtable_general (MonoClass *klass, MonoMethod **overrides, int o
  MonoMethod *im = ic->methods [im_index];
  int im_slot = ic_offset + im->slot;
 
- if (im->flags & METHOD_ATTRIBUTE_STATIC)
+ if (!m_method_is_virtual (im))
  continue;
  if (mono_method_get_is_reabstracted (im))
  continue;
@@ -2067,11 +2066,11 @@ mono_class_setup_vtable_general (MonoClass *klass, MonoMethod **overrides, int o
 
  g_assert (cur_slot <= max_vtsize);
 
- /* Ensure that all vtable slots are filled with concrete instance methods */
+ /* Ensure that all vtable slots are filled with concrete methods */
  // Now it is okay to implement a class that is not abstract and implements a interface that has an abstract method because it's reabstracted
  if (!mono_class_is_abstract (klass)) {
  for (i = 0; i < cur_slot; ++i) {
- if (vtable [i] == NULL || (vtable [i]->flags & (METHOD_ATTRIBUTE_ABSTRACT | METHOD_ATTRIBUTE_STATIC))) {
+ if (vtable [i] == NULL || (vtable [i]->flags & METHOD_ATTRIBUTE_ABSTRACT)) {
  if (vtable [i] != NULL && mono_method_get_is_reabstracted (vtable [i]))
  continue;
  char *type_name = mono_type_get_full_name (klass);

diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c
@@ -3172,7 +3172,9 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target
  mono_class_setup_vtable (target_method->klass);
 
  // Follow the rules for constrained calls from ECMA spec
- if (!m_class_is_valuetype (constrained_class)) {
+ if (m_method_is_static (target_method)) {
+ is_virtual = FALSE;
+ } else if (!m_class_is_valuetype (constrained_class)) {
  StackInfo *sp = td->sp - 1 - csignature->param_count;
  /* managed pointer on the stack, we need to deref that puppy */
  interp_add_ins (td, MINT_LDIND_I);
@@ -3197,7 +3199,7 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target
  if (target_method)
  mono_class_init_internal (target_method->klass);
 
- if (!is_virtual && target_method && (target_method->flags & METHOD_ATTRIBUTE_ABSTRACT)) {
+ if (!is_virtual && target_method && (target_method->flags & METHOD_ATTRIBUTE_ABSTRACT) && !m_method_is_static (target_method)) {
  if (!mono_class_is_interface (method->klass))
  interp_generate_bie_throw (td);
  else
@@ -7119,6 +7121,12 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header,
  if (method->wrapper_type == MONO_WRAPPER_NONE && m->iflags & METHOD_IMPL_ATTRIBUTE_SYNCHRONIZED)
  m = mono_marshal_get_synchronized_wrapper (m);
 
+ if (constrained_class) {
+ m = mono_get_method_constrained_with_method (image, m, constrained_class, generic_context, error);
+ goto_if_nok (error, exit);
+ constrained_class = NULL;
+ }
+
  if (G_UNLIKELY (*td->ip == CEE_LDFTN &&
  m->wrapper_type == MONO_WRAPPER_NONE &&
  mono_method_has_unmanaged_callers_only_attribute (m))) {

diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c
@@ -3826,6 +3826,48 @@ mono_emit_load_got_addr (MonoCompile *cfg)
  MONO_ADD_INS (cfg->bb_exit, dummy_use);
 }
 
+static MonoMethod*
+get_constrained_method (MonoCompile *cfg, MonoImage *image, guint32 token,
+ MonoMethod *cil_method, MonoClass *constrained_class,
+ MonoGenericContext *generic_context)
+{
+ MonoMethod *cmethod = cil_method;
+ gboolean constrained_is_generic_param =
+ m_class_get_byval_arg (constrained_class)->type == MONO_TYPE_VAR ||
+ m_class_get_byval_arg (constrained_class)->type == MONO_TYPE_MVAR;
+
+ if (cfg->current_method->wrapper_type != MONO_WRAPPER_NONE) {
+ if (cfg->verbose_level > 2)
+ printf ("DM Constrained call to %s\n", mono_type_get_full_name (constrained_class));
+ if (!(constrained_is_generic_param &&
+ cfg->gshared)) {
+ cmethod = mono_get_method_constrained_with_method (image, cil_method, constrained_class, generic_context, cfg->error);
+ CHECK_CFG_ERROR;
+ }
+ } else {
+ if (cfg->verbose_level > 2)
+ printf ("Constrained call to %s\n", mono_type_get_full_name (constrained_class));
+
+ if (constrained_is_generic_param && cfg->gshared) {
+ /*
+ * This is needed since get_method_constrained can't find
+ * the method in klass representing a type var.
+ * The type var is guaranteed to be a reference type in this
+ * case.
+ */
+ if (!mini_is_gsharedvt_klass (constrained_class))
+ g_assert (!m_class_is_valuetype (cmethod->klass));
+ } else {
+ cmethod = mono_get_method_constrained_checked (image, token, constrained_class, generic_context, &cil_method, cfg->error);
+ CHECK_CFG_ERROR;
+ }
+ }
+ return cmethod;
+
+ mono_error_exit:
+ return NULL;
+}
+
 static gboolean
 method_does_not_return (MonoMethod *method)
 {
@@ -5641,7 +5683,10 @@ handle_constrained_call (MonoCompile *cfg, MonoMethod *cmethod, MonoMethodSignat
  }
  }
 
- if (constrained_partial_call) {
+ if (m_method_is_static (cmethod)) {
+ /* Call to an abstract static method */
+ return NULL;
+ } if (constrained_partial_call) {
  gboolean need_box = TRUE;
 
  /*
@@ -7128,36 +7173,12 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
  MonoMethod *cil_method; cil_method = cmethod;
 
  if (constrained_class) {
- gboolean constrained_is_generic_param =
- m_class_get_byval_arg (constrained_class)->type == MONO_TYPE_VAR ||
- m_class_get_byval_arg (constrained_class)->type == MONO_TYPE_MVAR;
-
- if (method->wrapper_type != MONO_WRAPPER_NONE) {
- if (cfg->verbose_level > 2)
- printf ("DM Constrained call to %s\n", mono_type_get_full_name (constrained_class));
- if (!(constrained_is_generic_param &&
- cfg->gshared)) {
- cmethod = mono_get_method_constrained_with_method (image, cil_method, constrained_class, generic_context, cfg->error);
- CHECK_CFG_ERROR;
- }
- } else {
- if (cfg->verbose_level > 2)
- printf ("Constrained call to %s\n", mono_type_get_full_name (constrained_class));
+ if (m_method_is_static (cil_method) && mini_class_check_context_used (cfg, constrained_class))
+ // FIXME:
+ GENERIC_SHARING_FAILURE (CEE_CALL);
 
- if (constrained_is_generic_param && cfg->gshared) {
- /* 
- * This is needed since get_method_constrained can't find 
- * the method in klass representing a type var.
- * The type var is guaranteed to be a reference type in this
- * case.
- */
- if (!mini_is_gsharedvt_klass (constrained_class))
- g_assert (!m_class_is_valuetype (cmethod->klass));
- } else {
- cmethod = mono_get_method_constrained_checked (image, token, constrained_class, generic_context, &cil_method, cfg->error);
- CHECK_CFG_ERROR;
- }
- }
+ cmethod = get_constrained_method (cfg, image, token, cil_method, constrained_class, generic_context);
+ CHECK_CFG_ERROR;
 
  if (m_class_is_enumtype (constrained_class) && !strcmp (cmethod->name, "GetHashCode")) {
  /* Use the corresponding method from the base type to avoid boxing */
@@ -7405,7 +7426,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
 
  context_used = mini_method_check_context_used (cfg, cmethod);
 
- if (context_used && mono_class_is_interface (cmethod->klass)) {
+ if (context_used && mono_class_is_interface (cmethod->klass) && !m_method_is_static (cmethod)) {
  /* Generic method interface
  calls are resolved via a
  helper function and don't
@@ -10960,6 +10981,15 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
  cmethod = mini_get_method (cfg, method, n, NULL, generic_context);
  CHECK_CFG_ERROR;
 
+ if (constrained_class) {
+ if (m_method_is_static (cmethod) && mini_class_check_context_used (cfg, constrained_class))
+ // FIXME:
+ GENERIC_SHARING_FAILURE (CEE_LDFTN);
+ cmethod = get_constrained_method (cfg, image, n, cmethod, constrained_class, generic_context);
+ constrained_class = NULL;
+ CHECK_CFG_ERROR;
+ }
+
  mono_class_init_internal (cmethod->klass);
 
  mono_save_token_info (cfg, image, n, cmethod);

diff --git a/src/tests/issues.targets b/src/tests/issues.targets
@@ -1156,7 +1156,13 @@
  <ExcludeList Include="$(XunitTestBinBase)/Loader/classloader/MethodImpl/CovariantReturns/UnitTest/CompatibleWithTest/**">
  <Issue>Doesn't pass after LLVM AOT compilation.</Issue>
  </ExcludeList>
- <ExcludeList Include="$(XunitTestBinBase)/Loader/classloader/StaticVirtualMethods/**">
+ <ExcludeList Include="$(XunitTestBinBase)/Loader/classloader/StaticVirtualMethods/GenericContext/**">
+ <Issue>Static virtual methods are not yet implemented in the Mono runtime.</Issue>
+ </ExcludeList>
+ <ExcludeList Include="$(XunitTestBinBase)/Loader/classloader/StaticVirtualMethods/InterfaceVariance/**">
+ <Issue>Static virtual methods are not yet implemented in the Mono runtime.</Issue>
+ </ExcludeList>
+ <ExcludeList Include="$(XunitTestBinBase)/Loader/classloader/StaticVirtualMethods/NegativeTestCases/**">
  <Issue>Static virtual methods are not yet implemented in the Mono runtime.</Issue>
  </ExcludeList>