Bug #92564 for Math-GMP: 0+ overload converts to signed

Wed Jan 29 14:40:51 2014 DANAJ [...] cpan.org - Ticket created

Subject:

0+ overload converts to signed

When a Math::GMP object is passed to an XS module, SvUV gets sucked into intify() by the 0+ overload, which makes incorrect values. E.g. 9228015158111050049 turns into 4643121256274241. Interestingly, removing the 0+ overload entirely fixes the problem and the tests still pass. I note that Math::GMPz does not use a 0+ overload. However, almost all the tests in the test suite are on tiny numbers, so I don't think these paths are getting exercised at all. I also would consider making uintify and intify use UV / IV, so users from Perl don't have to work around sizeof(long) != sizeof(IV).

Wed Jul 29 07:28:33 2015 SHLOMIF [...] cpan.org - Correspondence added

Hi DANAJ, On Wed Jan 29 14:40:51 2014, DANAJ wrote: Show quoted text

> When a Math::GMP object is passed to an XS module, SvUV gets sucked > into intify() by the 0+ overload, which makes incorrect values. E.g. > 9228015158111050049 turns into 4643121256274241. >

can you provide a testcase and/or a patch for all that? Regards, -- Shlomi Fish Show quoted text

> Interestingly, removing the 0+ overload entirely fixes the problem and > the tests still pass. I note that Math::GMPz does not use a 0+ > overload. However, almost all the tests in the test suite are on tiny > numbers, so I don't think these paths are getting exercised at all. > > I also would consider making uintify and intify use UV / IV, so users > from Perl don't have to work around sizeof(long) != sizeof(IV).

Wed Jul 29 07:28:34 2015 The RT System itself - Status changed from 'new' to 'open'

Wed Jul 29 07:28:37 2015 SHLOMIF [...] cpan.org - Status changed from 'open' to 'stalled'

Thu Dec 31 09:23:17 2015 hv [...] crypt.org - Correspondence added

From:

hv [...] crypt.org

On Wed Jul 29 07:28:33 2015, SHLOMIF wrote: Show quoted text

> can you provide a testcase and/or a patch for all that?

Here's a testcase that exercises it using int(), when running on a 64-bit perl-5.18.4 with Math-GMP-2.11: % perl -MMath::GMP -wle '$a = 12874555777890293787; $b = Math::GMP->new($a); $c = int($a); $d = int($b); print for ($a, $b, $c, $d)' 12874555777890293787 12874555777890293787 12874555777890293787 3651183741035517979 % Hugo

Thu Dec 31 09:23:17 2015 The RT System itself - Status changed from 'stalled' to 'open'

Thu Dec 31 09:33:54 2015 hv [...] crypt.org - Correspondence added

From:

hv [...] crypt.org

On Thu Dec 31 09:23:17 2015, hv@crypt.org wrote: Show quoted text

> On Wed Jul 29 07:28:33 2015, SHLOMIF wrote:

> > can you provide a testcase and/or a patch for all that?

> > Here's a testcase that exercises it using int(), when running on a 64- > bit perl-5.18.4 with Math-GMP-2.11: > % perl -MMath::GMP -wle '$a = 12874555777890293787; $b = Math::GMP-

> >new($a); $c = int($a); $d = int($b); print for ($a, $b, $c, $d)'

> 12874555777890293787 > 12874555777890293787 > 12874555777890293787 > 3651183741035517979 > %

Oh sorry, this must go through a different pathway since it isn't fixed by the removing the 0+ overload; the case I reported earlier against Math::Prime::Util (https://rt.cpan.org/Public/Bug/Display.html?id=110813) is fixed by the patch below though. Before: % /path/to/perl testprog source 12874555777890293787 (3 ^ 1 . 4243 ^ 2 . 488239 ^ 2) dest 3651183741035517979 (19 ^ 4 . 28016848712299 ^ 1) source hex b2ab9c44861a981b dest hex 32ab9c44861a981b % After: % /path/to/perl testprog source 12874555777890293787 (3 ^ 1 . 4243 ^ 2 . 488239 ^ 2) dest 12874555777890293787 (3 ^ 1 . 4243 ^ 2 . 488239 ^ 2) source hex b2ab9c44861a981b dest hex b2ab9c44861a981b % The difference between them: --- ./lib/Math/GMP.pm.old 2015-08-16 19:12:39.000000000 +0100 +++ ./lib/Math/GMP.pm 2015-12-31 14:30:29.607982997 +0000 @@ -34,7 +34,8 @@ use overload ( '""' => sub { stringify($_[0]) }, - '0+' => sub { intify($_[0]) }, +# don't do this: it isn't needed, and truncates large UV to IV +# '0+' => sub { intify($_[0]) }, 'bool' => sub { $_[0] != 0 }, '<=>' => \&op_spaceship, Hugo

Fri Jan 01 19:06:33 2016 DANAJ [...] cpan.org - Correspondence added

I thought the original report was apparent for this part, but examples and patch: Without patch we get the wrong answer: $ perl -Iblib/lib -Iblib/arch -MMath::GMP -E 'use Math::GMP qw/:constant/; say int(9228015158111050049)' 4643121256274241 With patch: $ perl -Iblib/lib -Iblib/arch -MMath::GMP -E 'use Math::GMP qw/:constant/; say int(9228015158111050049)' 9228015158111050049 --- old.pm 2016-01-01 15:34:42.000000000 -0800 +++ new.pm 2016-01-01 15:35:00.000000000 -0800 @@ -34,7 +34,6 @@ use overload ( '""' => sub { stringify($_[0]) }, - '0+' => sub { intify($_[0]) }, 'bool' => sub { $_[0] != 0 }, '<=>' => \&op_spaceship, Hugo's is identical but with comments. The issue with unsigned long and UV not matching doesn't show up with overloaded ops since it converts things to mpz types by string conversion, but all the functions that use _ui calls have this issue. I think that should be a separate ticket. On a machine with 64-bit longs we get what we expect: $ perl -Iblib/lib -Iblib/arch -MMath::GMP -E 'my $a = Math::GMP->new(2)**48-1; say $a; say $a->uintify(); say 2**48-1;' 281474976710655 281474976710655 281474976710655 On a machine with 32-bit longs with 64-bit perl we see uintify mess things up: $ perl -Iblib/lib -Iblib/arch -MMath::GMP -E 'my $a = Math::GMP->new(2)**48-1; say $a; say $a->uintify(); say 2**48-1;' 281474976710655 4294967295 281474976710655 Here is an example that fixes the issue, but this is just a start. (1) more functions, (2) test on many systems. --- old.xs 2016-01-01 16:04:04.000000000 -0800 +++ new.xs 2016-01-01 16:03:55.000000000 -0800 @@ -173,7 +173,7 @@ mpz_t * n CODE: - RETVAL = mpz_get_ui(*n); + mpz_export(&(RETVAL), 0, -1, sizeof(RETVAL), 0, 0, *n); OUTPUT: RETVAL

Mon Jan 04 13:58:09 2016 SHLOMIF [...] cpan.org - Correspondence added

Hi DANAJ, thanks for your message and sorry for the late reply. On Fri Jan 01 19:06:33 2016, DANAJ wrote: Show quoted text

> I thought the original report was apparent for this part, but examples > and patch: >

Thanks for those, but it's still very hard for me to work like that. Can you send me a pull-request for the dist's repository on GitHub - https://github.com/turnstep/Math-GMP with: 1. New t/*.t test scripts that fail if the patch is not applied. 2. A Patch to the code. You can also send me a "git diff" or "git format-patch" diff. Otherwise , I am getting fragments of "tests" on the command line and patches to non-existent files. Also see: * http://www.shlomifish.org/open-source/resources/how-to-contribute-to-my-projects/ Regards, -- Shlomi Fish

Mon Jan 04 14:32:58 2016 DANAJ [...] cpan.org - Correspondence added

Thanks for the reply. github should be easier and more clear. I'm traveling this week but will get to it asap.

Mon Jan 04 14:43:29 2016 SHLOMIF [...] cpan.org - Correspondence added

On Mon Jan 04 14:32:58 2016, DANAJ wrote: Show quoted text

> Thanks for the reply. github should be easier and more clear. I'm > traveling this week but will get to it asap.

Excellent! Looking forward to it. Thanks for the quick confirmation.

Tue Apr 19 12:03:13 2016 SHLOMIF [...] cpan.org - Correspondence added

On Mon Jan 04 14:32:58 2016, DANAJ wrote: Show quoted text

> Thanks for the reply. github should be easier and more clear. I'm > traveling this week but will get to it asap.

Hi DANAJ! Any news? It's been over 3 months. Regards, -- SHLOMIF