Path: utzoo!attcan!utgpu!jarvis.csri.toronto.edu!mailrus!tut.cis.ohio-state.edu!cs.utexas.edu!uunet!mcvax!ukc!stl!stc!datlog!torch!richard
From: richard@torch.UUCP (Richard Nuttall)
Newsgroups: comp.windows.x
Subject: Re: bug in XCopyArea() of X11R3 Xsun server
Keywords: X11R3, Xsun, bw2, SUN4
Message-ID: <341@torch.UUCP>
Date: 10 May 89 14:55:08 GMT
References: <7438@spool.cs.wisc.edu>
Organization: TORCH Computers Ltd., Cambridge, England
Lines: 129

dave@romano.cs.wisc.edu (Dave Cohrs) writes:

>Environment:
>	Sun4/110
>	bw2 monitor (1 bit display)

>Description:
>	XCopyArea() in the above environment fails when copying a
>	pixmap on top of itself, when the destination is a positive
>	multiple of 32 bits *to the right* of the source.

This is a terrific bug, I can see loads of mail being generated about the 
pros and cons of the answer to this.

The problem is caused by the sparc processor.

XCopyArea eventually uses getbits and putbits. They both use SCRLEFT and 
SCRRIGHT.

These both use the >> and << operators respectively.

BUT!!!!!...... wait for it.....

On the sparc, x >> y == x >> (y % 32)  !!!! (think about it)
Thus 0xFFFFFFFF >> 32 == 0xFFFFFFFF.

This means that when the masks are generated and you need to shift by
 >= 32 bits, you don't get what you expected.

Other machines do the expected (to the people who wrote mfbbitblt.c) 
and put 0x0 in the result.

I don't particularly want to create a religious war as to whether a strictly 
out of range value such as 32 should have the results defined or not,
 (I think it should be nice and zero the word).

Here is the fix I put in. It appears only to be needed in mfbbitblt.c, so the
MFBBITBLT define is only defined in mfbbitblt.c

Unfortunately  we don't have the original source before we put all our 
speedups in, so I can't give a proper patch file.
Note that it is written so that only the sparc is affected.

File : maskbits.h

Add :
#ifdef MFBBITBLT
#ifdef sparc
#define ASR(val,shift) (((shift) > 31)? 0 : (val) >> (shift))
#else
#define ASR(val,shift) ((val) >> (shift))
#endif

#ifdef sparc
#define ASL(val,shift) (((shift) > 31)? 0 : (val) << (shift))
#else
#define ASL(val,shift) ((val) << (shift))
#endif

#else
#define ASR(val,shift) ((val) >> (shift))
#define ASL(val,shift) ((val) << (shift))
#endif

Change :
#if (BITMAP_BIT_ORDER == MSBFirst)      /* pc/rt, 680x0, sparc */
#define SCRLEFT(lw, n)  ASL((unsigned int)(lw),(n))
#define SCRRIGHT(lw, n) ASR((unsigned int)(lw),(n))
#else                                   /* vax, intel */
#define SCRLEFT(lw, n)  ((lw) >> (n))
#define SCRRIGHT(lw, n) ((lw) << (n))
#endif

Below is a little test program I wrote to demonstrate the problem. I would be
interested to know if the same problem occurs with any other processor chips.

/*
 * Test program to discover how << >> operations are handled for
 * a shift of >= 32
 *
 * written by Richard  Nuttall 10-May-1989
 *
 * Expected results:
 * On most machines: for shift of 0-31, correct shift.
 *                   for shift >= 32  , 0.
 * On sparc (SUN4) : for shift of 0-31, correct shift.
 *                   for shift >= 32  , result of shift % 32.
 */

#ifdef sparc
#define ASR(val,shift) (((shift) > 31)? 0 : val >> (shift))
#else
#define ASR(val,shift) ((val) >> (shift))
#endif

#ifdef sparc
#define ASL(val,shift) (((shift) > 31)? 0 : val << (shift))
#else
#define ASL(val,shift) ((val) << (shift))
#endif



main()
{
   unsigned int umask = 0xFFFFFFFF;
   int i;

   printf("i  ->    <<       >>       ASL      ASR\n");

   for(i = 0;i < 37;i++) /* use any number >= 32 */
   {
      unsigned int uleft  = umask << i;
      unsigned int uright = umask >> i;
      unsigned int sleft  = ASL(umask,i);
      unsigned int sright = ASR(umask,i);

      printf("%2d -> %8x %8x %8x %8x\n",i,uleft,uright,sleft,sright);
   }
}


Final note:
I guess this should really be logged as a bug in the mfbbitblt code.
 
-- 
Richard Nuttall               |    ukc!stc!datlog!torch!richard
Torch Technology Ltd.         |    0223 841000 X 309
Cambridge England             |