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
             {

_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Reply via email to