New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
big_int_of_string incorrectly parses some hexa literals. #6759
Comments
Comment author: @damiendoligez The bug is in |
Comment author: dcassirame 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. |
Comment author: @gasche 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? |
Comment author: dcassirame 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. |
Comment author: @gasche 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. |
Comment author: dcassirame 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). |
Comment author: @damiendoligez The source of the problem is that |
Comment author: @damiendoligez Fixed in 4.02 branch (rev 15972). |
Original bug ID: 6759
Reporter: strub
Status: closed (set by @damiendoligez on 2015-03-27T21:24:25Z)
Resolution: fixed
Priority: normal
Severity: major
Version: 4.02.1
Target version: 4.02.2+dev / +rc1
Fixed in version: 4.02.2+dev / +rc1
Category: otherlibs
Tags: patch
Has duplicate: #6875
Monitored by: @gasche @hcarty @yakobowski
Bug description
For instance, using OCaml 4.02.1:
On 32 bit systems:
string_of_big_int (big_int_of_string ("0x1000000000000000")) ;;
On 64 bit systems:
string_of_big_int (big_int_of_string "0x1000000000000000") ;;
string_of_big_int (big_int_of_string "0x100000000000000000") ;;
string_of_big_int (big_int_of_string "0x10000000000000000000000000000000") ;;
File attachments
The text was updated successfully, but these errors were encountered: