Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0006759OCamlotherlibspublic2015-01-23 15:572015-05-20 19:58
Reporterstrub 
Assigned To 
PrioritynormalSeveritymajorReproducibilityalways
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version4.02.1 
Target Version4.02.2+dev / +rc1Fixed in Version4.02.2+dev / +rc1 
Summary0006759: big_int_of_string incorrectly parses some hexa literals.
DescriptionFor 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"
Tagspatch
Attached Filespatch file icon nat__sys_nat_of_string.patch [^] (565 bytes) 2015-02-21 23:58 [Show Content]
diff file icon testsuite.diff [^] (717 bytes) 2015-03-07 11:36 [Show Content]

- Relationships
has duplicate 0006875closed big_int_of_string: incorrect result from hex string 

-  Notes
(0013319)
doligez (administrator)
2015-02-19 20:38

The bug is in `Nat.sys_nat_of_string`.
(0013327)
dcassirame (reporter)
2015-02-21 23:57

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.
(0013403)
gasche (administrator)
2015-03-07 11:37

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?
(0013466)
dcassirame (reporter)
2015-03-12 15:24

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.
(0013467)
gasche (administrator)
2015-03-12 15:40

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.
(0013507)
dcassirame (reporter)
2015-03-17 15:13
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).

(0013582)
doligez (administrator)
2015-03-27 20:56

The source of the problem is that `make_power_base` is seriously broken.
(0013583)
doligez (administrator)
2015-03-27 22:24

Fixed in 4.02 branch (rev 15972).

- Issue History
Date Modified Username Field Change
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
Powered by Mantis Bugtracker