[Mono-dev] review request: mini/trace.c

[Mark Mason]

Hello all,

I'd like to ask for review of the follow patch before committing it.
This patch attempts to properly handle on big endian systems the pointer
adjustments needed for extracting arguments from call slots when the
size of the argument is smaller than that of the call slot. The
MONO_TYPE_VALUETYPE case I was a bit unsure how to deal with, so I left
it alone.

Thanks in advance,
Mark

Index: trace.c
===================================================================
--- trace.c	(revision 121269)
+++ trace.c	(working copy)
@@ -305,6 +305,22 @@
 		return as;
 }
 
+/*
+ * cpos (ebp + arg_info[n].offset) points to the beginning of the
+ * stack slot for this argument.  On little-endian systems, we can
+ * simply dereference it. On big-endian systems, we need to adjust
+ * cpos upward first if the datatype we're referencing is smaller than
+ * a stack slot. Also - one can't assume that gpointer is also the
+ * size of a stack slot - use SIZEOF_REGISTER instead. The following
+ * helper macro tries to keep down the mess of all the pointer
+ * calculations.
+ */
+#if (G_BYTE_ORDER == G_LITTLE_ENDIAN)
+#define arg_in_stack_slot(cpos, type) ((type *)(cpos))
+#else
+#define arg_in_stack_slot(cpos, type) ((type *)((sizeof(type) <
SIZEOF_REGISTER) ? (((gssize)(cpos)) + SIZEOF_REGISTER - sizeof(type)) :
(gssize)(cpos)))
+#endif
+
 void
 mono_trace_enter_method (MonoMethod *method, char *ebp)
 {
@@ -343,9 +359,9 @@
 	if (mono_method_signature (method)->hasthis) {
 		gpointer *this = (gpointer *)(ebp + arg_info [0].offset);
 		if (method->klass->valuetype) {
-			printf ("value:%p, ", *this);
+			printf ("value:%p, ", *arg_in_stack_slot(this, gpointer *));
 		} else {
-			o = *((MonoObject **)this);
+			o = *arg_in_stack_slot(this, MonoObject *);
 
 			if (o) {
 				class = o->vtable->klass;
@@ -371,29 +387,29 @@
 		MonoType *type = mono_method_signature (method)->params [i];
 		
 		if (type->byref) {
-			printf ("[BYREF:%p], ", *cpos); 
+			printf ("[BYREF:%p], ", *arg_in_stack_slot(cpos, gpointer *));
 		} else switch (mono_type_get_underlying_type (type)->type) {
 			
 		case MONO_TYPE_I:
 		case MONO_TYPE_U:
-			printf ("%p, ", *((gpointer **)(cpos)));
+			printf ("%p, ", *arg_in_stack_slot(cpos, gpointer *));
 			break;
 		case MONO_TYPE_BOOLEAN:
 		case MONO_TYPE_CHAR:
 		case MONO_TYPE_I1:
 		case MONO_TYPE_U1:
-			printf ("%d, ", *((gint8 *)(cpos)));
+			printf ("%d, ", *arg_in_stack_slot(cpos, gint8));
 			break;
 		case MONO_TYPE_I2:
 		case MONO_TYPE_U2:
-			printf ("%d, ", *((gint16 *)(cpos)));
+			printf ("%d, ", *arg_in_stack_slot(cpos, gint16));
 			break;
 		case MONO_TYPE_I4:
 		case MONO_TYPE_U4:
-			printf ("%d, ", *((int *)(cpos)));
+			printf ("%d, ", *arg_in_stack_slot(cpos, int));
 			break;
 		case MONO_TYPE_STRING: {
-			MonoString *s = *((MonoString **)cpos);
+			MonoString *s = *arg_in_stack_slot(cpos, MonoString *);
 			if (s) {
 				char *as;
 
@@ -408,7 +424,7 @@
 		}
 		case MONO_TYPE_CLASS:
 		case MONO_TYPE_OBJECT: {
-			o = *((MonoObject **)cpos);
+			o = *arg_in_stack_slot(cpos, MonoObject *);
 			if (o) {
 				class = o->vtable->klass;
 		    
@@ -424,7 +440,7 @@
 				} else
 					printf ("[%s.%s:%p], ", class->name_space, class->name, o);
 			} else {
-				printf ("%p, ", *((gpointer *)(cpos)));				
+				printf ("%p, ", *arg_in_stack_slot(cpos, gpointer));
 			}
 			break;
 		}
@@ -432,17 +448,17 @@
 		case MONO_TYPE_FNPTR:
 		case MONO_TYPE_ARRAY:
 		case MONO_TYPE_SZARRAY:
-			printf ("%p, ", *((gpointer *)(cpos)));
+			printf ("%p, ", *arg_in_stack_slot(cpos, gpointer));
 			break;
 		case MONO_TYPE_I8:
 		case MONO_TYPE_U8:
-			printf ("0x%016llx, ", (long long)*((gint64 *)(cpos)));
+			printf ("0x%016llx, ", (long long)*arg_in_stack_slot(cpos, gint64));
 			break;
 		case MONO_TYPE_R4:
-			printf ("%f, ", *((float *)(cpos)));
+			printf ("%f, ", *arg_in_stack_slot(cpos, float));
 			break;
 		case MONO_TYPE_R8:
-			printf ("%f, ", *((double *)(cpos)));
+			printf ("%f, ", *arg_in_stack_slot(cpos, double));
 			break;
 		case MONO_TYPE_VALUETYPE: 
 			printf ("[");