[Mono-dev] RFC: For static fields with an RVA in an HMODULE, use the loaded image directly.

[Vincent Povirk]

I'm running into a problem with mixed-mode assemblies generated by
Visual Studio. The attached patch gets me past those problems.

Basically, these MS assemblies are using some static value fields not
as a container for data but as a pointer into the image. In this case,
I have a .exe that declares two pointer-sized field values: one for
the start of an array and one for the end of an array. It then gets
the address of each field (NOT the value of the fields) and uses them
in a loop like this:

while (start < end)
   do_something_with(*start++);

Because mono moves these fields into its own data structure for the
class, the memory between them is undefined, the memory actually
between them in the image is ignored, and we have a problem.

I think the only way to solve this is to use the fields directly in
the image instead of allocating new memory for them. Since this is (to
my knowledge) only useful for mixed-mode assemblies and I'm not sure
this will work properly with an image mapped by mono, I have made this
change only for images with an HMODULE (which shouldn't exist under
normal circumstances even on Windows).

Is this a change that can get into Mono? Are there potential bad effects?

Is it possible that the garbage collector will have trouble with these
static fields, since it won't know to look for them inside the
HMODULE, and is there an easy way to fix that?
-------------- next part --------------
From 81751eddbbf2c066665093e03df92d41d1f58359 Mon Sep 17 00:00:00 2001
From: Vincent Povirk <vincent at codeweavers.com>
Date: Thu, 3 May 2012 15:34:44 -0500
Subject: [PATCH] For static fields with an RVA in an HMODULE, use the loaded
 image directly.

This is needed for mixed-mode assemblies to work properly. In all other cases,
images won't be loaded from an HMODULE, so it's a no-op.
---
 mono/metadata/class.c  |    8 +++++++-
 mono/metadata/object.c |   23 +++++++++++++++++++++--
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/mono/metadata/class.c b/mono/metadata/class.c
index 71e0d72..85778ec 100644
--- a/mono/metadata/class.c
+++ b/mono/metadata/class.c
@@ -1455,7 +1455,7 @@ mono_class_setup_fields (MonoClass *class)
 					mono_class_set_failure (class, MONO_EXCEPTION_TYPE_LOAD, g_strdup_printf ("Missing field layout info for %s", field->name));
 					break;
 				}
-				if (field->offset < -1) { /*-1 is used to encode special static fields */
+				if (field->offset < -2) { /*-1 is used to encode special static fields, -2 for RVA fields */
 					mono_class_set_failure (class, MONO_EXCEPTION_TYPE_LOAD, g_strdup_printf ("Invalid negative field offset %d for %s", field->offset, field->name));
 					break;
 				}
@@ -1818,6 +1818,12 @@ mono_class_layout_fields (MonoClass *class)
 			
 		if (!(field->type->attrs & FIELD_ATTRIBUTE_STATIC) || field->type->attrs & FIELD_ATTRIBUTE_LITERAL)
 			continue;
+#ifdef HOST_WIN32
+		if ((field->type->attrs & FIELD_ATTRIBUTE_HAS_FIELD_RVA) &&
+		    field->parent->image->is_module_handle &&
+		    mono_field_get_data(field))
+		    continue;
+#endif
 		if (mono_field_is_deleted (field))
 			continue;
 
diff --git a/mono/metadata/object.c b/mono/metadata/object.c
index 4ffd33f..2fde95c 100644
--- a/mono/metadata/object.c
+++ b/mono/metadata/object.c
@@ -684,8 +684,8 @@ compute_class_bitmap (MonoClass *class, gsize *bitmap, int size, int offset, int
 			if (field->type->byref)
 				break;
 
-			if (static_fields && field->offset == -1)
-				/* special static */
+			if (static_fields && (field->offset == -1 || field->offset == -2))
+				/* special static or RVA */
 				continue;
 
 			pos = field->offset / sizeof (gpointer);
@@ -1989,6 +1989,19 @@ mono_class_create_runtime_vtable (MonoDomain *domain, MonoClass *class, gboolean
 			continue;
 		if (mono_field_is_deleted (field))
 			continue;
+#ifdef HOST_WIN32
+		if ((field->type->attrs & FIELD_ATTRIBUTE_HAS_FIELD_RVA) && field->parent->image->is_module_handle) {
+			const char *data = mono_field_get_data (field);
+
+			g_assert (!(field->type->attrs & FIELD_ATTRIBUTE_HAS_DEFAULT));
+			/* some fields don't really have rva, they are just zeroed (bss? bug #343083) */
+			if (data)
+			{
+			    field->offset = -2;
+			    continue;
+			}
+		}
+#endif
 		if (!(field->type->attrs & FIELD_ATTRIBUTE_LITERAL)) {
 			gint32 special_static = class->no_special_static_fields ? SPECIAL_STATIC_NONE : field_is_special_static (class, field);
 			if (special_static != SPECIAL_STATIC_NONE) {
@@ -3014,6 +3027,8 @@ mono_field_static_set_value (MonoVTable *vt, MonoClassField *field, void *value)
 		addr = g_hash_table_lookup (vt->domain->special_static_fields, field);
 		mono_domain_unlock (vt->domain);
 		dest = mono_get_special_static_data (GPOINTER_TO_UINT (addr));
+    } else if (field->offset == -2) {
+        dest = mono_field_get_data (field);
 	} else {
 		dest = (char*)mono_vtable_get_static_field_data (vt) + field->offset;
 	}
@@ -3049,6 +3064,8 @@ mono_field_get_addr (MonoObject *obj, MonoVTable *vt, MonoClassField *field)
 			addr = g_hash_table_lookup (vt->domain->special_static_fields, field);
 			mono_domain_unlock (vt->domain);
 			src = mono_get_special_static_data (GPOINTER_TO_UINT (addr));
+		} else if (field->offset == -2) {
+		    src = mono_field_get_data (field);
 		} else {
 			src = (guint8*)mono_vtable_get_static_field_data (vt) + field->offset;
 		}
@@ -3305,6 +3322,8 @@ mono_field_static_get_value_for_thread (MonoInternalThread *thread, MonoVTable *
 		/* Special static */
 		gpointer addr = g_hash_table_lookup (vt->domain->special_static_fields, field);
 		src = mono_get_special_static_data_for_thread (thread, GPOINTER_TO_UINT (addr));
+	} else if (field->offset == -2) {
+	    src = mono_field_get_data (field);
 	} else {
 		src = (char*)mono_vtable_get_static_field_data (vt) + field->offset;
 	}
-- 
1.7.9.5