Skip Menu |
 

This queue is for tickets about the Math-GMP CPAN distribution.

Report information
The Basics
Id: 92564
Status: open
Priority: 0/
Queue: Math-GMP

People
Owner: Nobody in particular
Requestors: DANAJ [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: Normal
Broken in: 2.07
Fixed in: (no value)



Subject: 0+ overload converts to signed
Download (untitled) / with headers
text/plain 596b
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).
Download (untitled) / with headers
text/plain 758b
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).
From: hv [...] crypt.org
Download (untitled) / with headers
text/plain 433b
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
From: hv [...] crypt.org
Download (untitled) / with headers
text/plain 1.5k
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
Download (untitled) / with headers
text/plain 1.7k
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
Download (untitled) / with headers
text/plain 757b
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
Download (untitled) / with headers
text/plain 112b
Thanks for the reply. github should be easier and more clear. I'm traveling this week but will get to it asap.
Download (untitled) / with headers
text/plain 228b
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.
Download (untitled) / with headers
text/plain 228b
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


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.