The following reply was made to PR pkg/33969; it has been noted by GNATS.

From: SigmFSK@aol.com
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: pkg/33969: TME sun3 emulator incorrectly emulates cmp2 and chk2 M68K inst...
Date: Sun, 1 Oct 2006 20:01:22 EDT

 Following is the patch in "diff -u" format.
 
 --- m68k-insns.c.orig 2006-10-01 08:11:18.000000000 -0400
 +++  m68k-insns.c 2006-10-01 08:19:55.000000000 -0400
 @@ -485,84 +485,145  @@
 TME_M68K_INSN(tme_m68k_cmp2_chk2)
 {
 tme_uint32_t ireg;
 -  unsigned int size_bytes, size_name,  size_ireg;
 +  unsigned int size_bytes, size_ireg;
 tme_uint32_t uvalue, ulower, uupper;
 -  tme_int32_t value, lower,  upper;
 +
 
 TME_M68K_INSN_CANFAULT;
 
 +
 /* get the register to  check and the operand size: */
 ireg = TME_M68K_IREG_D0 +  TME_FIELD_EXTRACTU(TME_M68K_INSN_SPECOP, 12, 4);
 size_bytes =  TME_FIELD_EXTRACTU(TME_M68K_INSN_OPCODE, 9, 2);
 size_ireg = 2 -  size_bytes;
 -  size_name = TME_M68K_SIZE_8 + size_bytes;
 +
 +   /* size comes back from cp2 instruction as:
 +     0 :  byte
 +     1 : word
 +     2 :  long
 +      we convert  to
 +     0 : byte
 +     1 :  word
 +     4 : long
 +  */
 size_bytes = 1 << size_bytes;
 
 +
 /* read in  the two bounds: */
 -  (*_tme_m68k_read_mem[size_name])(ic,  TME_M68K_IREG_MEMX32 << size_ireg);
 +   (*_tme_m68k_read_mem[size_bytes])(ic, TME_M68K_IREG_MEMX32 <<  size_ireg);
 +
 if (!TME_M68K_SEQUENCE_RESTARTING)  {
 ic->_tme_m68k_ea_address +=  size_bytes;
 }
 -  (*_tme_m68k_read_mem[size_name])(ic,  TME_M68K_IREG_MEMY32 << size_ireg);
 +   (*_tme_m68k_read_mem[size_bytes])(ic, TME_M68K_IREG_MEMY32 <<  size_ireg);
 
 -  /* if we have an address register, sign-extend  the bounds to 32
 -     bits: */
 +  /* if value is  an address register, sign-extend the bounds to 32  bits,
 +     then set size of bounds and value to 32 bits  (so we check entire
 +     longword value)
 +   */
 if (ireg >= TME_M68K_IREG_A0) {
 -    if  (size_name == TME_M68K_SIZE_8) {
 +    if (size_bytes ==  TME_M68K_SIZE_8) {
 ic->tme_m68k_ireg_int32(TME_M68K_IREG_MEMX32) =  
 TME_EXT_S8_S32(ic->tme_m68k_ireg_int8(TME_M68K_IREG_MEMX8));
 ic->tme_m68k_ireg_int32(TME_M68K_IREG_MEMY32) =  
 TME_EXT_S8_S32(ic->tme_m68k_ireg_int8(TME_M68K_IREG_MEMY8));
 }
 -    else if (size_name == TME_M68K_SIZE_16)  {
 +    else if (size_bytes == TME_M68K_SIZE_16)  {
 ic->tme_m68k_ireg_int32(TME_M68K_IREG_MEMX32) =  
 TME_EXT_S16_S32(ic->tme_m68k_ireg_int16(TME_M68K_IREG_MEMX16));
 ic->tme_m68k_ireg_int32(TME_M68K_IREG_MEMY32) =  
 TME_EXT_S16_S32(ic->tme_m68k_ireg_int16(TME_M68K_IREG_MEMY16));
 }
 -    size_bytes =  sizeof(tme_uint32_t);
 -    size_name =  TME_M68K_SIZE_32;
 +
 +    size_bytes =  TME_M68K_SIZE_32;
 }
 
 -  /* get the values to  check: */
 -  switch (size_name) {
 +  /* get the bounds and value  */
 +  switch (size_bytes) {
 case  TME_M68K_SIZE_8:
 -    uvalue =  ic->tme_m68k_ireg_uint8(ireg);
 ulower =  ic->tme_m68k_ireg_uint8(TME_M68K_IREG_MEMX8);
 uupper = ic->tme_m68k_ireg_uint8(TME_M68K_IREG_MEMY8);
 -     value = ic->tme_m68k_ireg_int8(ireg);
 -    lower =  ic->tme_m68k_ireg_int8(TME_M68K_IREG_MEMX8);
 -    upper =  ic->tme_m68k_ireg_int8(TME_M68K_IREG_MEMY8);
 +
 +    /*  if value is a data register, read the entire register,  extract
 +       the appropriate number of  bytes, and sign extend to our own
 +        longword size for comparison.
 +       if value  is an address register, just use the entire register as is  
 +    */
 +    if (ireg <  TME_M68K_IREG_A0) 
 +      uvalue =  (tme_uint32_t)TME_EXT_S8_S32
 +                      ((tme_int32_t)(ic->tme_m68k_ireg_uint32(ireg) &  
 0xFF));
 +    else
 +      uvalue =  ic->tme_m68k_ireg_uint32(ireg);
 +
 break;
 +
 case TME_M68K_SIZE_16:
 -    uvalue  = ic->tme_m68k_ireg_uint16(ireg);
 ulower =  ic->tme_m68k_ireg_uint16(TME_M68K_IREG_MEMX16);
 uupper =  ic->tme_m68k_ireg_uint16(TME_M68K_IREG_MEMY16);
 -    value  = ic->tme_m68k_ireg_int16(ireg);
 -    lower =  ic->tme_m68k_ireg_int16(TME_M68K_IREG_MEMX16);
 -    upper =  ic->tme_m68k_ireg_int16(TME_M68K_IREG_MEMY16);
 +
 +    /*  if value is a data register, read the entire register,  extract
 +       the appropriate number of  bytes, and sign extend to our own
 +        longword size for comparison.
 +       if value  is an address register, just use the entire register as is  
 +    */
 +    if (ireg <  TME_M68K_IREG_A0) 
 +      uvalue =  (tme_uint32_t)TME_EXT_S16_S32
 +                      ((tme_int32_t)(ic->tme_m68k_ireg_uint32(ireg) &  
 0xFFFF));
 +    else
 +      uvalue  = ic->tme_m68k_ireg_uint32(ireg);
 +
 break;
 +
 case TME_M68K_SIZE_32:
 -    uvalue  = ic->tme_m68k_ireg_uint32(ireg);
 ulower =  ic->tme_m68k_ireg_uint32(TME_M68K_IREG_MEMX32);
 uupper =  ic->tme_m68k_ireg_uint32(TME_M68K_IREG_MEMY32);
 -    value  = ic->tme_m68k_ireg_int32(ireg);
 -    lower =  ic->tme_m68k_ireg_int32(TME_M68K_IREG_MEMX32);
 -    upper =  ic->tme_m68k_ireg_int32(TME_M68K_IREG_MEMY32);
 +
 +     uvalue = ic->tme_m68k_ireg_uint32(ireg);
 +
 break;
 default: abort();
 }
 
 +   /* cmp2 / chk2 can be used for unsigned, or signed.
 +      for either type, the lower bound "should be" <= upper  bound
 +     (per m68000 family programmer's reference  manual)
 +
 +     cmp2 instruction doesn't know if  signed or unsigned
 +     if bound   250  250   then bound is the one value, check unsigned or  
 signed
 +
 +     if bound   253  255   could be unsigned 253  255
 +                                 or signed    -3  -1
 +     either way,  ok to check.
 +
 +     if bound    255   5   then only makes sense to check signed   -1  5
 +
 +     if bound   255  253   then doesn't make sense either  way!
 +                           its either 255 253 unsigned or -1 -3 signed.
 +
 +      reverse engineering the code, by running many test cases shows  that
 +     the motorola 68020 microcode does the  following.
 +
 +     Always check  unsigned.
 +     if low <= high, then out of bounds if  either < low  or > high.
 +     if high >  low,  then out of bounds if BOTH   < low AND >  high.
 +  */
 +     
 +
 /* do the  comparison.  if the value is out-of-bounds and this  is
 a chk2 instruction, trap:  */
 +
 ic->tme_m68k_ireg_ccr = (ic->tme_m68k_ireg_ccr  & TME_M68K_FLAG_X);
 +
 if (uvalue ==  ulower
 || uvalue == uupper)  {
 ic->tme_m68k_ireg_ccr |=  TME_M68K_FLAG_Z;
 +
 }
 -  else if ((ulower >  uupper)
 -    /* signed comparison: */
 -    ?  (value < lower || value > upper)
 -    /* unsigned  comparison: */
 -    : (uvalue < ulower || uvalue >  uupper)) {
 +  else if (((ulower <= uupper) && (uvalue <  ulower || uvalue > uupper))  ||
 +           ((ulower  >  uupper) && (uvalue < ulower && uvalue >  uupper))) {
 +
 ic->tme_m68k_ireg_ccr |=  TME_M68K_FLAG_C;
 -    if (TME_M68K_INSN_OPCODE &  TME_BIT(11)) {
 +
 +    /* if chk2  instruction,
 +          also  cause a CHK instruction exception (vector number 6)
 +     */
 +    if (TME_FIELD_EXTRACTU(TME_M68K_INSN_SPECOP, 11, 1))  {
 ic->tme_m68k_ireg_pc_last =  ic->tme_m68k_ireg_pc;
 ic->tme_m68k_ireg_pc =  ic->tme_m68k_ireg_pc_next;
 TME_M68K_INSN_EXCEPTION(TME_M68K_EXCEPTION_INST(TME_M68K_VECTOR_CHK));