bugGNU Octave - Bugs: bug #61841, rethrowing an exception leads to...

 
 

bug #61841: rethrowing an exception leads to an internal error

Submitter:  Sébastien Villemot <svillemot>
Submitted:  Mon 17 Jan 2022 02:34:23 PM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Unexpected Error or Warning
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * 7.0.90 Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 24 Jan 2022 03:57:35 AM UTC, comment #18: 

I checked in jwe's amended changeset here: http://hg.savannah.gnu.org/hgweb/octave/rev/8c6486ffc1d9.

Marking as Fixed and closing report.

Rik <rik5>
Group administrator
Fri 21 Jan 2022 06:30:24 PM UTC, comment #17: 

Oops, I accidentally left a diff hunk in the commit message.  Try the file attached to this comment instead.

(file #52705)

John W. Eaton <jwe>
Group administrator
Fri 21 Jan 2022 04:51:02 PM UTC, comment #16: 

@jwe: I applied the patch in file #52702.  It passes 'make check' now, but I'm concerned that the patch was malformed.  I have a section that looks like part of a diff hunk, but it is showing up in the Mercurial commit message.  If it was meant to be part of the patch then something went wrong.

Rik <rik5>
Group administrator
Fri 21 Jan 2022 02:59:33 AM UTC, comment #15: 

OK, for now we can fix the rethrow function to ensure that the element of the stack struct array has the column field set to -1.  In the future, when we add MException objects to Octave (see bug #40828) I think we should deprecate the use of a struct array as an argument to throw and rethrow.  Also, according to the Matlab documentation, the MException constructor doesn't accept the stack struct in the MException constructor.  That is added to the object when the exception is thrown.

I'm attaching another proposed change based on the one posted in comment #14.  Note that the test in comment #14 will fail because it compares the contents of lasterror before and after the rethrow and they are different now because rethrow adds the "column" field to the stack struct array.  But I think the behavior is better now and the test is no longer correct.  In the new changeset I updated the test in the error.tst file.

(file #52702)

John W. Eaton <jwe>
Group administrator
Wed 19 Jan 2022 07:35:22 PM UTC, comment #14: 

I upgraded the diff from jwe into a true changeset (added commit message).  It is attached as rethrow.cset.  Unfortunately, it segfaults under regression testing.  The problem case is attached as errtst.m


%!test
%! try
%!   union ({'a'}, 1);
%! catch
%!   x = lasterror ();
%!   try
%!     y = struct ("message", "msg", "identifier", "", "stack",
%!                 struct ("file", "foo.m", "name", "foo", "line", 13));
%!     rethrow (y);
%!   catch
%!     assert (y, lasterror ());
%!   end_try_catch
%! end_try_catch


Reproduce with "test errtst"

The issue is that rethrow (and throw() if we implemented it) can take an exception object as an input.  The "stack" field in turn must have fieldnames "file", "name", and "line" for Matlab compatibility.  In the problem case above, rethrow() is called with just such a struct, but when Octave tries to get "column(i)" it fails as an indexing operation because, while the fieldname existis, it is empty (0x0).
https://savannah.gnu.org/bugs/?61841

(file #52685, file #52686)

Rik <rik5>
Group administrator
Wed 19 Jan 2022 05:03:22 PM UTC, comment #13: 

I think the empty matrix is appearing when the stack struct array is extended.  The code that eliminated the invalid (= -1) column field from the stack frame struct seemed like a good idea but then when the stack struct array is extended to another element, the default is for elements to be filled with [].

The attached should fix the error, but we will need to do a better job of ensuring that line and column info is always valid instead of sometimes being set to -1.  But maybe it is better to leave the invalid values there and work to fix them over time.

(file #52684)

John W. Eaton <jwe>
Group administrator
Wed 19 Jan 2022 04:18:29 PM UTC, comment #12: 

I can't exactly find where the column for an anonymous function is defined, but that is the place where we want to intervene.  If we make only the change in error.cc I suggested then rethrow () will work.  However, in Octave userspace the exception struct will still have [] for the column which isn't good.

Rik <rik5>
Group administrator
Mon 17 Jan 2022 07:46:52 PM UTC, comment #11: 

Actually the column value is also wrong (empty) when f is a regular function. Sorry for the noise.

Sébastien Villemot <svillemot>
Mon 17 Jan 2022 06:29:45 PM UTC, comment #10: 

Note that the problem is not limited to anonymous functions. It also happens if f in my initial example is a regular function with a dedicated source file.

Sébastien Villemot <svillemot>
Mon 17 Jan 2022 06:03:19 PM UTC, comment #9: 

@Rik: I agree that having the "column" number adopt the same value as "line" number when they are both undefined would be the cleanest solution.

Pantxo Diribarne <pantxo>
Group Member
Mon 17 Jan 2022 05:56:42 PM UTC, comment #8: 

We can stop the problem by changing the code in error.cc in the function make_stack_frame_list.  However, I think it would be better form to change the code that produces the column numbers so that valid data is always created.

If we were to modify error.cc then this code


for (octave_idx_type i = 0; i < nel; i++)
      frames.push_back (frame_info (file(i).string_value (),
                                    name(i).string_value (),
                                    line(i).int_value (),
                                    (have_column
                                     ? column(i).int_value () : -1)));


could be changed to


for (octave_idx_type i = 0; i < nel; i++)
      frames.push_back (frame_info (file(i).string_value (),
                                    name(i).string_value (),
                                    line(i).int_value (),
                                    (have_column && ! column(i).isempty ()
                                     ? column(i).int_value () : -1)));


Rik <rik5>
Group administrator
Mon 17 Jan 2022 05:37:21 PM UTC, comment #7: 

I used this code for testing


f = @(x) x+1;

try
  f(1, 2)
catch ME
  keyboard;
  disp ('in catch block');
  rethrow(ME)
end


When Octave stops in the debugger I inspected the values of the variable ME.


keyboard> ME.stack.file
ans = /home/rik/wip/Projects_Mine/octave-stable/tst_rethrow.m
ans = /home/rik/wip/Projects_Mine/octave-stable/tst_rethrow.m
keyboard> ME.stack.name
ans = @<anonymous>
ans = tst_rethrow
keyboard> ME.stack.line
ans = -1
ans = 4
keyboard> ME.stack.column
ans = [](0x0)
ans = 3


I think Pantxo is write that is is the column field for the anonymous function that is incorrect.  I would propose setting this value to -1 to match what Octave does for the line number.


Rik <rik5>
Group administrator
Mon 17 Jan 2022 05:22:16 PM UTC, comment #6: 
Pantxo Diribarne <pantxo>
Group Member
Mon 17 Jan 2022 05:20:45 PM UTC, comment #5: 
Pantxo Diribarne <pantxo>
Group Member
Mon 17 Jan 2022 05:15:08 PM UTC, comment #4: 

The problem seems to be that the column field in the stack is expected to be an int, not an empty matrix like in that case. E.g the following works


f = @(x) x+1;

try
  f(1, 2)
catch ME
  ME.stack(1).column = -1;
  rethrow(ME)
end


Pantxo Diribarne <pantxo>
Group Member
Mon 17 Jan 2022 05:01:38 PM UTC, comment #3: 

This happens with 6.4.0 as well so it is not a 7.0.90 error.


error: octave_base_value::int_value (): wrong type argument 'matrix'
octave:4> ver
----------------------------------------------------------------------
GNU Octave Version: 6.4.0 (hg id: 08487765a674)


Anonymous
Mon 17 Jan 2022 04:56:22 PM UTC, comment #2: 

Where is the rethrown exception being caught? If nothing is catching it, isn't this expected behavior?

Anonymous
Mon 17 Jan 2022 03:43:50 PM UTC, comment #1: 

Confirmed. Same error also happens with other internal errors, e.g. provoking a max_recursion_depth error


1;
function recurse ()
  recurse ();
endfunction

try
  recurse ()
catch ME
  rethrow (ME)
end


Pantxo Diribarne <pantxo>
Group Member
Mon 17 Jan 2022 02:34:23 PM UTC, original submission:  

Hi,

When running the following snippet in 7.0.90:

f = @(x) x+1;

try
  f(1, 2)
catch ME
  rethrow(ME)
end


Octave fails with:

error: octave_base_value::int_value (): wrong type argument 'matrix'
error: called from
    rethrow_err at line 6 column 3


Of course, one would have excepted the rethrow to show the initial exception.

Note that this is not the same as bug #61789, since the problem also manifests with the current head of the stable branch (b20da6bed444).

Sébastien Villemot <svillemot>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #52705:  rethrow-cset-3.txt added by jwe (4KiB - text/plain)
file #52702:  rethrow-cset-2.txt added by jwe (5KiB - text/plain)
file #52685:  rethrow.cset added by rik5 (2KiB - application/octet-stream)
file #52686:  errtst.m added by rik5 (315B - text/x-matlab)

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-01-24 rik5 StatusConfirmed Fixed
        Open/ClosedOpen Closed
    2022-01-21 jwe Attached File- Added rethrow-cset-3.txt, #52705
    2022-01-21 jwe Attached File- Added rethrow-cset-2.txt, #52702
    2022-01-19 rik5 Attached File- Added rethrow.cset, #52685
        Attached File- Added errtst.m, #52686
    2022-01-19 jwe Attached File- Added stack-struct-array-diffs.txt, #52684
    2022-01-17 mmuetzel Summaryrethrowing a &quot;too many inputs&quot; exception leads to an internal error rethrowing an exception leads to an internal error
    2022-01-17 pantxo StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code