[RFA] Avoid macro expansion side-effects w/DEBUG_LOCALS_INSN

[RFA] Avoid macro expansion side-effects w/DEBUG_LOCALS_INSN

[Keith Seitz]

[-- Attachment #1: Type: text/plain, Size: 1478 bytes --]

Hi,

The attached patch fixes all users of the DEBUG_LOCALS_INSN macro (in 
the debug interpreter) which are currently experiencing macro side 
effects in certain insns. An example is op_storea:

    op_storea:
      STOREA(GET1U ());
      NEXT_INSN;

In the DEBUG interpreter, this is expanded to:

    op_storea:
      do
        {
          DEBUG_LOCALS_INSN (GET1U (), 'o');
          locals[GET1U ()].o = (--sp)->o;
        }
      while (0);

GET1U is defined as "(pc++)->int_val" (via INTVAL macro).

Obviously, this causes really bad things to happen in the DEBUG 
interpreter with the double-increment of the PC.

I have fixed these using temporary variables in the various STORE* 
macros to hold the passed value before using it in DEBUG_LOCALS_INSN and 
the rest of the macro.

Since the code doesn't diff particularly well, I would appreciate it if 
more eyes would double-check the translation, since I could very well 
have made a typo. [Although I have double-checked it forty times...]

QCC?

Keith

ChangeLog
2007-05-08  Keith Seitz  <keiths@redhat.com>

         * interpret.cc (STOREA): Rewrite using temporary variable to
         avoid double-macro expansion side-effects.
         (STOREI): Likewise.
         (STOREF): Likewise.
         (STOREL)[SIZEOF_VOID_P == 8]: Likewise.
         (STORED)[SIZEOF_VOID_P == 8]: Likewise.
         (STOREL)[SIZEOF_VOID_P != 8]: Likewise.
         (STORED)[SIZEOF_VOID_P != 8]: Likewise.
         (POKEI): Likewise.


[-- Attachment #2: DEBUG_LOCALS_INSN-macro-side-effects.patch --]
[-- Type: text/x-patch, Size: 3682 bytes --]

Index: interpret.cc
===================================================================
--- interpret.cc	(revision 124559)
+++ interpret.cc	(working copy)
@@ -180,60 +180,81 @@
 # define LOADD(I)  LOADL(I)
 #endif
 
-#define STOREA(I)               \
-  do {                          \
-    DEBUG_LOCALS_INSN (I, 'o'); \
-    locals[I].o = (--sp)->o;    \
+#define STOREA(I)			\
+  do					\
+    {					\
+      jint __idx = (I);			\
+      DEBUG_LOCALS_INSN (__idx, 'o');	\
+      locals[__idx].o = (--sp)->o;	\
+    }					\
+  while (0)
+#define STOREI(I)		       	\
+  do					\
+    {					\
+      jint __idx = (I);			\
+      DEBUG_LOCALS_INSN (__idx, 'i');	\
+      locals[__idx].i = (--sp)->i;	\
   } while (0)
-#define STOREI(I)               \
-  do {                          \
-    DEBUG_LOCALS_INSN (I, 'i'); \
-    locals[I].i = (--sp)->i;    \
-  } while (0)
-#define STOREF(I)               \
-  do {                          \
-    DEBUG_LOCALS_INSN (I, 'f'); \
-    locals[I].f = (--sp)->f;    \
-  } while (0)
+#define STOREF(I)			\
+  do					\
+    {					\
+      jint __idx = (I);			\
+      DEBUG_LOCALS_INSN (__idx, 'f');	\
+      locals[__idx].f = (--sp)->f;	\
+    }					\
+  while (0)
 #if SIZEOF_VOID_P == 8
-# define STOREL(I)                   \
-  do {                               \
-    DEBUG_LOCALS_INSN (I, 'l');      \
-    DEBUG_LOCALS_INSN (I + 1, 'x');  \
-    (sp -= 2, locals[I].l = sp->l);  \
-  } while (0)
-# define STORED(I)                   \
-  do {                               \
-    DEBUG_LOCALS_INSN (I, 'd');      \
-    DEBUG_LOCALS_INSN (I + 1, 'x');  \
-    (sp -= 2, locals[I].d = sp->d);  \
-  } while (0)
+# define STOREL(I) \
+  do						\
+    {						\
+      jint __idx = (I);				\
+      DEBUG_LOCALS_INSN (__idx, 'l');		\
+      DEBUG_LOCALS_INSN (__idx + 1, 'x');	\
+      (sp -= 2, locals[__idx].l = sp->l);	\
+    }						\
+  while (0)
+# define STORED(I)				\
+  do						\
+    {						\
+      jint __idx = (I);				\
+      DEBUG_LOCALS_INSN (__idx, 'd');		\
+      DEBUG_LOCALS_INSN (__idx + 1, 'x');	\
+      (sp -= 2, locals[__idx].d = sp->d);	\
+    }						\
+  while (0)
 
 #else
-# define STOREL(I)                   \
-  do {                               \
-    DEBUG_LOCALS_INSN (I, 'l');      \
-    DEBUG_LOCALS_INSN (I + 1, 'x');  \
-    jint __idx = (I);                \
-    locals[__idx+1].ia[0] = (--sp)->ia[0];  \
-    locals[__idx].ia[0] = (--sp)->ia[0];    \
+# define STOREL(I)				\
+  do						\
+    {						\
+      jint __idx = (I);				\
+      DEBUG_LOCALS_INSN (__idx, 'l');		\
+      DEBUG_LOCALS_INSN (__idx + 1, 'x');	\
+      locals[__idx + 1].ia[0] = (--sp)->ia[0];	\
+      locals[__idx].ia[0] = (--sp)->ia[0];	\
+    }						\
+  while (0)
+# define STORED(I)				\
+  do {						\
+    jint __idx = (I);				\
+    DEBUG_LOCALS_INSN (__idx, 'd');		\
+    DEBUG_LOCALS_INSN (__idx + 1, 'x');		\
+    locals[__idx + 1].ia[0] = (--sp)->ia[0];	\
+    locals[__idx].ia[0] = (--sp)->ia[0];	\
   } while (0)
-# define STORED(I)                   \
-  do {                               \
-    DEBUG_LOCALS_INSN (I, 'd');      \
-    DEBUG_LOCALS_INSN (I + 1, 'x');  \
-    jint __idx = (I);                \
-    locals[__idx+1].ia[0] = (--sp)->ia[0];  \
-    locals[__idx].ia[0] = (--sp)->ia[0];    \
-  } while (0)
 #endif
 
 #define PEEKI(I)  (locals+(I))->i
 #define PEEKA(I)  (locals+(I))->o
 
-#define POKEI(I,V)				\
-  DEBUG_LOCALS_INSN(I,'i');			\
-  ((locals+(I))->i = (V))
+#define POKEI(I,V)			\
+  do					\
+    {					\
+      jint __idx = (I);			\
+      DEBUG_LOCALS_INSN (__idx, 'i');	\
+      ((locals + __idx)->i = (V));	\
+    }					\
+  while (0)
 
 
 #define BINOPI(OP) { \

Re: [RFA] Avoid macro expansion side-effects w/DEBUG_LOCALS_INSN

[Tom Tromey]

>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

Keith> The attached patch fixes all users of the DEBUG_LOCALS_INSN macro (in
Keith> the debug interpreter) which are currently experiencing macro side
Keith> effects in certain insns.

Thanks, this is ok.

Tom

Re: [RFA] Avoid macro expansion side-effects w/DEBUG_LOCALS_INSN

[Keith Seitz]

Tom Tromey wrote:
> 
> Thanks, this is ok.

Done. Thank you for taking a peek at this.

Keith