Re: New failures related to recent developments

2012-02-28 Thread Niels Möller
Torbjorn Granlund t...@gmplib.org writes:

 I compared mpn/fib_table.c with a system that did not report any
 failures (but this table to was generated with mini-gmp.c).  They have
 the same contents.

I compared the output of gen-fib table 32 0, in current gmp and
gmp-5.0.2. Result is identical on my machine.

 But fib-table.h has

 #define FIB_TABLE_LIMIT 47
 #define FIB_TABLE_LUCNUM_LIMIT  47

 on one of the failing systems and

 #define FIB_TABLE_LIMIT 47
 #define FIB_TABLE_LUCNUM_LIMIT  46

I also get 

#define FIB_TABLE_LUCNUM_LIMIT  46

for both current and gmp-5.0.2. So I think we can conclude that's the
correct definition.

I'm not entirely sure I understand fib-gen is supposed work. luc_limit
is only assigned like this, in gen-fib.c:generate,

  if (mpz_cmp (l, limit)  0)
luc_limit = i-1;
  
Looking at mini-gmp.c:mpz_cmp, I've spotted one bug, but I think
that's unrelated since it affects negative numbers only.

diff -r e21157bb513d mini-gmp/mini-gmp.c
--- a/mini-gmp/mini-gmp.c   Mon Feb 27 14:37:02 2012 +0100
+++ b/mini-gmp/mini-gmp.c   Tue Feb 28 09:26:59 2012 +0100
@@ -1694,7 +1694,7 @@ mpz_cmp (const mpz_t a, const mpz_t b)
   else if (asize  0)
 return mpn_cmp (a-_mp_d, b-_mp_d, asize);
   else if (asize  0)
-return -mpn_cmp (a-_mp_d, b-_mp_d, asize);
+return -mpn_cmp (a-_mp_d, b-_mp_d, -asize);
   else
 return 0;
 }

Regards,
/Niels

-- 
Niels Möller. PGP-encrypted email is preferred. Keyid C0B98E26.
Internet email is subject to wholesale government surveillance.
___
gmp-devel mailing list
gmp-devel@gmplib.org
http://gmplib.org/mailman/listinfo/gmp-devel


Re: New failures related to recent developments

2012-02-28 Thread Torbjorn Granlund
ni...@lysator.liu.se (Niels Möller) writes:

  I'm not entirely sure I understand fib-gen is supposed work. luc_limit
  is only assigned like this, in gen-fib.c:generate,
  
if (mpz_cmp (l, limit)  0)
luc_limit = i-1;

  Looking at mini-gmp.c:mpz_cmp, I've spotted one bug, but I think
  that's unrelated since it affects negative numbers only.
  
  diff -r e21157bb513d mini-gmp/mini-gmp.c
  --- a/mini-gmp/mini-gmp.c Mon Feb 27 14:37:02 2012 +0100
  +++ b/mini-gmp/mini-gmp.c Tue Feb 28 09:26:59 2012 +0100
  @@ -1694,7 +1694,7 @@ mpz_cmp (const mpz_t a, const mpz_t b)
 else if (asize  0)
   return mpn_cmp (a-_mp_d, b-_mp_d, asize);
 else if (asize  0)
  -return -mpn_cmp (a-_mp_d, b-_mp_d, asize);
  +return -mpn_cmp (a-_mp_d, b-_mp_d, -asize);
 else
   return 0;
   }
  
Since this happens on just a few systems, I don't think it is a simple
logical bug.

I would guess it is a dependency on uninitialised data.

The failing systems seem to have no working debugging environment (which
I understand).

-- 
Torbjörn
___
gmp-devel mailing list
gmp-devel@gmplib.org
http://gmplib.org/mailman/listinfo/gmp-devel


Re: New failures related to recent developments

2012-02-28 Thread Niels Möller
Torbjorn Granlund t...@gmplib.org writes:

 The failing systems seem to have no working debugging environment (which
 I understand).

Printing out the value of limit (ought to be two limbs: 0, 1) and inputs
and output of the mpz_cmp call would be helpful, I think. Then we'll see
which operation goes wrong.

/Niels

-- 
Niels Möller. PGP-encrypted email is preferred. Keyid C0B98E26.
Internet email is subject to wholesale government surveillance.
___
gmp-devel mailing list
gmp-devel@gmplib.org
http://gmplib.org/mailman/listinfo/gmp-devel


Re: New failures related to recent developments

2012-02-28 Thread Torbjorn Granlund
There is a systematic problem in mini-gmp.c when MPZ_REALLOC is called
when a destination variable is the same as some other (source or
destination) variable.

After MPZ_REALLOC, all cached pointers must be considered to be defunct.

I've spotted this error in 4 functions, but I haven't made a proper code
review.

-- 
Torbjörn
___
gmp-devel mailing list
gmp-devel@gmplib.org
http://gmplib.org/mailman/listinfo/gmp-devel


Re: New failures related to recent developments

2012-02-28 Thread Niels Möller
Torbjorn Granlund t...@gmplib.org writes:

 After MPZ_REALLOC, all cached pointers must be considered to be defunct.

It seems I was only paying attention to the destination pointer. I'll
go through all uses of MPZ_REALLOC.

Regards,
/Niels

-- 
Niels Möller. PGP-encrypted email is preferred. Keyid C0B98E26.
Internet email is subject to wholesale government surveillance.
___
gmp-devel mailing list
gmp-devel@gmplib.org
http://gmplib.org/mailman/listinfo/gmp-devel


Re: New failures related to recent developments

2012-02-28 Thread Torbjorn Granlund
ni...@lysator.liu.se (Niels Möller) writes:

  Torbjorn Granlund t...@gmplib.org writes:
  
   After MPZ_REALLOC, all cached pointers must be considered to be defunct.
  
  It seems I was only paying attention to the destination pointer. I'll
  go through all uses of MPZ_REALLOC.
  
This is a (partial?) patch.  It seems to fix the present problem.



diff
Description: Binary data

-- 
Torbjörn
___
gmp-devel mailing list
gmp-devel@gmplib.org
http://gmplib.org/mailman/listinfo/gmp-devel


Re: New failures related to recent developments

2012-02-28 Thread Torbjorn Granlund
ni...@lysator.liu.se (Niels Möller) writes:

  Will you commit these fixes, or do you want me to do that?
  
Please take care of any fixing.  It's your baby after all.  :-)

  I have found the same four direct MPZ_REALLOC problems when reviewing
  the code: mpz_abs_add, mpz_and, mpz_ior and mpz_xor. Then I have loooked
  for functions which use cached pointers over a call to a function using
  MPZ_REALLOC. But I didn't find any problems of that type.
  
These other usages all seem to be safe, I finished a review too, since
four eyes see more than two.

(I think the mini-gmp test suite should have caught these errors.  I'd
suggest that you enhance it, and make sure these types of errors are
actually detected.)

-- 
Torbjörn
___
gmp-devel mailing list
gmp-devel@gmplib.org
http://gmplib.org/mailman/listinfo/gmp-devel