| CC: | slaven [...] rezic.de |
| Subject: | brootrem test failure in 2.12 |
As shown by test failures in http://www.cpantesters.org/cpan/report/3c9262e4-a6b0-11e6-a597-88fd411d5d86, versions of libgmp before 5.1 have a bug in mpz_rootrem when dealing with negative numbers.
Attached 3 proposed patches:
0001-brootrem-work-around-bug-in-older-versions-of-libgmp.patch
- Handle negative numbers explicitly if running against an older libgmp. This involves a check every time, but I think the checking is fast enough compared to the cost of the function itself. (I originally wrote it as a compile-time version check, but it would have caused problems if libgmp got downgraded after build.)
0002-Add-_gmp_build_version-and-_gmp_lib_version.patch
- These feel like useful functions to have. Even better would be to report them automatically in smoke reports, but I'm not sure how to do that. I'm also not sure if a vstring is the ideal way to expose them, or if documenting them is useful.
0003-additional-test-for-root-of-negative.patch
- As far as I can see the libgmp bug affected only mpz_rootrem, not mpz_root. But we should have a test for the case anyway.
Hugo
| Subject: | 0001-brootrem-work-around-bug-in-older-versions-of-libgmp.patch |
From a56c5297a684820a70821a7e777e166f5ae2f5d1 Mon Sep 17 00:00:00 2001
From: Hugo van der Sanden <hv@crypt.org>
Date: Thu, 17 Nov 2016 15:01:31 +0000
Subject: [PATCH 1/3] brootrem: work around bug in older versions of libgmp
The intent is that ($root, $remainder) = $x->brootrem($y) should return
$root = trunc($x ** (1/$y)), but versions before libgmp-5.1 returned floor()
instead of trunc() (ie rounded towards -inf rather than towards 0). Add code
to work around this where needed.
---
GMP.xs | 34 +++++++++++++++++++++++++++++++++-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/GMP.xs b/GMP.xs
index 83ddea8..a79d55d 100644
--- a/GMP.xs
+++ b/GMP.xs
@@ -32,6 +32,28 @@ Durham, NC 27713
#define SWAP_GMP if (swap) { mpz_t* t = m; m = n; n = t; }
+/*
+ * mpz_rootrem() has bug with negative first argument before 5.1.0
+ */
+static int need_rootrem_workaround(mpz_t* m, unsigned long n) {
+ /* workaround only valid for n odd (n even should be an error) */
+ if ((n & 1) == 0)
+ return 0;
+
+ /* workaround only relevant for m negative */
+ if (mpz_sgn(*m) >= 0)
+ return 0;
+
+ /* workaround only needed for gmp_version < 5.1.0 */
+ if ((gmp_version[0] && gmp_version[1] != '.') /* >= 10.0.0 */
+ || (gmp_version[0] > '5') /* >= 6.0.0 */
+ || (gmp_version[0] == '5' && gmp_version[2] != '0') /* >= 5.1.0 */
+ )
+ return 0;
+
+ return 1;
+}
+
static int
not_here(char *s)
{
@@ -602,7 +624,17 @@ brootrem(m,n)
remainder = malloc (sizeof(mpz_t));
mpz_init(*root);
mpz_init(*remainder);
- mpz_rootrem(*root, *remainder, *m, n);
+ if (need_rootrem_workaround()) {
+ /* Older libgmp have bugs for negative m, but if we need to we can
+ * work on abs(m) then negate the results.
+ */
+ mpz_neg(*root, *m);
+ mpz_rootrem(*root, *remainder, *root, n);
+ mpz_neg(*root, *root);
+ mpz_neg(*remainder, *remainder);
+ } else {
+ mpz_rootrem(*root, *remainder, *m, n);
+ }
EXTEND(SP, 2);
PUSHs(sv_setref_pv(sv_newmortal(), "Math::GMP", (void*)root));
PUSHs(sv_setref_pv(sv_newmortal(), "Math::GMP", (void*)remainder));
--
2.10.2
| Subject: | 0002-Add-_gmp_build_version-and-_gmp_lib_version.patch |
From 2fdfe94625a9223ae9917e4c72618c3d68512ec5 Mon Sep 17 00:00:00 2001
From: Hugo van der Sanden <hv@crypt.org>
Date: Thu, 17 Nov 2016 15:05:43 +0000
Subject: [PATCH 2/3] Add _gmp_build_version() and _gmp_lib_version()
This exposes the version of libgmp against which the module has been
built, and the version against which it is now running.
---
GMP.xs | 20 ++++++++++++++++++++
lib/Math/GMP.pm | 20 ++++++++++++++++++++
2 files changed, 40 insertions(+)
diff --git a/GMP.xs b/GMP.xs
index a79d55d..daf0c31 100644
--- a/GMP.xs
+++ b/GMP.xs
@@ -141,6 +141,26 @@ destroy(n)
free(n);
SV *
+_gmp_build_version()
+ CODE:
+ char buf[] = STRINGIFY(__GNU_MP_VERSION)
+ "." STRINGIFY(__GNU_MP_VERSION_MINOR)
+ "." STRINGIFY(__GNU_MP_VERSION_PATCHLEVEL);
+ RETVAL = newSV(6);
+ scan_vstring(buf, buf + sizeof(buf), RETVAL);
+ OUTPUT:
+ RETVAL
+
+SV *
+_gmp_lib_version()
+ CODE:
+ const char* v = gmp_version;
+ RETVAL = newSV(6);
+ scan_vstring(v, v + strlen(v), RETVAL);
+ OUTPUT:
+ RETVAL
+
+SV *
stringify(n)
mpz_t * n
diff --git a/lib/Math/GMP.pm b/lib/Math/GMP.pm
index e9e7cc6..7da533b 100644
--- a/lib/Math/GMP.pm
+++ b/lib/Math/GMP.pm
@@ -452,6 +452,26 @@ Returns the size of $x in base $plain_int_base .
Returns the value of the object as an unblessed (and limited-in-precision)
integer.
+=head2 _gmp_build_version()
+
+ my $gmp_version = Math::GMP::_gmp_build_version;
+ if ($gmp_version ge 6.0.0) {
+ print "Math::GMP was built against libgmp-6.0.0 or later";
+ }
+
+Class method that returns as a vstring the version of libgmp against which
+this module was built.
+
+=head2 _gmp_lib_version()
+
+ my $gmp_version = Math::GMP::_gmp_lib_version;
+ if ($gmp_version ge 6.0.0) {
+ print "Math::GMP is now running with libgmp-6.0.0 or later";
+ }
+
+Class method that returns as a vstring the version of libgmp it is currently
+running.
+
=head2 gcd()
An alias to bgcd() .
--
2.10.2
| Subject: | 0003-additional-test-for-root-of-negative.patch |
From fabe2c8709a7deb169ce8bc4b9d2311690747e3e Mon Sep 17 00:00:00 2001
From: Hugo van der Sanden <hv@crypt.org>
Date: Thu, 17 Nov 2016 15:57:59 +0000
Subject: [PATCH 3/3] additional test for root of negative
Verify that broot() doesn't have the same problem with negative numbers
that brootrem() had.
---
t/01_gmppm.t | 1 +
1 file changed, 1 insertion(+)
diff --git a/t/01_gmppm.t b/t/01_gmppm.t
index 09d7e24..5f5113e 100644
--- a/t/01_gmppm.t
+++ b/t/01_gmppm.t
@@ -628,6 +628,7 @@ babcdefgh,36:808334348993
16:i4:2
999:i3:9
-1:i3:-1
+-2:i3:-1
&rootrem
16:i2:L4,0
999:i3:L9,270
--
2.10.2