GNOME Bugzilla – Bug 629325
xmlXPathCompNumber() introduces rouding errors for floating point constants
Last modified: 2021-07-05 13:26:45 UTC
Created attachment 169994 [details] Patch for xmlXPathCompNumber The current xmlXPathCompNumber logic does multiple divisions, which accumulates rounding errors. xmlXPathStringEvalNumber() has similar logic but does this conversion accurately. The symptom is that number("100.15") is not 100.15, as in: <xsl:if test="number('100.15') != 100.15"> <xsl:text>not good</xsl:text> </xsl:if> The floating point values differ because of the rounding error introduce in xmlXPathCompNumber: % cat /tmp/test.xslt <?xml version="1.0" encoding="UTF-8" standalone="yes"?> <xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform" version="1.0"> <xsl:output method="xml" indent="yes"/> <xsl:template match="/"> <op-script-results> <output> <xsl:value-of select="format-number('100.15', '####.################')"/> </output> <output> <xsl:value-of select="format-number(100.15, '####.################')"/> </output> <output> <xsl:value-of select="format-number(number('100.15') , '####.################')"/> </output> </op-script-results> </xsl:template> </xsl:stylesheet> % xsltproc /tmp/test.xslt /tmp/test.xml <?xml version="1.0"?> <op-script-results> <output>100.1500000000000057</output> <output>100.1499999999999915</output> <output>100.1500000000000057</output> </op-script-results> This fix is to mimic the logic on xmlXPathStringEvalNumber().
I fear that's not sufficient. A few lines above the patched code, the multiplications by 10 and additions introduce rounding errors for 17 digits and more. An integer near a rounding boundary will probably not be rounded correctly. Please try on: 18446744073709550593.0 - 18446744073709550591.0 which should be equal to 2048 as 18446744073709550593 = (2^54-1)*1024+1 should round to (2^54)*1024 and 18446744073709550591 = (2^54-1)*1024-1 should round to (2^54-2)*1024. Here, without the patch, I get 0. But note that the patch won't change anything since there are no fractional parts. The conversion should be done right-to-left instead of left-to-right, and with more precision than double precision. Or exact computations should be done (on integers, in multiple precision). I think that some implementations use GMP. GNU MPFR can be used too (it provides correctly-rounded base conversion), but that would be an additional dependency. Ideally, the work should be done in the C library.
Created attachment 170166 [details] [review] Second patch, using strtod() This patch changes xmlXPathStringEvalNumber and xmlXPathCompNumber to use strtod(), which solves a number of floating point rounding issues. A new function xmlXPathStringIsNumber is used to detect syntax that is allowed by strtod that is not allowed by XPath. I've added strtod to configure.in as a check func, but have not done anything when the check fails. The function is >10 yrs old and is part of C99, so it will hopefully be available in the libc of all users.
Some warning: AFAIK, strtod is locale-dependent, in particular for the decimal point (also for things like "INF" in case you want to use it for that). But XPath expressions aren't. I don't know the libxml2 internals, but this may be a problem if the locale hasn't been switched back to "C" or if the string hasn't been rewritten to match the current locale. Note that the strtod function can also be buggy. This is the case of the glibc, for instance: http://sourceware.org/bugzilla/show_bug.cgi?id=3479 However I don't think such that bugs are a reason to reject strtod. It's better to have the implementation in only one place and fix it there (if there's a fix).
Okay I have applied the first patch, at least it makes the behaviour more coherent. I'm afraid I can't rely on strtod due to the locale, libxslt being a library can't set the locale and I need a behaviour which matches the spec independently of the user context (locale/OS/etc ...). I really think this need to be fixed in the library probably by designing a new parsing piece of code, so that's not FIXED but at least libxml2/libxslt is more coherent now, thanks ! Daniel
Actually I was thinking of changing the locale to "C" before the call to strtod() and changing it back to the old value after the call. There shouldn't be a noticeable performance loss, however this is rather awkward. Now I'm thinking about it, even the C99 standard doesn't require correct rounding for strtod(), so that's a bad idea to use it. BTW, if you don't want to depend on the environment, then you shouldn't use native floating-point, because the rounding direction (a.k.a. rounding mode) and rounding precision can be dynamical (in particular the rounding direction).
*** Bug 783238 has been marked as a duplicate of this bug. ***
*** Bug 709361 has been marked as a duplicate of this bug. ***
GNOME is going to shut down bugzilla.gnome.org in favor of gitlab.gnome.org. As part of that, we are mass-closing older open tickets in bugzilla.gnome.org which have not seen updates for a longer time (resources are unfortunately quite limited so not every ticket can get handled). If you can still reproduce the situation described in this ticket in a recent and supported software version, then please follow https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines and create a new ticket at https://gitlab.gnome.org/GNOME/libxml2/-/issues/ Thank you for your understanding and your help.