bugGNU Octave - Bugs: bug #61132, Colon operator fails for int64 and...


bug #61132: Colon operator fails for int64 and uint64

Submitted by:  None
Submitted on:  Thu 09 Sep 2021 01:19:46 AM UTC  
Category:  Interpreter Severity:  5 - Blocker
Priority:  7 - High Item Group:  Unexpected Error or Warning
Status:  Ready For Test Assigned to:  None
Originator Name:  Originator Email:  -email is unavailable-
Open/Closed:  Open Release:  7.0.90
Operating System:  Any

Add a New Comment (Rich Markup)

You are not logged in

Please log in, so followups can be emailed to you.


( Jump to the original submission)

Fri 03 Dec 2021 05:49:07 PM UTC, comment #22: 
Markus Mützel <mmuetzel>
Project Member
Fri 03 Dec 2021 07:38:35 AM UTC, comment #21: 

A dangling reference again?

Maybe the attached patch helps...

(file #52419)

Markus Mützel <mmuetzel>
Project Member
Fri 03 Dec 2021 07:31:07 AM UTC, comment #20: 

One of the Ubuntu runners on GitHub segfaulted during the tests for nchoosek.m:

  specfun/nchoosek.m .............................................fatal: caught signal Segmentation fault -- stopping myself...
/bin/bash: line 1: 182410 Segmentation fault      (core dumped) /bin/bash ../run-octave --no-init-file --silent --no-history -p /home/runner/work/octave/octave/.build/test/mex /home/runner/work/octave/octave/.build/../test/fntests.m /home/runner/work/octave/octave/.build/../test
make[3]: *** [Makefile:32243: check-local] Error 139
make[3]: Leaving directory '/home/runner/work/octave/octave/.build'
make[2]: *** [Makefile:28342: check-am] Error 2
make[2]: Leaving directory '/home/runner/work/octave/octave/.build'
make[1]: *** [Makefile:28050: check-recursive] Error 1
make[1]: Leaving directory '/home/runner/work/octave/octave/.build'
make: *** [Makefile:28344: check] Error 2
make: Leaving directory '/home/runner/work/octave/octave/.build'

That might be caused by this change.

Markus Mützel <mmuetzel>
Project Member
Thu 02 Dec 2021 06:16:06 PM UTC, comment #19: 

Pushed your patch to the tests here:

Markus Mützel <mmuetzel>
Project Member
Thu 02 Dec 2021 05:15:17 PM UTC, comment #18: 

I pushed to stable:

Marking as ready for test.

Markus Mützel <mmuetzel>
Project Member
Thu 02 Dec 2021 02:28:22 PM UTC, comment #17: 

I am attaching some simplifications to test/range.tst for bug #61132 and bug #61300, mostly to merge code duplication.

(file #52407)

Arun Giridhar <arungiridhar>
Thu 02 Dec 2021 12:03:34 AM UTC, comment #16: 

Patch v3 works perfectly for me. Thank you, Markus!

Arun Giridhar <arungiridhar>
Wed 01 Dec 2021 07:45:23 PM UTC, comment #15: 

The attached patch stores the "reverse" flag in files only for types that need it.
It also fixes saving and loading range objects in HDF5 format.

It should be ready for review now.

(file #52401)

Markus Mützel <mmuetzel>
Project Member
Tue 30 Nov 2021 09:49:18 PM UTC, comment #14: 

The following expressions all pass with v2:

hi = intmax ("uint8"),  lo = hi-10, hi:-1:lo
hi = intmax ("uint16"), lo = hi-10, hi:-1:lo
hi = intmax ("uint32"), lo = hi-10, hi:-1:lo
hi = intmax ("uint64"), lo = hi-10, hi:-1:lo

Arun Giridhar <arungiridhar>
Tue 30 Nov 2021 09:34:14 PM UTC, comment #13: 

The v2 patch works properly for the descending integer ranges. The tests in range.tst all pass. There are some unrelated regressions related to saving and loading:

  bug-53468/bug-53468.tst ........................................ pass    0/3
                                                              REGRESSION   3
  classdef/classdef.tst .......................................... pass   34/37
                                                    (reported bug) XFAIL   3
  fcn-handle/bug-51567.tst ....................................... pass    0/2
                                                    (reported bug) XFAIL   2


  PASS                            17040
  FAIL                                2
  REGRESSION                          3

Bug #53468 is about loading files created with older versions of Octave.

Arun Giridhar <arungiridhar>
Tue 30 Nov 2021 08:36:25 PM UTC, comment #12: 

Appending another wip patch that limits the scope of "reverse" ranges.
It also adds some initial support for saving and loading them from files.
Is it ok to add the new property to the save files for all ranges? Or should we only do this for the (unsigned) integer ranges?

(file #52392)

Markus Mützel <mmuetzel>
Project Member
Tue 30 Nov 2021 08:13:40 PM UTC, comment #11: 

I was also undecided whether to keep the conditions simple or to limit the scope of the change. I'll try to do the latter.

While testing saving and loading ranges to the various formats, I came across this error (even without this change):

>> a = b = 1:4;

>> sv_file = [tempname(), ".h5"];

>> save (sv_file, "a", "-hdf5");

warning: <unknown type>: saving hdf5 files not available in this version of Octave
error: save: error while writing 'a' to hdf5 file

I can save other types (like a double array) in HDF5 format.

There is a `ov_range<T>::save_hdf5` in `ov-range.cc`. Shouldn't this be working?

Markus Mützel <mmuetzel>
Project Member
Tue 30 Nov 2021 07:14:29 PM UTC, comment #10: 

Getting a build error. Do you see something like this?

  CXX      liboctave/array/libarray_la-idx-vector.lo
../liboctave/array/idx-vector.cc:195:76: error: too few arguments to function call, expected 4, have 3
      (static_cast<double> (m_start+1), static_cast<double> (m_step), m_len);
../liboctave/array/Range.h:112:21: note: 'make_n_element_range' declared here
    static range<T> make_n_element_range (const T& base, const T& increment,
1 error generated.
make[2]: *** [Makefile:22111: liboctave/array/libarray_la-idx-vector.lo] Error 1

Arun Giridhar <arungiridhar>
Tue 30 Nov 2021 06:19:46 PM UTC, comment #9: 

If I understand correctly, the only cases that need special treatment are when the base and limit are unsigned integers (of the same type) and the increment is a negative double value.  If so, then maybe we could limit this special handling of "reverse" ranges to those unsigned integer ranges only and have the signed integer and floating point ranges always defined with the "reverse" flag set to false?

We never had integer ranges before, so saving and loading those shouldn't be a compatibility issue.

John W. Eaton <jwe>
Project Administrator
Tue 30 Nov 2021 05:36:07 PM UTC, comment #8: 

I'm attaching a possible patch that adds initial support for reverse ranges.

I haven't yet checked if saving and loading still works with these ranges. (It probably won't work correctly at least for some file formats.) But the test suite still passes, and the examples in this report work for me with this change.

Before I continue working on this: Is the approach ok?

(file #52391)

Markus Mützel <mmuetzel>
Project Member
Fri 26 Nov 2021 04:24:02 PM UTC, comment #7: 

See also bug #61565.  Marking this as a blocker for the version 7 release.

John W. Eaton <jwe>
Project Administrator
Wed 06 Oct 2021 06:28:25 PM UTC, comment #6: 

Found this earlier bug report through a search. Linking related bug #61300 about integer range violations

Arun Giridhar <arungiridhar>
Tue 14 Sep 2021 06:35:43 PM UTC, comment #5: 

Oops again.  When I recently added integer ranges, I made the mistake of assuming that we could check for type compatibility (integers of the same type possibly mixed with doubles) and then if we have an integer range (integer operands possibly mixed with integer values stored as doubles), we could convert all the operands to the single integer type.  I completely forgot about the possibility of something like

lo = uint8 (10);
hi = uint8 (20);

I'll take another look at that since I think it should be valid.

Reopening as "in progress".

John W. Eaton <jwe>
Project Administrator
Mon 13 Sep 2021 10:26:09 PM UTC, comment #4: 

Hello jwe. A follow-up problem for all unsigned integer types, not just uint64.

hi = intmax("uint64");  # OK
lo = hi-99;             # OK
up = lo:hi;             # OK
down = hi:-1:lo;        # error: colon operator increment invalid (not an integer or out of range for given integer type)
down = fliplr(up);      # current workaround to the error
charup = 'a':'z';       # for comparison. this is OK.
chardown = 'z':-1:'a';  # for comparison. this is OK too!

Is this a conscious limit that one cannot construct descending ranges for unsigned integer types with the colon operator or is it a bug?

Thu 09 Sep 2021 02:22:33 PM UTC, comment #3: 

I added a test and pushed the following changeset


Closing as fixed.

John W. Eaton <jwe>
Project Administrator
Thu 09 Sep 2021 01:51:56 PM UTC, comment #2: 

I confirm that the patch works as intended. Thank you to jwe for the very fast fix!

Thu 09 Sep 2021 03:45:30 AM UTC, comment #1: 

Oops.  I'm attaching a possible patch.

(file #51880)

John W. Eaton <jwe>
Project Administrator
Thu 09 Sep 2021 01:19:46 AM UTC, original submission:  

This problem happens for int64 and uint64 types on Octave 7:

octave:16> lo = uint64(10) ^ 17
lo = 100000000000000000
octave:17> hi = lo + 1e3
hi = 100000000000001000
octave:18> class(lo)
ans = uint64
octave:19> class(hi)
ans = uint64
octave:20> lo < intmax ("uint64")
ans = 1
octave:21> hi < intmax ("uint64")
ans = 1
octave:22> rng = (lo:hi)
error: colon operator upper bound invalid (not an integer or out of range for given integer type)

The values lo and hi are within the legal range for int64 and uint64 but the colon operator fails when trying to create what should be a valid range.



(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:

Attached Files
file #52419:  bug61132-lifetime.patch added by mmuetzel (1KiB - application/octet-stream)
file #52407:  range_test.patch added by arungiridhar (3KiB - text/x-patch)
file #52401:  bug61132-range-reverse-v3.patch added by mmuetzel (34KiB - application/octet-stream)
file #52392:  bug61132-range-reverse-v2.patch added by mmuetzel (23KiB - application/octet-stream)
file #52391:  bug61132-range-reverse.patch added by mmuetzel (15KiB - application/octet-stream)
file #51880:  range-error-diffs.txt added by jwe (2KiB - text/plain)


Depends on the following items: None found

   bug dependencies.


Carbon-Copy List
  • -email is unavailable- added by mmuetzel (Updated the item)
  • -email is unavailable- added by arungiridhar (Posted a comment)
  • -email is unavailable- added by jwe (Updated the item)

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.




    Follow 18 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-12-03 mmuetzel Attached File- => Added bug61132-lifetime.patch, #52419
    2021-12-02 mmuetzel StatusPatch Submitted => Ready For Test
    2021-12-02 arungiridhar Attached File- => Added range_test.patch, #52407
    2021-12-01 mmuetzel Dependencies- => bugs #61565 is dependent
    2021-12-01 mmuetzel Attached File- => Added bug61132-range-reverse-v3.patch, #52401
    2021-11-30 mmuetzel Attached File- => Added bug61132-range-reverse-v2.patch, #52392
    2021-11-30 mmuetzel Attached File- => Added bug61132-range-reverse.patch, #52391
        StatusIn Progress => Patch Submitted
        Releasedev => 7.0.90
        Operating SystemGNU/Linux => Any
    2021-11-26 jwe Severity3 - Normal => 5 - Blocker
        Priority5 - Normal => 7 - High
    2021-09-14 jwe StatusFixed => In Progress
        Open/ClosedClosed => Open
    2021-09-09 jwe StatusPatch Submitted => Fixed
        Open/ClosedOpen => Closed
    2021-09-09 jwe Attached File- => Added range-error-diffs.txt, #51880
        StatusNone => Patch Submitted

    Back to the top

    Powered by Savane 3.7