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

[Rik]

Update of bug #54414 (project octave):

                  Status:                    None => Patch Submitted        

    _______________________________________________________

Follow-up Comment #1:

Tracing this down.  The zeros function is in data.cc which calls the
fill_matrix function which is also in data.cc.  The fill_matrix command calls
get_dimensions in libinterp/corefcn/utils.cc.  Within get_dimensions there is
this line of code


    const Array<octave_idx_type> v = a.octave_idx_type_vector_value ();


The octave_idx_type_vector_value function is defined for octave_value objects
in ov.cc.  The prototype for that function is



Array<octave_idx_type>
octave_value::octave_idx_type_vector_value (bool require_int,
                                            bool force_string_conv,
                                            bool force_vector_conversion)
const


For converting doubles to octave_idx_type the following code is used.


      const NDArray a = array_value (force_string_conv);

      if (require_int)
        {
          retval.resize (a.dims ());
          for (octave_idx_type i = 0; i < a.numel (); i++)
            {
              double ai = a.elem (i);
              octave_idx_type v = static_cast<octave_idx_type> (ai);
              if (ai == v)
                retval.xelem (i) = v;
              else
                {
                  error_with_cfn ("conversion to integer value failed");
                  break;
                }
            }
        }
      else
        retval = Array<octave_idx_type> (a);


If the require_int boolean is true then octave checks for overflow during
conversion.  The default, however, is false which means results aren't checked
and the constructor Array<octave_idx_type> is invoked which can map doubles to
negative values.

Attached is a small diff which sets the require_int bool to true when called
from get_dimensions.  It works, but there may be a better option for the fix. 
In particular, the error message isn't very informative


octave:1> zeros (1e31)
error: zeros: conversion to integer value failed


Running 'make check' passes though with this small change.

A grep through libinterp shows get_dimensions used 40 times, but some of these
seem to be for a mex function of the same name which can probably be ignored.


corefcn/mxarray.in.h:205:  virtual mwSize * get_dimensions (void) const = 0;
corefcn/mxarray.in.h:426:  mwSize * get_dimensions (void) const { return
rep->get_dimensions (); }
corefcn/utils.h:87:  get_dimensions (const octave_value& a, const char
*warn_for,
corefcn/utils.h:91:  get_dimensions (const octave_value& a, const
octave_value& b,
corefcn/utils.h:96:  get_dimensions (const octave_value& a, const char
*warn_for,
corefcn/utils.h:199:OCTAVE_DEPRECATED (5, "use 'octave::get_dimensions'
instead")
corefcn/utils.h:201:get_dimensions (const octave_value& a, const char
*warn_for,
corefcn/utils.h:204:OCTAVE_DEPRECATED (5, "use 'octave::get_dimensions'
instead")
corefcn/utils.h:206:get_dimensions (const octave_value& a, const octave_value&
b,
corefcn/utils.h:210:OCTAVE_DEPRECATED (5, "use 'octave::get_dimensions'
instead")
corefcn/utils.h:212:get_dimensions (const octave_value& a, const char
*warn_for,
corefcn/data.cc:3875:      octave::get_dimensions (args(0), fcn, dims);
corefcn/data.cc:3985:      octave::get_dimensions (args(0), fcn, dims);
corefcn/data.cc:4049:      octave::get_dimensions (args(0), fcn, dims);
corefcn/data.cc:4114:      octave::get_dimensions (args(0), fcn, dims);
corefcn/data.cc:4169:      octave::get_dimensions (args(0), fcn, dims);
corefcn/data.cc:5013:      octave::get_dimensions (args(0), "eye", nr, nc);
corefcn/data.cc:5020:      octave::get_dimensions (args(0), args(1), "eye",
nr, nc);
corefcn/mex.cc:196:            mwSize *xdims = retval->get_dimensions ();
corefcn/mex.cc:269:    get_dimensions ();
corefcn/mex.cc:277:  mwSize * get_dimensions (void) const
corefcn/mex.cc:298:    get_dimensions ();
corefcn/mex.cc:321:    get_dimensions ();
corefcn/mex.cc:584:    get_dimensions ();
corefcn/mex.cc:811:  mwSize * get_dimensions (void) const { return dims; }
corefcn/mex.cc:1044:    mwSize *d = get_dimensions ();
corefcn/mex.cc:1153:    mwSize *dv = get_dimensions ();
corefcn/mex.cc:2864:  return ptr->get_dimensions ();
corefcn/sparse.cc:154:      octave::get_dimensions (args(0), args(1),
"sparse", m, n);
corefcn/sparse.cc:187:          octave::get_dimensions (args(3), args(4),
"sparse", m, n);
corefcn/utils.cc:1078:  void get_dimensions (const octave_value& a, const char
*warn_for,
corefcn/utils.cc:1108:  void get_dimensions (const octave_value& a, const char
*warn_for,
corefcn/utils.cc:1131:  void get_dimensions (const octave_value& a, const
octave_value& b,
corefcn/utils.cc:1604:get_dimensions (const octave_value& a, const char
*warn_for,
corefcn/utils.cc:1607:  return octave::get_dimensions (a, warn_for, dim);
corefcn/utils.cc:1611:get_dimensions (const octave_value& a, const
octave_value& b,
corefcn/utils.cc:1615:  return octave::get_dimensions (a, b, warn_for, nr,
nc);
corefcn/utils.cc:1619:get_dimensions (const octave_value& a, const char
*warn_for,
corefcn/utils.cc:1622:  return octave::get_dimensions (a, warn_for, nr, nc);
octave-value/ov-cell.cc:1254:      octave::get_dimensions (args(0), "cell",
dims);


Ignoring the mex functions it is only data.cc, sparse.cc, and ov-cell.cc that
use get_dimensions.

Or, maybe this should be tackled in the Array constructor?


(file #44685)
    _______________________________________________________

Additional Item Attachment:

File name: 54114.diff                     Size:0 KB


    _______________________________________________________

Reply to this item at:

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

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