Bug #93887 for Math-BigInt: Math::BigInt->new() wrongly converts large floats

Sat Mar 15 11:39:43 2014 bitcard [...] volkerschatz.com - Ticket created

Subject:

Math::BigInt->new() wrongly converts large floats

A conversion of large integral floating-point numbers into BigInts can lose significant digits. Test case: 504403158265495552.0 == 7 * 2**56, which is exactly representable as a float/double. $ perl -MMath::BigInt -e 'print Math::BigInt->new(504403158265495552.0)->bstr(), "\n";' 504403158265496000 A test case that reqires more than 64 integer bits is 7 * 2**66 = 516508834063867445248.0. The cause is that the _split function in Math::BigInt treats its argument as a string and hereby triggers Perl's automatic conversion which does not output enough digits for large integers. A fix is to explicitly convert via sprintf("%f",..), which retains all digits before the decimal point. Doing this for non-floats is superfluous and possibly harmful, so we test the accuracy of a roundtrip with the automatic conversion first: @@ -3048,6 +3048,8 @@ # invalid input. my $x = shift; + $x= sprintf("%f", $x) unless "$x" == $x; # ensure enough digits from float + # strip white space at front, also extraneous leading zeros $x =~ s/^\s*([-]?)0*([0-9])/$1$2/g; # will not strip ' .2' $x =~ s/^\s+//; # but this will Line numbers refer to version 1.9991, which my distribution ships but CPAN does not yet (?!). This issue was first reported to the perlbug list, as [perl #121136].

Wed Apr 16 10:04:37 2014 peter.john.acklam [...] gmail.com - Correspondence added

Good catch! Alas, your suggested fix isn't good enough. The sprintf("%f", ...) conversion would also be applied to input like 0.00000005044031582654955529, since it's stringified version different: it's "5.04403158265496e-08". Now, sprintf("%f", 0.00000005044031582654955529) returns "0.000000", which Math::BigInt returns as zero. However, the current behaviour is to return NaN when the input is something other than an integer. This behaviour is likely to change, so that the semantics are the same as those of core Perl, but until then, we have to use a better fix. We can't introduce inconsistent behaviour.

Wed Apr 16 10:04:37 2014 The RT System itself - Status changed from 'new' to 'open'

Wed Apr 16 16:59:23 2014 foss [...] volkerschatz.com - Correspondence added

Subject:	Re: [rt.cpan.org #93887] Math::BigInt->new() wrongly converts large floats
Date:	Wed, 16 Apr 2014 22:59:06 +0200 (CEST)
To:	Peter John Acklam via RT <bug-Math-BigInt [...] rt.cpan.org>
From:	Volker Schatz <foss [...] volkerschatz.com>

Thanks for reminding me of fractional input, I had not thought of that. Playing around with non-integers, I found two more test cases that are broken in 1.9991: 4503599627370495.5 = 2**52 - 0.5, the largest non-integer representable as a double. This needs to be sprintf'ed because autoconversion discards some digits, including the fractional digit that triggers the NaN. This and all values <1 and >0 are fixed by replacing the line inserted in my patch by: $x= sprintf("%f", $x) if "$x" != $x && $x > 1; The second, more problematic test case I dug up is 1+DBL_EPSILON = 1.00000000000000022. This is handled correctly in _split() if one raises the precision of the sprintf: $x= sprintf("%.17f", $x) if "$x" != $x && $x > 1; 17 = DBL_DECIMAL_DIG, the number of digits sufficient for a double->string->double roundtrip conversion. However, in this case split() is never reached because of a "shortcut" in new() that also applies a regex to the autoconverted value. Applying the same roundtrip conversion check as in _split() should fix this too: @@ -534,7 +534,7 @@ my $self = bless {}, $class; # shortcut for "normal" numbers - if ((!ref $wanted) && ($wanted =~ /^([+-]?)[1-9][0-9]*\z/)) + if ((!ref $wanted) && "$wanted" == $wanted && ($wanted =~ /^([+-]?)[1-9][0-9]*\z/)) { $self->{sign} = $1 || '+'; @@ -3048,6 +3048,8 @@ # invalid input. my $x = shift; + $x= sprintf("%.17f", $x) if "$x" != $x && $x > 1; # ensure enough digits from float + # strip white space at front, also extraneous leading zeros $x =~ s/^\s*([-]?)0*([0-9])/$1$2/g; # will not strip ' .2' $x =~ s/^\s+//; # but this will This is quite an effort to make for the NaN behaviour, considering it is on the way out, and has never worked for large numbers like 2**52 - 0.5. I hope you remember to revert back to the version from my last post when the time comes, so code complexity is kept down a little.