[Octave-bug-tracker] [bug #54414] Not recognizing that indices greater than (roughly) 2^63 or 20 digits are too large

[Dan Sebald]

Follow-up Comment #7, bug #54414 (project octave):

"Yes" and "no".  The same phenomenon is the core issue in both instances, but
these are different pieces of code.  Bug #45945 is discussing that LONG_INT ->
DOUBLE -> LONG_INT issue in the interpreter.  Here in the fread() and
get_size() routines, it is the fact that the index is being stored as a
double.  If Bug #45945 is fixed, it isn't going to fix the fread problem,
*unless* the indices are kept as octave_idx_type the whole way.

This is why I'm suggesting the hunk of code for that region of numbers for
which the double loses resolution:


+#if 1
+        // FIXME: The size specification involves the use of double
+        //        representation, so roughly any typed-in index values from
+        //        2^53 (mantissa size of double) to 2^63 can create aliased
+        //        nr and nc, i.e., they turn out negative from wrap-around
+        //        effect.  (See Bug #54405.)  Is this necessary given huge
+        //        size?  Should this be done in interpretter?
+        if (d >= 9007199254740992)
+          ::warning ("%s: possible loss of resolution converting from float
to octave_idx_type",
+                   who.c_str ());
+#endif


and it is noted as a FIXME for the simple fact the situation may be avoidable.
 Basically, the above is just a warning that things are on shaky ground with
the float to long int conversion.

But, as I look at this, the above doesn't seem to cover all the issues.  The
above is a loss of resolution, sure, but why the strange wrapping effect when
the numbers are supposedly greater than
std::numeric_limits<octave_idx_type>::max ()?

So, we know that the following is dodgy:


        if (d > std::numeric_limits<octave_idx_type>::max ())
          ::error ("%s: dimension too large for Octave's index type",
                   who.c_str ());


d, a double, could have been entered with a string, say, one greater than


octave:12> int64(inf)
ans = 9223372036854775807


i.e., [9223372036854775808, 9223372036854775808] and that string gets
translated to float and consequently becomes


octave:13> x = 9223372036854775808
x =    9.2234e+18
octave:14> x < int64(inf)
ans = 1


That's the loss of resolution at work.  Eventually as we increase x's string
value, the double will test greater than
std::numeric_limits<octave_idx_type>::max ().

But why is the following:


        retval = math::nint_big (d);


returning something zero or negative?  It should still be returning some
rather large number which will cause the "too big of index" error.

Let's take a look at that nint_big() routine from lo-mappers.cc.  There are
several of them:


    octave_idx_type
    nint_big (double x)
    {
      if (x > std::numeric_limits<octave_idx_type>::max ())
        return std::numeric_limits<octave_idx_type>::max ();
      else if (x < std::numeric_limits<octave_idx_type>::min ())
        return std::numeric_limits<octave_idx_type>::min ();
      else
        return static_cast<octave_idx_type> ((x > 0.0) ? (x + 0.5)
                                                       : (x - 0.5));
    }


Two things should be considered.  1) Could adding the 0.5 prior to the cast
cause an issue?  2) I believe the promotion rules means that the size
comparison is done by converting the integer to double, then compare.  But
perhaps what really should be done is convert the floating point to integer
then test.  Hold on, let me tweak the nbig_int...

Yes, that looks to have fixed the problem.  Take a look at the attached
changeset and see if you agree with it.  We still need to address the float
value being grossly larger than, but then there also needs to be a subtle
analysis at that decreased resolution boundary region.

Do we need to cover the


    int
    nint (double x)


case?  The double mantissa is adequate resolution for 32-bit int, but can
there be some value, say 1234.56789, that causes problems, i.e., the fraction
part takes away integer resolution?

(file #44686)
    _______________________________________________________

Additional Item Attachment:

File name: octave-nint_math_wrap_around-djs2018aug01.patch Size:4 KB


    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?54414>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/