GNOME Bugzilla – Bug 144980
gnc-numeric integer overflow (multiply, others?)
Last modified: 2018-06-29 20:44:28 UTC
gnc-numeric doesn't properly multiply certain large numbers. E.g.: multiply (LCD): [1000000 / 1000000] * [10000000000000 / 1000000] = [-8446744073709 / 1000000] In particular, the problem appears to be when 2^63 <= num*num < 2^64 it wont catch the fact that the number actually DID "carry"... It's unclear if this problem exists with other gnc-numeric operations other than multiply. They should all get tested. This is a problem in both 1.8 and HEAD.
I commited some code into HEAD that I think fixes this problem. Linas, can you audit? Should we backport the 128-bit math to 1.8?
I haven't yet tried to understand why that fix works, but it seems to, it passes the multiply & divide tests. (I had to add the same fix in divide code as well). Backport the entire gnc-numeric.c file to the 1.8 branch, I think its completely compatible.
The problem is that a gnc_numeric numerator is a gint64, but the gncint128.lo is a guint64. That means we have an overflow condition when bigprod.hi > 0 __OR__ the 64th bit is set in bigprod.lo. So basically I'm testing for "number >= 2^63". We should make sure it still works when dealing with negative numbers, but I think it does because you have a separate "isneg" flag. Does this explain why the fix works?
FYI, there's still an overflow bug in gnc_numeric_div(). I just committed a new test in test-numeric that shows the problem: FAILURE expected [2578135489 / 7617691200] got <ERROR> [-2 / 0] = [782592055622866 / 89025] / [2222554708930978 / 85568] for div round test-numeric.c:76 I should add that I'm not 100% sure that the expected value is correct, but -2/0 is certainly not correct ;) This bug is causing test-query to fail as well.
FYI, the expected value could be 2578135489763407 / 7617691200000000
The division problem appears to be a true overflow, the denominator, even after reducing all common factors, still won't fit into a 64-bit int. I think the bug is now fixed in head, needs to be ported to 1.8 branch
Well, there's still the problem that other tests are failing because they generate random numbers that fail in this particular manner. That's where I got this particular test-case from -- from stepping through another test and noticing the numbers they had. Other than limiting the randomness of "random" gnc-numerics in the tests, I don't know what other way to "fix" this if this really is a division overflow.
OK, there's been a huge amount of work on gnc-numeric, I hope we've nailed all the bugs.
GnuCash bug tracking has moved to a new Bugzilla host. This bug has been copied to https://bugs.gnucash.org/show_bug.cgi?id=144980. Please update any external references or bookmarks.