|Anonymous | Login | Signup for a new account||2019-02-22 21:06 CET|
|Main | My View | View Issues | Change Log | Roadmap|
|View Issue Details|
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0006759||OCaml||otherlibs||public||2015-01-23 15:57||2015-05-20 19:58|
|Target Version||4.02.2+dev / +rc1||Fixed in Version||4.02.2+dev / +rc1|
|Summary||0006759: big_int_of_string incorrectly parses some hexa literals.|
|Description||For instance, using OCaml 4.02.1:|
On 32 bit systems:
# string_of_big_int (big_int_of_string ("0x1000000000000000")) ;;
- : string = "0"
On 64 bit systems:
# string_of_big_int (big_int_of_string "0x1000000000000000") ;;
- : string = "1152921504606846976"
# string_of_big_int (big_int_of_string "0x100000000000000000") ;;
- : string = "0"
# string_of_big_int (big_int_of_string "0x10000000000000000000000000000000") ;;
- : string = "0"
|Attached Files|| nat__sys_nat_of_string.patch [^] (565 bytes) 2015-02-21 23:58 [Show Content]
testsuite.diff [^] (717 bytes) 2015-03-07 11:36 [Show Content]
|The bug is in `Nat.sys_nat_of_string`.|
|The error is indeed in that function: the loop uses "pint" instead of "pmax" to check how many string digits have been obtained from parsing the input string.|
I looked at this function and I cannot be convinced that the bug is as Didier describes (that said, I don't understand much of the code, which is rather painful to follow). The documentation of make_power_base says that pint is the number of characters that fit an integer, so using pint rather to control the end of an integer reference seems correct.
I just uploaded a patch on the testsuite that checks for the bug. Is anyone (Didier?) interested in proposing a concrete patch that solves the issue?
|I played around with the code before posting that note, and it seemed that using "pmax" did fix the issue. The patch therefore is trivial, but if your test case fails with it, I guess my impression was wrong.|
|I have not tested whether using "pmax" fix the issue, and I can believe you it does. But I don't understand why, and I am not sure that is the right fix (and that it fixes all cases) because it does not make much sense to me. So I don't want to apply it without an explanation of what is going on.|
edited on: 2015-03-17 15:20
I understand. I uploaded the patch for your convenience. I don't quite remember exactly the reason the original code did not work as intended: I think the value used to check overflow is too optimistic as to how many digits can be parsed before filling the temporary variable up; in the case of hexadecimal strings, it falls exactly on the maximum value allowed by this temporary, so the temporary overflows to zero. I agree with you that this algorithm is a quite complex. Using less digits does work, but I cannot say if using pmax is optimal (ie, if using more than pmax will always overflow).
(I remember now that I used Octave to check the results of the function when I did my tests, but I could only check the most significant digits, so this bug definitely deserves more investigation).
|The source of the problem is that `make_power_base` is seriously broken.|
Fixed in 4.02 branch (rev 15972).
|2015-01-23 15:57||strub||New Issue|
|2015-02-19 20:24||doligez||Summary||string_of_big_int incorrectly parses some hexa literals. => big_int_of_string incorrectly parses some hexa literals.|
|2015-02-19 20:38||doligez||Note Added: 0013319|
|2015-02-19 20:38||doligez||Status||new => confirmed|
|2015-02-19 20:38||doligez||Target Version||=> 4.02.2+dev / +rc1|
|2015-02-21 23:57||dcassirame||Note Added: 0013327|
|2015-02-21 23:58||dcassirame||File Added: nat__sys_nat_of_string.patch|
|2015-03-07 11:36||gasche||File Added: testsuite.diff|
|2015-03-07 11:37||gasche||Note Added: 0013403|
|2015-03-12 15:24||dcassirame||Note Added: 0013466|
|2015-03-12 15:40||gasche||Note Added: 0013467|
|2015-03-17 15:13||dcassirame||Note Added: 0013507|
|2015-03-17 15:20||dcassirame||Note Edited: 0013507||View Revisions|
|2015-03-27 19:52||doligez||Tag Attached: patch|
|2015-03-27 20:56||doligez||Note Added: 0013582|
|2015-03-27 22:24||doligez||Note Added: 0013583|
|2015-03-27 22:24||doligez||Status||confirmed => closed|
|2015-03-27 22:24||doligez||Resolution||open => fixed|
|2015-03-27 22:24||doligez||Fixed in Version||=> 4.02.2+dev / +rc1|
|2015-05-20 19:58||doligez||Relationship added||has duplicate 0006875|
|2017-02-23 16:42||doligez||Category||OCaml otherlibs => otherlibs|
|Copyright © 2000 - 2011 MantisBT Group|