diff options
author | Stephan Bergmann <sbergman@redhat.com> | 2021-09-16 11:23:03 +0200 |
---|---|---|
committer | Stephan Bergmann <sbergman@redhat.com> | 2021-09-16 13:00:40 +0200 |
commit | 1d4adcee1ba82cb8f02ac73be3e558d10dea8eda (patch) | |
tree | 7982b02888848e4cd368174f897f8a980222d465 /starmath | |
parent | ba5156abace2e41ec4d21397c0875f7e1efd2beb (diff) |
Increase accuracy of ParseMathMLUnsignedNumber Fraction result
The new test from d5e55d204b71710eb5eb5d2c683dd6698626df3c "tdf#144319:
sw_odfexport: Add unittest" started to cause UBSan failure during
CppunitTest_sw_odfexport (<https://ci.libreoffice.org/job/lo_ubsan/2136/>),
> /workdir/UnpackedTarball/boost/boost/rational.hpp:605:22: runtime error: signed integer overflow: 1073741824 * 2 cannot be represented in type 'int'
> #0 0x2b450983594f in boost::rational<int>::operator/=(boost::rational<int> const&) /workdir/UnpackedTarball/boost/boost/rational.hpp:605:22
> #1 0x2b450982e216 in Fraction::operator/=(Fraction const&) /tools/source/generic/fract.cxx:224:7
> #2 0x2b45098312d3 in operator/(Fraction const&, Fraction const&) /tools/source/generic/fract.cxx:320:10
> #3 0x2b46066963d5 in (anonymous namespace)::lcl_CountBlanks(MathMLAttributeLengthValue const&, int*, int*) /starmath/source/mathml/mathmlimport.cxx:1497:30
> #4 0x2b46066943eb in (anonymous namespace)::SmXMLSpaceContext_Impl::startFastElement(int, com::sun::star::uno::Reference<com::sun::star::xml::sax::XFastAttributeList> const&) /starmath/source/mathml/mathmlimport.cxx:1526:25
[...]
Formula-14/content.xml in sw/qa/extras/odfexport/data/tdf144319.odt contains
width="0.444em", which resulted in the inaccurate Fraction 476741369/1073741824
since 67d83e40e2c4f3862c50e6abeabfc24a75119fc8 "speedup rational_FromDouble"
(which computes dVal=4.76741e+08 and nDen=1073741824; where before that speedup
it computed dVal=4.44e+08 and nDen=1000000000, which would have resulted in the
accurate Fraction 111/250). The excessively large denominator (1073741824 =
std::numeric_limits<sal_Int32>::max()/2 + 1) then caused overflow later on, as
shown above.
Change-Id: I221549528e4fa155e10697d218ec76bf678e8b2f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122186
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
Diffstat (limited to 'starmath')
-rw-r--r-- | starmath/source/mathml/mathmlattr.cxx | 25 |
1 files changed, 24 insertions, 1 deletions
diff --git a/starmath/source/mathml/mathmlattr.cxx b/starmath/source/mathml/mathmlattr.cxx index 008f8e2a81f6..7da7f947a8c6 100644 --- a/starmath/source/mathml/mathmlattr.cxx +++ b/starmath/source/mathml/mathmlattr.cxx @@ -9,6 +9,8 @@ #include <mathmlattr.hxx> +#include <o3tl/safeint.hxx> + #include <unordered_map> static sal_Int32 ParseMathMLUnsignedNumber(const OUString& rStr, Fraction& rUN) @@ -16,6 +18,9 @@ static sal_Int32 ParseMathMLUnsignedNumber(const OUString& rStr, Fraction& rUN) auto nLen = rStr.getLength(); sal_Int32 nDecimalPoint = -1; sal_Int32 nIdx; + sal_Int64 nom = 0; + sal_Int64 den = 1; + bool validNomDen = true; for (nIdx = 0; nIdx < nLen; nIdx++) { auto cD = rStr[nIdx]; @@ -28,11 +33,29 @@ static sal_Int32 ParseMathMLUnsignedNumber(const OUString& rStr, Fraction& rUN) } if (cD < u'0' || u'9' < cD) break; + if (validNomDen + && (o3tl::checked_multiply(nom, sal_Int64(10), nom) + || o3tl::checked_add(nom, sal_Int64(cD - u'0'), nom) + || (nDecimalPoint >= 0 && o3tl::checked_multiply(den, sal_Int64(10), den)))) + { + validNomDen = false; + } } if (nIdx == 0 || (nIdx == 1 && nDecimalPoint == 0)) return -1; - rUN = Fraction(rStr.copy(0, nIdx).toDouble()); + // If the input "xx.yyy" can be represented with nom = xx*10^n + yyy and den = 10^n in sal_Int64 + // (where n is the length of "yyy"), then use that to create an accurate Fraction (and TODO: we + // could even ignore trailing "0" characters in "yyy", for a smaller n and thus a greater chance + // of validNomDen); if not, use the less accurate approach of creating a Fraction from double: + if (validNomDen) + { + rUN = Fraction(nom, den); + } + else + { + rUN = Fraction(rStr.copy(0, nIdx).toDouble()); + } return nIdx; } |