bugGNU Octave - Bugs: bug #35857, Octave crashes when plotting with...

 
 

bug #35857: Octave crashes when plotting with units = normalized

Submitted by:  Muhali <muhali>
Submitted on:  Thu 15 Mar 2012 09:13:36 PM UTC  
 
Category: PlottingSeverity: 5 - Blocker
Priority: 5 - NormalItem Group: Segfault, Bus Error, etc.
Status: FixedAssigned to: None
Originator Name: Open/Closed: Closed
Release: devOperating System: GNU/Linux

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 Jump to the original submission)

Sun 26 Apr 2015 02:33:54 PM UTC, comment #21:

Marking bug as fixed and closing report.

Rik <rik5>
Project Administrator
Sun 26 Apr 2015 02:28:42 PM UTC, comment #20:

OK to close this report?

John W. Eaton <jwe>
Project Administrator
Wed 08 Apr 2015 08:52:42 PM UTC, comment #19:

I pushed my change here:

http://hg.savannah.gnu.org/hgweb/octave/rev/10600b2dd3c1

John W. Eaton <jwe>
Project Administrator
Wed 08 Apr 2015 04:42:19 PM UTC, comment #18:

I'll do it soon.

John W. Eaton <jwe>
Project Administrator
Wed 08 Apr 2015 04:00:11 PM UTC, comment #17:

I verified the patch. I'd commit it so we can finally get to releasing 4.0.

Rik <rik5>
Project Administrator
Wed 08 Apr 2015 03:36:07 PM UTC, comment #16:

If someone can verify that the patch linked in comment #15 works, then I'll add a commit message and push the changeset.

John W. Eaton <jwe>
Project Administrator
Wed 08 Apr 2015 03:34:28 PM UTC, comment #15:

I'm attaching another patch. It attempts to fix the label positioning for all possible unit values and also correctly inherit defaults for the axes labels.

Inheriting defaults wasn't working for the axes labels because they are created inside the axes constructor and so the parent (axes) object wasn't fully constructed and inserted into the gh_manager map of handles -> objects, so looking up parent objects could not work correctly.

I'm still not sure that what I've done in the attached patch is the best solution, but it seems to work for me.

(file #33597)

John W. Eaton <jwe>
Project Administrator
Mon 06 Apr 2015 12:58:02 AM UTC, comment #14:

definitely, yes. no longer blocker!

Michael Godfrey <godfrey>
Project Member
Mon 06 Apr 2015 12:45:52 AM UTC, comment #13:

I think I like the first patch better. With the second patch, if I do a mouse click after the plot() command but before the title() command it resizes the axes to some strange value.

With the crash fixed I think we can downgrade this to "Incorrect Result" and change the Severity back to normal so this bug doesn't delay the 4.0 release.

Rik <rik5>
Project Administrator
Sat 04 Apr 2015 08:47:04 PM UTC, comment #12:

OK, I'm attaching a new patch. The only change is to text::properties::update_units so that an auto positionmode property is preserved. That at least makes the units not be (0,0,0), but it does not make them normalized. Instead, they still seem to be the "data" values. I tried to add a call to convert_position in the axes::properties::update_title_position function but I don't seem to know what I'm doing with the conversions/transformations. Maybe someone who understands this code better can see what to do.

(file #33539)

John W. Eaton <jwe>
Project Administrator
Sat 04 Apr 2015 05:54:25 PM UTC, comment #11:

@jwe: The diff prevents the segfault, but the title and [xyz]labels all show up at position (0,0,0). The original code shows the problem.

The ordinary code, assuming no defaulttextunits, works correctly

By setting some breakpoints I can see that the position field is updated in normal 'data' mode. (It starts with update_axes_layout->update_ticklength->update_xlabel_position->set_position). In fact, the first initialization() which is called through _go_axes_ works fine. It is the subsequent call to _go_axes_init_ which deletes the label objects and then re-creates them that causes the problem. The value in the override of the position is definitely (0,0,0).

Backtrace is:

More to the point,

If I change 'defaultextposition' to something reasonable for normalized units ([0.5, 0.5, 0]) then the label shows up correctly (middle of screen).

I tried

but this may be meant for something else because it set the position value in "data" units completely ignoring the "normalized" value on xlabel.

Rik <rik5>
Project Administrator
Sat 04 Apr 2015 07:54:59 AM UTC, comment #10:

If it may help there is a method that lets you retrieve parent defaults:

I was planning to rewrite "override_defaults" using this (in the future).

Pantxo Diribarne <pantxo>
Project Member
Fri 03 Apr 2015 11:31:18 PM UTC, comment #9:

Rik, thanks for all that info. I was headed in that direction, but I think you definitely saved me some time.

I'm attaching a patch and hoping that the following explains it at least a little.

OK, so there are a couple of things happening here.

One is that we call

from inside constructors and then expect to be able to get the actual type of "this", but since "this" is being used before it is fully constructed, it is just a pointer to the base type. I guess C++ wizards would know that you can't dispatch to virtual functions from inside a constructor?

So that makes the call to valid_object in override_defaults return false because it is always the function from the base class. Oops. My patch avoids this problem by moving the call to override_defaults outside of the constructor. But I think a better solution would be a way to apply defaults from the parent objects from inside the constructor.

The second issue was that we were doing something like this:

but while constructing the new text object, we tried to update positions of xlabel, ylabel, etc. That failed because we were accessing the xlabel object that was no longer valid, but apparently the handle was still accessible somehow. In any case, I worked around this problem by first invalidating the handles, then constructing them. At the point inside the construction of these objects where the position is updated, I just skip that if the handles are not valid. Then I call the update functions in the axes::properties::set_defaults function after xlabel, ylabel, etc. are otherwise fully initialized.

Please take a look at the patch. I'm sure it could be improved, but it seems to avoid the crash for me.

(file #33523)

John W. Eaton <jwe>
Project Administrator
Fri 03 Apr 2015 07:52:56 PM UTC, comment #8:

I've single-stepped through a lot of code and figured a bit about what is happening.

Using the original example:

The first call to set is fine. The plot call invokes newplot() to prepare the figure and axes. At the end of newplot this involves calling

The backtrace of interest is

The initial position returned in pos is (0,0,0). The value of pos after convert_text_position() is an empty matrix. The subsequent call of set_position (pos) then causes a chain of errors.

The top of the convert_text_position() function is shown below.

What happens is that ax.valid_object () returns false, and so retval assumes the default value of 2-D empty matrix. Why is the axes not valid? That is the question. I hacked a solution by using an else clause off the if statement and returning the input pos if the axis is not valid. This works and stops the segfault, but doesn't put the labels in the correct location.

The chain starts in axes::properties::set_defaults in graphics.cc. The first thing the code does is delete all the axes children. This succeeds. It is the first call to make_graphics_handle that fails. Maybe the problem is that somehow the adoption needs to take place first in order for get_ancestor ("axes") to return a valid object.

Rik <rik5>
Project Administrator
Thu 19 Feb 2015 12:27:36 PM UTC, comment #7:

Hi,

@Rik: the crash also occurs for me with absolute coordinates such as "centimeters". The "units" property is supposed to be used for interpreting the position, not the size of the text object.

I still don't know why but it is related to deleting/recreating labels and title when using newplot function on an axes object that has "nextplot" = "replace".

The following doesn't crash Octave but doesn't set the expected units neither.

Now another question is whether default text object properties should be applied to axes labels and titles. Those object are not meant to be manipulated the same way as regular text objects. What if someone changes the "defaulttextposition" ... I'll check this in Matlab at work.

Pantxo Diribarne <pantxo>
Project Member
Sun 15 Feb 2015 03:58:56 PM UTC, comment #6:

This still crashes Octave, but the code combination is rare. Most people use an absolute size such as points, inches, or centimeters when setting sizes. I'm not going to require that this be fixed before the 4.0 release.

Rik <rik5>
Project Administrator
Fri 30 Jan 2015 04:34:51 PM UTC, comment #5:

I am raising the severity and changing this bug back to a crash, because it now does crash the Octave GUI using Qt plotting.

The errors remain for both Gnuplot and FLTK based plotting.

Mike Miller <mtmiller>
Project Administrator
Fri 11 Oct 2013 04:56:13 PM UTC, comment #4:

After this changeset fixing validation of properties to set (http://hg.savannah.gnu.org/hgweb/octave/rev/94dd9bba06a0) I no longer get a segfault. Octave still issues horrible error messages and won't plot anything properly until you remove the "normalized" property (or set it to "data") with

Still, this no longer leads to a crash so I'm changing the Item Group to "Incorrect Result" and this will no longer stop the 3.8 release.

Rik <rik5>
Project Administrator
Sun 25 Mar 2012 08:44:42 PM UTC, comment #3:

Bug #35856 has been fixed on the development branch but the code in this report still crashes Octave. Running under gdb and looking at the crash, this seems to be related to default properties and order of initialization. I'm going to mark this postponed as Michael will be looking into that at the end of the summer.

Rik <rik5>
Project Administrator
Fri 16 Mar 2012 12:02:13 AM UTC, comment #2:

(opps) I expect this is a more severe example of

https://savannah.gnu.org/bugs/?35856

Ben Abbott <bpabbott>
Project Member
Fri 16 Mar 2012 12:00:41 AM UTC, comment #1:

I expect this is a more severe example of

Ben Abbott <bpabbott>
Project Member
Thu 15 Mar 2012 09:13:36 PM UTC, original submission:

This code

crashes octave (using bec37a92cb3b tip).

Muhali <muhali>

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #33597:  diffs-3.txt added by jwe (12KiB - text/plain)
file #33539:  diffs-2.txt added by jwe (9KiB - text/plain)
file #33523:  diffs.txt added by jwe (8KiB - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by godfrey (Posted a comment)
  • -unavailable- added by jwe (Updated the item)
  • -unavailable- added by pantxo (Posted a comment)
  • -unavailable- added by mtmiller (Posted a comment)
  • -unavailable- added by rik5 (Posted a comment)
  • -unavailable- added by rik5
  • -unavailable- added by bpabbott (Posted a comment)
  • -unavailable- added by jordigh (Updated the item)
  • -unavailable- added by muhali (Submitted the item)
  •  

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

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 14 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 26 Apr 2015 02:33:54 PM UTCrik5StatusPatch Submitted=>Fixed
      Open/ClosedOpen=>Closed
    Wed 08 Apr 2015 03:36:07 PM UTCjweStatusConfirmed=>Patch Submitted
    Wed 08 Apr 2015 03:34:28 PM UTCjweAttached File-=>Added diffs-3.txt, #33597
    Sat 04 Apr 2015 08:47:04 PM UTCjweAttached File-=>Added diffs-2.txt, #33539
    Fri 03 Apr 2015 11:31:18 PM UTCjweAttached File-=>Added diffs.txt, #33523
    Fri 30 Jan 2015 04:35:16 PM UTCmtmillerSummarycrash with units = normalized=>Octave crashes when plotting with units = normalized
    Fri 30 Jan 2015 04:34:51 PM UTCmtmillerSeverity3 - Normal=>5 - Blocker
      Item GroupIncorrect Result=>Segfault, Bus Error, etc.
      StatusPostponed=>Confirmed
    Fri 11 Oct 2013 04:56:13 PM UTCrik5Item GroupSegfault, Bus Error, etc.=>Incorrect Result
    Sun 25 Mar 2012 08:44:42 PM UTCrik5StatusConfirmed=>Postponed
      Carbon-Copy-=>Added -unavailable-
    Thu 15 Mar 2012 09:19:30 PM UTCjordighStatusNone=>Confirmed

    Back to the top


    Powered by Savane 3.1-cleanup1