bugGNU Octave - Bugs: bug #47837, Segmentation fault with missing...

 
 

bug #47837: Segmentation fault with missing warning state "all"

Submitted by:  Guillaume <gyom>
Submitted on:  Tue 03 May 2016 03:01:18 PM UTC  
 
Category:  Octave Function Severity:  4 - Important
Priority:  5 - Normal Item Group:  Segfault, Bus Error, etc.
Status:  Fixed Assigned to:  Lachlan Andrew <lachlan>
Originator Name:  Guillaume Open/Closed:  Closed
Release:  dev 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 15 Jul 2016 10:11:52 PM UTC, comment #15:

I verified the fix works. Closing report.

Rik <rik5>
Project Administrator
Fri 15 Jul 2016 05:23:02 PM UTC, comment #14:

I pushed Lachlan's change for bug #47543:

http://hg.savannah.gnu.org/hgweb/octave/rev/879f297f3194

I think that fixes this problem too. Can this report be closed now?

John W. Eaton <jwe>
Project Administrator
Fri 15 Jul 2016 03:48:02 PM UTC, comment #13:

As far as I understand it, the behavior of throwing an exception from a signal handler is not specified, so I don't think that is something we should do.

John W. Eaton <jwe>
Project Administrator
Thu 05 May 2016 07:38:49 AM UTC, comment #12:

Rik, I agree that segfaults are to be avoided whenever possible.

I have been toying with the idea of causing the signal handler to throw an exception that can be caught, instead of exiting. This would allow a clean solution for bug #36067 ("exist" on a buggy mex/oct file causing a crash), and would also make it cleaner for a general seg-fault to drop back to the command prompt with a warning to save whatever is possible and then restart.

What do you think?

Lachlan Andrew <lachlan>In charge of this item.
Wed 04 May 2016 03:45:39 PM UTC, comment #11:

I added bug #47543 as a dependency. Since Mike did most of the review over on the other report, I'll let him decide whether it is acceptable to push it now. In general, I hate segfaults since user's can lose valuable work when they unexpectedly happen. My vote would be to plug the hole, and then figure out the best real repair.

Rik <rik5>
Project Administrator
Wed 04 May 2016 03:37:00 PM UTC, comment #10:

Thanks Lachlan. Would be great if you could apply your patch as it is now.

Guillaume <gyom>
Wed 04 May 2016 02:03:31 AM UTC, comment #9:

Mike was right. The reason "all" is being removed is the same as bug #47543, and the patch there fixes this.

Its behaviour isn't ideal -- doing 'warning ("off", "all")' only changes the "all" state, and doesn't clear warnings that were set explicitly. However, it is better than crashing, and I think it would be good to apply that patch now, and implement the proper "all" behaviour in the new framework I mentioned.

Lachlan Andrew <lachlan>In charge of this item.
Tue 03 May 2016 11:02:16 PM UTC, comment #8:

I'll look into this.

I am planning an overhaul of the handling of warnings, which will include the ability to warn of non-existing warnings in the Octave: namespace, though not in other name spaces for the reason Rik mentions. I posted a request for comments on the maintainers' mailing list, and got no response. Since it will require a substantial overhaul, I'd really like to make sure that it will be approved once it is written.

Lachlan Andrew <lachlan>In charge of this item.
Tue 03 May 2016 05:54:20 PM UTC, comment #7:

To my surprise:

but, actually, the error is due to the identifier itself: according to Matlab's doc, it can only contain alphanumerics (A–Z, a–z, 0–9) and underscores, starting with a letter. It then works as I would expect:

So you can ignore my comment #5.

Guillaume <gyom>
Tue 03 May 2016 05:41:41 PM UTC, comment #6:

Unfortunately, maintaining a list of possible warnings to verify against would be horribly difficult and almost always out-of-date. What happens if a programmer wants to make use of their own special warning ID? The Octave core would never know about it and issue a warning.

Matlab might try to finesse the issue by only issuing an error if the warning ID has not already been used. What does Matlab do for the following?

Rik <rik5>
Project Administrator
Tue 03 May 2016 04:59:59 PM UTC, comment #5:

Thanks for looking into this. Just to add that Matlab's behavior with warning('query',...) and an unknown identifier is to return an error:

In the development branch, when the 'all' identifier exists, Octave returns:

Guillaume <gyom>
Tue 03 May 2016 04:40:47 PM UTC, comment #4:

I think bug #47543 is different. It was introduced on the change from 4.0.0 to 4.0.1. On the other hand, this report deals with a change from 4.0.2 to default.

Rik <rik5>
Project Administrator
Tue 03 May 2016 04:14:13 PM UTC, comment #3:

May be related to bug #47543, which is also on default but not stable, there is some discussion there.

Mike Miller <mtmiller>
Project Administrator
Tue 03 May 2016 04:04:40 PM UTC, comment #2:

Confirmed. This "works" with version 4.0.2 where it only produces an error message. But on the development branch it hits an assert statement which causes a panic.

The default branch code in the function warning_query in error.cc looks like this:

On the development branch the code now looks like this:

According to 'hg blame' the cset that made the change was 20535:b70cc4bd8109 by jwe. I've added him to the CC list for this bug. The easiest solution would be to revert the code, but it's also true that one shouldn't be able to get rid of the 'all' identifier. That just sounds like a harder fix.

Rik <rik5>
Project Administrator
Tue 03 May 2016 03:12:34 PM UTC, comment #1:

A faster way to reproduce the error (that does not involve object comparison):

Guillaume <gyom>
Tue 03 May 2016 03:01:18 PM UTC, original submission:

The following lines of code remove the warning state "all":

This generates a segmentation fault when comparing objects with isequal/isequaln:

According to the comment in error.cc, the warning state "all" should always be present.

Guillaume <gyom>

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

Digest:
   bug dependencies.

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 lachlan (Posted a comment)
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by rik5
  • -email is unavailable- added by gyom (Submitted 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 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-07-15 rik5 StatusReady For Test => Fixed
        Open/ClosedOpen => Closed
    2016-07-15 jwe StatusPatch Submitted => Ready For Test
    2016-07-01 rik5 Severity3 - Normal => 4 - Important
    2016-05-04 rik5 Assigned toNone => lachlan
        Dependencies- => Depends on bugs #47543
    2016-05-04 lachlan StatusConfirmed => Patch Submitted
    2016-05-03 rik5 StatusNone => Confirmed
        Carbon-Copy- => Added jwe

    Back to the top


    Powered by Savane 3.4