mpn_rshift / mpn_lshift bug on m68000

[Linus Nordberg]

Hi,

It seems like the mailing list archive has trouble displaying the
attachments I used for the previous three messages.

For the record, we proudly present once again the three bug reports
from Patrick Pelissier.  Drum rolls...

-- 
Linus Nordberg
List admin
Swox AB  <http://swox.com/>

---------------------------------------------------------------------
From: Patrick Pelissier <Patrick.Pelissier at loria.fr>
Subject: Re: mpn_rshift / mpn_lshift bug on m68000
Cc: gmp-bugs at swox.com
Date: Sat, 18 Oct 2003 10:56:14 +0200

Kevin Ryde wrote:
> But yes, if the value is 16-bits and it's passed with garbage in the
> upper part then we have to account for that.
> 
> I think the PalmOS tools use int==16bits, but I'm not sure we ever got
> as far as actually running on that, so I don't know if there's an
> actual problem or merely a hypothetical one.
> 
> I guess we can load just 16-bits either way, since the shift count is
> only ever 1 to 31.  
  But mc68k are big endian :)
  If it int=16, you should read
movew 10(a7),d0
  Otherwise, if int=32, and you want to read the lower part :
movew 12(a7),d0

>Feel free to propose a patch.
  Maybe a separate asm file for 16-bits mc68k ?
---------------------------------------------------------------------
From: Patrick Pelissier <Patrick.Pelissier at loria.fr>
Subject: Re: mpn_rshift / mpn_lshift bug on m68000
Cc: gmp-bugs at swox.com
Date: Mon, 20 Oct 2003 12:42:25 +0200

> I tried deliberately sticking a random value in the upper part, and
> mpn_lshift still worked fine.
  Or course, because:
  1. mc68000 is big endian!
  2. The real shift count is modulo 32: the important bits are the 5
lowest bits.

  Here is what happens:
	movew	#258,-(sp)	; Garbash due to an old call
	jsr	dummy_thing
         movew	#4,-(sp)	; Push count (int)
         movel	#16,-(sp)       ; Push size (long)
	movel	#$DEADDEA0,-(sp) ; Push src (void*)
	movel	#$ABCDABC0,-(sp) ; Push dest (void*)

Here the stack looks like:
sp->	003BF8: $AB, $CD, $AB, $C0, $DE, $AD, $DE, $A0
	003C00: $00, $00, $00, $10, $00, $04, $01, $02

Then the call:
	jsr	__gmpn_rshift
	...

__gmpn_rshift:
C Save used registers on the stack.
	moveml	d2-d6/a2, M(-,sp)
  C Copy the arguments to registers.
	movel	M(sp,28), res_ptr
	movel	M(sp,32), s_ptr
	movel	M(sp,36), s_size
	movel	M(sp,40), cnt

Here:
	res_ptr = ABCDABC0
	s_ptr = DEADDEA0
	s_size = 123
	cnt = 4*65536+258
So the real shift is (4*65536+258)%32=2, not 4 :)

Your trick works only for little endian processors (such as x86).

> We ask that bug reports include a sample program exhibiting the
> claimed bad behaviour in order to ensure that a reported problem
> really is a problem.
  Every programs which use mpn_rshift/mpn_lshift (ie every GMP programs
?) is buggy.
  I have tested myself a small MPFR/GMP program for mc68000, and after
using the assembly debugger, I have found what the problem was. After a
small fix to rshift.asm/lshift.asm, it works fine. It isn't a
hypothetical problem, but a real tested one.
  If you really want, I can put a test case, but it appears that you
can't test it on a real mc68000. Nevertheless, if you are not already 
convinced, I can send you such a program.

Sincerely,
  Patrick Pelissier
---------------------------------------------------------------------
From: Patrick Pelissier <Patrick.Pelissier at loria.fr>
Subject: Re: mpn_rshift / mpn_lshift bug on m68000
To: gmp-bugs at swox.com
Date: Tue, 21 Oct 2003 10:44:21 +0200

> I had an idea the stack is kept longword aligned.
  No. Until on 68000 (I am not sure for mcpu32, but I am sure it isn't
the case on 68000).

> If you think that's
> not the case then please post a program / compiler info / etc
> illustrating that, or more to the point illustrating the problem
> you're concerned about.
  I thought I have explained everything.

> Yes, see "reporting bugs" in the manual.  We ask for all that info so
> as to have something definite to fix or refute (as the case may be).
  I have added the smallest test case I can do. There is the .c, the
.s and the .i.
  It doesn't work on mc68000 systems.
  Compiler is gcc v3.3.1 and GNU Binutils v2.14.
  GMP 4.1.2 build with host=mc68000-gnu-linux.
  The results of rshift/lshift are incorrect.

> We have a netbsd 68040 among our test systems.
  It isn't a 68000!
Maybe try to use the -mshort -mc68000 options of gcc compiler.
See
http://gcc.gnu.org/onlinedocs/gcc-3.3.1/gcc/M680x0-Options.html#M680x0%20Options
Build GMP on netbsd 68040 with theses options, and try the test. I
hope you will see the bug.

  Sincerely,
   Patrick Pelissier