bugGNU Octave - Bugs: bug #49642, rethrow (lasterror ()) loses stack...

 
 

bug #49642: rethrow (lasterror ()) loses stack information from previous error structure

Submitter:  Philip Nienhuis <philipnienhuis>
Submitted:  Fri 18 Nov 2016 09:10:25 AM UTC
   
 
Category:  Interpreter Severity:  4 - Important
Priority:  5 - Normal Item Group:  Regression
Status:  Fixed Assigned to:  None
Originator Name:  Philip Nienhuis Open/Closed:  * Closed
Release:  * 4.2.0 Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 23 Nov 2016 03:31:23 AM UTC, comment #14: 

I verified the fix on stable.  Closing report.

Rik <rik5>
Group administrator
Tue 22 Nov 2016 09:33:03 PM UTC, comment #13: 

I checked in the following changeset on stable and merged with default:

http://hg.savannah.gnu.org/hgweb/octave/rev/1d3d0321bc5d

This was a bit more complicated than it should have been and things are still a bit of a mess in error.cc.  There are too many things that have side effects in there...

John W. Eaton <jwe>
Group administrator
Mon 21 Nov 2016 07:10:44 PM UTC, comment #12: 

I didn't take it as a complaint.

I'll try to look at this some more this afternoon.

John W. Eaton <jwe>
Group administrator
Mon 21 Nov 2016 05:36:19 PM UTC, comment #11: 

Sorry if my test came across as complaining.  This definitely preserves the call stack which is most important.  The printing of just a single bit of the error stack seems to be in Frethrow itself.  We probably just need to figure out either the right internal function to call that iterates over the stack, or have a for loop in Frethrow over the whole stack.

Rik <rik5>
Group administrator
Mon 21 Nov 2016 03:34:47 PM UTC, comment #10: 

I don't think it's correct for the rethrow of the lasterror to print something different.  Probably the message should be constructed in the same way from the stack info in both cases.  That's something additional that I would like to fix.  But at least this minimal patch does preserve the stack info across the call to rethrow.

John W. Eaton <jwe>
Group administrator
Mon 21 Nov 2016 06:02:48 AM UTC, comment #9: 

I tested the quick patch on stable and it seems to work.  One thing I did notice is that when the error is rethrown the printed stack depth is only one deep.  In the original error, it includes the functions validsetargs and union.  In the subsequent rethrow, it only includes validsetargs.  Is that correct?


>> union ({'a'}, 1)
error: union: cell array of strings cannot be combined with a nonstring value
error: called from
    validsetargs at line 39 column 9
    union at line 51 column 8
>> lasterror ().stack.file
ans = /home/rik/wip/Projects_Mine/octave-stable/scripts/set/private/validsetargs.m
ans = /home/rik/wip/Projects_Mine/octave-stable/scripts/set/union.m
>> rethrow (lasterror ())
union: cell array of strings cannot be combined with a nonstring value
error: called from 'validsetargs' in file /home/rik/wip/Projects_Mine/octave-stable/scripts/set/private/validsetargs.m near line 39, column 9
>> lasterror ().stack.file
ans = /home/rik/wip/Projects_Mine/octave-stable/scripts/set/private/validsetargs.m
ans = /home/rik/wip/Projects_Mine/octave-stable/scripts/set/union.m
>>



Rik <rik5>
Group administrator
Sun 20 Nov 2016 07:42:58 PM UTC, comment #8: 

Here's a minimal patch that could probably go on stable.  I can see a few other improvements that could be made, but this may be enough for 4.2.1.  If it works for you, then I'll push it with a proper commit message.

(file #39032)

John W. Eaton <jwe>
Group administrator
Sun 20 Nov 2016 06:17:21 PM UTC, comment #7: 

It looks to me like error_1 always ends with "throw" so it won't return.

I'll look at how to reorganize this properly given the change from the global error_state variable to using exceptions.
f

John W. Eaton <jwe>
Group administrator
Fri 18 Nov 2016 07:21:34 PM UTC, comment #6: 

The problem is in error.cc in Frethrow.  The code is


  // Ugh.
  std::string tmp_msg (msg);
  if (tmp_msg[len-1] == '\n')
    {
      if (len > 1)
        {
          tmp_msg.erase (len - 1);
          rethrow_error (id.c_str (), "%s\n", tmp_msg.c_str ());
        }
    }
  else
    rethrow_error (id.c_str (), "%s", tmp_msg.c_str ());

  // FIXME: is this the right thing to do for Vlast_error_stack?
  //        Should it be saved and restored with unwind_protect?

  Vlast_error_stack = err_stack;

  std::cerr << "Pre-Stack 2" << std::endl;


I added simple printf debug code and the internal call to rethrow_error never returns to this function.  That means Vlast_error_stack is not re-initialized.  The code for rethrow_error is


void
rethrow_error (const char *id, const char *fmt, ...)
{
  va_list args;
  va_start (args, fmt);
  error_1 (std::cerr, 0, id, fmt, args);
  va_end (args);
}


I tried removing the NO_RETURN attribute on error_1 but that didn't help. 

I'm adding jwe to the CC list since I'm at a loss.

Rik <rik5>
Group administrator
Fri 18 Nov 2016 06:31:13 PM UTC, comment #5: 

Here is a simpler example to demonstrate the error


>> union ({'a'}, 1)
error: union: cell array of strings cannot be combined with a nonstring value
error: called from
    validsetargs at line 39 column 9
    union at line 51 column 8
>> lasterror.stack.file
ans = /opt/gnu/octave/share/octave/4.2.0/m/set/private/validsetargs.m
ans = /opt/gnu/octave/share/octave/4.2.0/m/set/union.m
>> rethrow (lasterror ())
union: cell array of strings cannot be combined with a nonstring value
>> lasterror.stack.file
## nothing


Mike Miller <mtmiller>
Group Member
Fri 18 Nov 2016 06:27:15 PM UTC, comment #4: 

Then it looks like this is about


rethrow (lasterror ())


losing all of the stack information from the lasterror structure. According to https://www.mathworks.com/help/matlab/ref/rethrow.html the stack of the error structure is supposed to be reinstated at the point when rethrow is called, and that doesn't seem to be happening. This did work in 4.0.3, so marking as a regression.

Mike Miller <mtmiller>
Group Member
Fri 18 Nov 2016 04:45:03 PM UTC, comment #3: 

I hit it with installing an OF package from after your cset
http://hg.savannah.gnu.org/hgweb/octave/rev/093b49ac544e
("Capitalize ....")

If you simply hg update to -say- 4a31bd79d7e8 ("maint: Correct spacing...") and then try to install octclip:

pkg install -forge -verbose octclip

you'll see it happening right away.
But of course the next issue is that the error is in a try-catch so debug_on_error(1) actually gets triggered in the catch clause when lasterr is rethrown. (I did use debug_on_error(1) initially.)

And that may be a clue - it could be that the try-catch hides errors happening inside the try clause and as a consequence obfuscates the backtrace.

Philip Nienhuis <philipnienhuis>
Group Member
Fri 18 Nov 2016 04:27:05 PM UTC, comment #2: 

Can you create a simple test case and tar up the directory and files and attach it to this bug report?

Also, what does this return


debug_on_error (1)
# Do stuff to cause error
dbstack


I want to know if the backtrace stack is correct, but error() just isn't reporting it, or if the problem is deeper.

Rik <rik5>
Group administrator
Fri 18 Nov 2016 10:59:32 AM UTC, comment #1: 

...tracking down here the bug...  => ...tracking down *w*here the bug...

Philip Nienhuis <philipnienhuis>
Group Member
Fri 18 Nov 2016 09:10:25 AM UTC, original submission:  

Revisiting big #49635, an issue that hindered effective tracking down here the bug occurred was that the error traceback was incomplete (stanza for convenience repeated from bug report 49635):

:
make: Leaving directory `/tmp/oct-qVNFk3/octclip-1.0.8/src'
copyfile C:\Users\philip\AppData\Local\Temp\oct-qVNFk3\octclip-1.0.8\src\_oc_polyboo
l.oct C:\Users\philip\AppData\Local\Temp\oct-qVNFk3\octclip-1.0.8\inst\x86_64-w64-mi
ngw32-api-v51
'desc' undefined near line 30 column 43
error: called from
    install at line 242 column 5
    pkg at line 394 column 9


while the actual error occurred in
./pkg/private/default_prefix.m   (not mentioned, but correct line number in traceback)
called from:
./pkg/private/install.m >subfunction> create_pkgadddel  (not mentioned)
called from:
./pkg/private/install.m  (mentioned fine, "correct"[*] line no.).

Maybe this is due to default_prefix being called from a subfunction of another private function?

[*]
Another issue is that the line number in install.m is "correct" in the sense that it is the line number where
"rethrow (lasterror())"
is called. But the actual error occurred in a function call on L.228.  IOW, this is a bit deceiving.
Is there a way to uncover the actual line number where an error occurred when calling rethrow?

Philip Nienhuis <philipnienhuis>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #39032:  diffs.txt added by jwe (3KiB - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by rik5
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by philipnienhuis (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 11 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-11-23 rik5 StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2016-11-21 rik5 StatusConfirmed In Progress
    2016-11-20 jwe Attached File- Added diffs.txt, #39032
    2016-11-18 rik5 Carbon-Copy- Added jwe
    2016-11-18 mtmiller StatusNeed Info Confirmed
    2016-11-18 mtmiller Severity3 - Normal 4 - Important
        Releasedev 4.2.0
        SummaryTraceback from errors in private functions is incomplete rethrow (lasterror ()) loses stack information from previous error structure
    2016-11-18 mtmiller Item GroupIncorrect Result Regression
    2016-11-18 rik5 StatusNone Need Info

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code