Date: prev next · Thread: first prev next last
2015 Archives by date, by thread · List index


On 08/12/2015 05:53 PM, Eike Rathke wrote:
On Wednesday, 2015-08-12 17:48:57 +0200, Eike Rathke wrote:
The new test code triggers a division by zero now (as seen with
-fsanitize=undefined):

First, that shouldn't matter as it produces a double INF value.

I wonder though why that didn't propagate into the += operation and
whether that behavior is really undefined.. the test should had failed
already.

Seeing that it already tested for an error things should be correct.

So, should fsanitize maybe be taught about double infinity? ;)

...or, developers be taught about the C++ Standard? ;) It is quite clear on this: "If the second operand of / or % is zero the behavior is undefined." ([expr.mul]/4)

Now, ISO/IEC 60559 may mandate the result of division by zero to be infinity under certain circumstances (if the dividend is finite and non-zero) an NaN otherwise, when operating in its default exception handling mode, and C++ implementations may follow that, and we may even require that at least certain parts of the LO code base must be executed in conformance to ISO/IEC 60559 in its default exception handling mode.

However, in other parts of the LO code base it might be unexpected that floating-point division by zero happens, and its occurrence might indicate a bug (cf. "git log --grep -fsanitize=float-divide-by-zero"), so I'm reluctant to disable -fsanitize=float-divide-by-zero wholesale.

For those places where we apparently require floating-point division to adhere to ISO/IEC 60559 in its default exception handling mode, would it be possible to "dress those up" (like in the below ad-hoc patch), or would that be unreasonable?


diff --git a/sc/source/core/tool/interpr3.cxx b/sc/source/core/tool/interpr3.cxx
index ce3dc91..a0e02b1 100644
--- a/sc/source/core/tool/interpr3.cxx
+++ b/sc/source/core/tool/interpr3.cxx
@@ -30,11 +30,12 @@
 #include "scmatrix.hxx"
 #include "globstr.hrc"

-#include <math.h>
+#include <cmath>
 #include <vector>
 #include <algorithm>
 #include <boost/math/special_functions/log1p.hpp>
 #include <comphelper/random.hxx>
+#include <config_options.h>

 using ::std::vector;
 using namespace formula;
@@ -2821,6 +2822,23 @@ void ScInterpreter::ScFTest()
     PushDouble(2.0*GetFDist(fF, fF1, fF2));
 }

+namespace {
+
+double divide(double a, double b) {
+#if !ENABLE_RUNTIME_OPTIMIZATIONS
+    if (b == 0) {
+        if (std::isfinite(a) && a != 0) {
+            return std::copysign(INFINITY, a);
+        } else {
+            return NAN;
+        }
+    }
+#endif
+    return a / b;
+}
+
+}
+
 void ScInterpreter::ScChiTest()
 {
     if ( !MustHaveParamCount( GetByte(), 2 ) )
@@ -2850,7 +2868,7 @@ void ScInterpreter::ScChiTest()
             {
                 double fValX = pMat1->GetDouble(i,j);
                 double fValE = pMat2->GetDouble(i,j);
-                fChi += (fValX - fValE) * (fValX - fValE) / fValE;
+                fChi += divide((fValX - fValE) * (fValX - fValE), fValE);
             }
             else
             {


Context


Privacy Policy | Impressum (Legal Info) | Copyright information: Unless otherwise specified, all text and images on this website are licensed under the Creative Commons Attribution-Share Alike 3.0 License. This does not include the source code of LibreOffice, which is licensed under the Mozilla Public License (MPLv2). "LibreOffice" and "The Document Foundation" are registered trademarks of their corresponding registered owners or are in actual use as trademarks in one or more countries. Their respective logos and icons are also subject to international copyright laws. Use thereof is explained in our trademark policy.