>Number:         34690
>Category:       pkg
>Synopsis:       TME sun3 emulator incorrectly performs: movel sp,-(sp)
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    pkg-manager
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Oct 01 19:15:00 +0000 2006
>Originator:     Arthur Townsend
>Release:        3.0
>Organization:
>Environment:
3.0 for i386
>Description:
When performing the m68k instruction: movel sp,-(sp), TME (incorrectly) puts the predecremented sp value on the stack.  Using this m68k instruction seems a little silly, but it needs to work for a variety of programs to run (such as any code compiled with verdix ada that raises an ada exception, handled or not), and gcc-3.2.3 running on sunos 4.1.1 to be able to correctly preprocess the gcc-3.2.3 testcases 981001-3.c, c94-digraph-1.c, strp2.c and two dozen others.
>How-To-Repeat:
assemble the following test code with gcc-3.3.3 on netbsd 3.0.  It prints the same two numbers on a real sun3, but different numbers in TME.

#NO_APP
        .globl  _i
        .data
        .even
        .text
LC0:
        .ascii "%d\12\0"
        .even
        .globl  _main
_main:
        link a6,#0
        jbsr ___main

        movel sp,a0
        movel a0,sp@-
        pea LC0
        jbsr _printf
        addql #8,sp

        movel sp,sp@-
        pea LC0
        jbsr _printf
        addql #8,sp

        unlk a6
        rts
>Fix:
I made the following changes to m68k-insns-auto.c.
This file is automatically generated by m68k-insns-auto.sh, so these changes should somehow be put in that script.

I updated the code so that for any move instruction with the same address register in both the source and destination, with either a predecrement/postincrement in the source or a predecrement in the destination, fix instructions of the format
  move.l sp,-(sp)
ignore (as tested successfully) instructions of the type
  move.l (sp)+,(028,sp)
  move.l (028,sp),-(sp)
and generate a warning for any others.  I've been running both NetBSD and SunOS operating systems inside TME for a few days and no other instructions have been flagged.  If one pops up, it should be checked to ensure that it is being performed correctly.  The BEST fix would be to modify the code so that the move function gets the correct information without needing to add four back, but that appears to be difficult to implement.  I went for the low-risk fix.


--- m68k-insns-auto.c.orig      2006-10-01 08:37:12.000000000 -0400
+++ m68k-insns-auto.c   2006-10-01 08:42:12.000000000 -0400
@@ -2,8 +2,73 @@
 _TME_RCSID("$Id: m68k-insns-auto.sh,v 1.23 2005/03/10 13:26:23 fredette Exp $");
 
 #include "m68k-impl.h"
+#include <stdio.h>
 
 
+/* following are to fix the movel sp,-(sp) problem (of the value being pushed
+   on the stack being off by four) and to warn if there are other possible
+   problems that haven't been tested */
+
+#define PREDEC_POSTINC_CONCERN \
+\
+/* if source and destination register are the same number */ \
+   ( \
+    (TME_FIELD_EXTRACTU(TME_M68K_INSN_OPCODE,0,3) == \
+     TME_FIELD_EXTRACTU(TME_M68K_INSN_OPCODE,9,3)) &&  \
+ \
+/* and neither is a data register */ \
+    ( \
+     (TME_FIELD_EXTRACTU(TME_M68K_INSN_OPCODE,3,3) != 0) && \
+     (TME_FIELD_EXTRACTU(TME_M68K_INSN_OPCODE,6,3) != 0) \
+    ) && \
+ \
+/* and there is a predec or postinc in the src, or a predec in dest */ \
+    ( \
+     (TME_FIELD_EXTRACTU(TME_M68K_INSN_OPCODE,3,3) == 3) || \
+     (TME_FIELD_EXTRACTU(TME_M68K_INSN_OPCODE,3,3) == 4) || \
+     (TME_FIELD_EXTRACTU(TME_M68K_INSN_OPCODE,6,3) == 4) \
+    ) \
+   )
+
+
+#define DEST_IS_PREDEC_SRC_IS_REG_BOTH_SP \
+    ( \
+     (TME_FIELD_EXTRACTU(TME_M68K_INSN_OPCODE,6,3) == 4) && \
+     (TME_FIELD_EXTRACTU(TME_M68K_INSN_OPCODE,3,3) == 1) && \
+     (TME_FIELD_EXTRACTU(TME_M68K_INSN_OPCODE,0,3) == 7) && \
+     (TME_FIELD_EXTRACTU(TME_M68K_INSN_OPCODE,9,3) == 7) \
+    ) \
+
+/* following modes test ok */
+#define MODE35_OR_MODE54_BOTH_SP \
+    ( \
+     ( \
+ \
+/* src/dest mode 3,5 : this is move.l (sp)+,(028,sp) */ \
+      ( \
+       (TME_FIELD_EXTRACTU(TME_M68K_INSN_OPCODE,3,3) == 3) && \
+       (TME_FIELD_EXTRACTU(TME_M68K_INSN_OPCODE,6,3) == 5) \
+      ) || \
+ \
+/* or src/dest mode 5,4 : this is move.l (028,sp),-(sp) */ \
+      ( \
+       (TME_FIELD_EXTRACTU(TME_M68K_INSN_OPCODE,3,3) == 5) && \
+       (TME_FIELD_EXTRACTU(TME_M68K_INSN_OPCODE,6,3) == 4) \
+      )  \
+     ) && \
+     (TME_FIELD_EXTRACTU(TME_M68K_INSN_OPCODE,0,3) == 7) && \
+     (TME_FIELD_EXTRACTU(TME_M68K_INSN_OPCODE,9,3) == 7) \
+    ) \
+
+
+#define PRINT_WEIRD_SRC_DST_REG_MODE \
+printf ("src reg: %x src mode: %x dest reg: %x dest mode %x at pc:%x\n", \
+        TME_FIELD_EXTRACTU(TME_M68K_INSN_OPCODE,0,3), \
+        TME_FIELD_EXTRACTU(TME_M68K_INSN_OPCODE,3,3), \
+        TME_FIELD_EXTRACTU(TME_M68K_INSN_OPCODE,9,3), \
+        TME_FIELD_EXTRACTU(TME_M68K_INSN_OPCODE,6,3), \
+        ic->tme_m68k_ireg_pc);
+
 /* this does a 8-bit "add SRC, DST": */
 TME_M68K_INSN(tme_m68k_add8)
 {
@@ -237,6 +302,16 @@
   /* perform the operation: */
   res = op1;
 
+  if PREDEC_POSTINC_CONCERN {
+    if DEST_IS_PREDEC_SRC_IS_REG_BOTH_SP {
+      printf("ERROR: move8: how can addr/addr be 8 bits? at pc:%x\n",ic->tme_m68k_ireg_pc);
+    } else {
+      printf("WEIRD: move8");
+      PRINT_WEIRD_SRC_DST_REG_MODE
+    }
+  }
+
+
   /* store the result: */
   *((tme_uint8_t *) _op0) = res;
 
@@ -1281,6 +1356,17 @@
   /* perform the operation: */
   res = op1;
 
+  if PREDEC_POSTINC_CONCERN {
+    if DEST_IS_PREDEC_SRC_IS_REG_BOTH_SP {
+      printf("move16: chg predec dest res from:%x to %x for register %d at pc %x\n", res, res+2, TME_FIELD_EXTRACTU(TME_M68K_INSN_OPCODE,0,3), ic->tme_m68k_ireg_pc);
+      res = res + 2;
+    } else {
+      printf("WEIRD: move16");
+      PRINT_WEIRD_SRC_DST_REG_MODE
+    }
+  }
+
+
   /* store the result: */
   *((tme_uint16_t *) _op0) = res;
 
@@ -2578,6 +2664,17 @@
   /* perform the operation: */
   res = op1;
 
+  if PREDEC_POSTINC_CONCERN {
+    if DEST_IS_PREDEC_SRC_IS_REG_BOTH_SP {
+      printf("move32: chg predec dest res from:%x to %x for register %d at pc %x\n", res, res+4, TME_FIELD_EXTRACTU(TME_M68K_INSN_OPCODE,0,3), ic->tme_m68k_ireg_pc);
+      res = res + 4;
+    } else if (!(MODE35_OR_MODE54_BOTH_SP)) {
+      printf("WEIRD: move32");
+      PRINT_WEIRD_SRC_DST_REG_MODE
+    }
+  }
+
+
   /* store the result: */
   *((tme_uint32_t *) _op0) = res;