Fix setting or getting pointer fields with reflection (mono#20891)

Setting a pointer via reflection incorrectly added an extra layer of
indirection, and getting a pointer via reflection incorrectly
dereferenced the value once.

As a result, setting and then getting only via reflection worked
correctly, but doing either step by directly accessing the field and the
other via reflection was broken.

Also adjusts `FieldInfoTest` to test this, for both static and instance
fields (the set path especially is different for those.)

Fixes mono#20872

This change is released under the MIT license.

Author: spaarmann

mcs/class/corlib/Test/System.Reflection/FieldInfoTest.cs

@@ -536,25 +536,71 @@ public void GetValueOnConstantOfOpenGeneric ()
 		public static unsafe void* ip;
 
 		[Test]
-		public unsafe void GetSetValuePointers ()
+		public unsafe void GetSetValuePointersStatic ()
 		{
 			Pointer p0 = (Pointer)typeof (FieldInfoTest).GetField ("ip").GetValue (null);
 			int *p0i = (int*)Pointer.Unbox (p0);
 			Assert.AreEqual (IntPtr.Zero, new IntPtr (p0i));
 
 			int i = 5;
+
 			void *p = &i;
 			typeof (FieldInfoTest).GetField ("ip").SetValue (null, (IntPtr)p);
-			Pointer p2 = (Pointer)typeof (FieldInfoTest).GetField ("ip").GetValue (null);
 
+			Pointer p2 = (Pointer)typeof (FieldInfoTest).GetField ("ip").GetValue (null);
 			int *pi = (int*)Pointer.Unbox (p2);
 			Assert.AreEqual (5, *pi);
 
 			typeof (FieldInfoTest).GetField ("ip").SetValue (null, (UIntPtr)p);
+
 			p2 = (Pointer)typeof (FieldInfoTest).GetField ("ip").GetValue (null);
+			pi = (int*)Pointer.Unbox (p2);
+			Assert.AreEqual (5, *pi);
+
+			FieldInfoTest.ip = p;
 
+			p2 = (Pointer)typeof (FieldInfoTest).GetField ("ip").GetValue (null);
 			pi = (int*)Pointer.Unbox (p2);
 			Assert.AreEqual (5, *pi);
+
+			typeof (FieldInfoTest).GetField ("ip").SetValue (null, (IntPtr)p);
+			pi = (int*)FieldInfoTest.ip;
+			Assert.AreEqual (5, *pi);
+		}
+
+		public unsafe void* ip_inst;
+
+		[Test]
+		public unsafe void GetSetValuePointersInstance ()
+		{
+			Pointer p0 = (Pointer)typeof (FieldInfoTest).GetField ("ip_inst").GetValue (this);
+			int *p0i = (int*)Pointer.Unbox (p0);
+			Assert.AreEqual (IntPtr.Zero, new IntPtr (p0i));
+
+			int i = 5;
+
+			void *p = &i;
+			typeof (FieldInfoTest).GetField ("ip_inst").SetValue (this, (IntPtr)p);
+
+			Pointer p2 = (Pointer)typeof (FieldInfoTest).GetField ("ip_inst").GetValue (this);
+			int *pi = (int*)Pointer.Unbox (p2);
+			Assert.AreEqual (5, *pi);
+
+			typeof (FieldInfoTest).GetField ("ip_inst").SetValue (this, (UIntPtr)p);
+
+			p2 = (Pointer)typeof (FieldInfoTest).GetField ("ip_inst").GetValue (this);
+			pi = (int*)Pointer.Unbox (p2);
+			Assert.AreEqual (5, *pi);
+
+			this.ip_inst = p;
+
+			p2 = (Pointer)typeof (FieldInfoTest).GetField ("ip_inst").GetValue (this);
+			pi = (int*)Pointer.Unbox (p2);
+			Assert.AreEqual (5, *pi);
+
+			typeof (FieldInfoTest).GetField ("ip_inst").SetValue (this, (IntPtr)p);
+			pi = (int*)this.ip_inst;
+			Assert.AreEqual (5, *pi);
 		}
 
 		[Test]

mono/metadata/object.c

@@ -3415,11 +3415,7 @@ mono_field_set_value_internal (MonoObject *obj, MonoClassField *field, void *val
 		return;
 
 	dest = (char*)obj + field->offset;
-#if ENABLE_NETCORE
 	mono_copy_value (field->type, dest, value, value && field->type->type == MONO_TYPE_PTR);
-#else
-	mono_copy_value (field->type, dest, value, FALSE);
-#endif
 }
 
 /**
@@ -3462,7 +3458,7 @@ mono_field_static_set_value_internal (MonoVTable *vt, MonoClassField *field, voi
 	} else {
 		dest = (char*)mono_vtable_get_static_field_data (vt) + field->offset;
 	}
-	mono_copy_value (field->type, dest, value, FALSE);
+	mono_copy_value (field->type, dest, value, value && field->type->type == MONO_TYPE_PTR);
 }
 
 /**
@@ -3727,12 +3723,7 @@ mono_field_get_value_object_checked (MonoDomain *domain, MonoClassField *field,
 			mono_field_get_value_internal (obj, field, v);
 		}
 
-#if ENABLE_NETCORE
 		args [0] = ptr;
-#else
-		/* MONO_TYPE_PTR is passed by value to runtime_invoke () */
-		args [0] = ptr ? *ptr : NULL;
-#endif
 		args [1] = mono_type_get_object_checked (mono_domain_get (), type, error);
 		goto_if_nok (error, return_null);