bugGNU Octave - Bugs: bug #53156, examples/code/make_int.cc produces...

 
 

bug #53156: examples/code/make_int.cc produces a segfault at exit of Octave

Submitter:  Carlo de Falco <cdf>
Submitted:  Wed 14 Feb 2018 04:10:26 PM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Other
Status:  Fixed Assigned to:  cdf
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 21 Feb 2018 12:43:06 AM UTC, comment #13: 

I saw the changeset in the log:

http://hg.savannah.gnu.org/hgweb/octave/rev/5f445419559f

and wondered about the strange comment.  Seems to be making things fragile to the point that future programmers will avoid a proper fix or even know to look for a bug because it's covered up.  (Also, I think there may be some compiler/OSes that complain at exit if there is some unfreed memory.  Microsoft and latest Windows systems comes to mind.)  Is there a crash prior to this make_int.cc for some other example?  Or is it only this code where there is a known crash?  Maybe it is the code.

In that regard, looking at what's attached to this bug report, just a couple questions from a consistency standpoint.  I see in the make_int.cc


DEFUNOP_OP (gnot, integer, !)
DEFUNOP_OP (uminus, integer, -)
DEFUNOP_OP (transpose, integer, /* no-op */)
DEFUNOP_OP (hermitian, integer, /* no-op */)

[snip]

      INSTALL_UNOP_TI (ti, op_not, octave_integer, gnot);
      INSTALL_UNOP_TI (ti, op_uminus, octave_integer, uminus);
      INSTALL_UNOP_TI (ti, op_transpose, octave_integer, transpose);
      INSTALL_UNOP_TI (ti, op_hermitian, octave_integer, hermitian);


There is a consistency there in the way what looks like four operators are defined.  Yet, looking at the implementations of these operators there is


  octave_value gnot (void) const { return octave_value ((double) ! scalar); }

  octave_value uminus (void) const { return new octave_integer (- scalar); }

  octave_value transpose (void) const { return new octave_integer (scalar); }

  octave_value hermitian (void) const { return new octave_integer (scalar); }


which isn't consistent.  One operator does not make use of "new" while three of them do.

Higher in the header portion of the patch is use of a "new"


  octave_base_value * clone (void) { return new octave_integer (*this); }


which makes sense to me that it return a pointer because of the "new" operation, while three operators mentioned above aren't returning pointers.

Dan Sebald <sebald>
Wed 21 Feb 2018 12:32:21 AM UTC, comment #12: 

It may not be the ultimate fix, but the patch works for me.  Marking as fixed and closing report.

Rik <rik5>
Group administrator
Tue 20 Feb 2018 08:00:49 PM UTC, comment #11: 

I checked in the following changeset.

  http://hg.savannah.gnu.org/hgweb/octave/rev/5f445419559f

Although it avoids the crash for me, but it is definitely not a proper fix.  The core issue is that we still don't have resource management right in the interpreter.

The situation is made more complex by things like dynamically loaded data types and classdef objects that might run destructors.  I'm not sure what the best solution is.  I'd like to be able to have all resource initialization for the interpreter handled by the interpreter constructor and all resource cleanup be handled by the corresponding destructor.  Likewise for all the objects that make up the interpreter.  It seems like that should be possible, but we are definitely not there yet.

John W. Eaton <jwe>
Group administrator
Mon 19 Feb 2018 11:39:44 PM UTC, comment #10: 

Re-titling report since the remaining issue is a segfault when Octave is exiting.

Steps to reproduce:


# At shell
mkoctfile make_int.cc
octave
# Within Octave
x = make_int (1.2)
exit



Rik <rik5>
Group administrator
Fri 16 Feb 2018 10:14:06 PM UTC, comment #9: 
Carlo de Falco <cdf>
Group Member
Fri 16 Feb 2018 09:09:38 PM UTC, comment #8: 

Yeah, just make the changes on default.

I'll try to take a look at the one on stable before 4.2.2 to see whether there is anything that can be done.

John W. Eaton <jwe>
Group administrator
Fri 16 Feb 2018 09:06:04 PM UTC, comment #7: 

Go ahead and check in your change on the development branch.  I think the segfault on exit is probably because make-int.cc is not unregistering itself correctly, but that can be fixed later.

Rik <rik5>
Group administrator
Fri 16 Feb 2018 05:52:32 PM UTC, comment #6: 

As 4.2.2 is coming out soon I checked on the stable branch and I see the same bug there.
CC-ing jwe in case he sees an easy fix that can be applied before the release, but this is of course not high priority.


Carlo de Falco <cdf>
Group Member
Wed 14 Feb 2018 10:28:56 PM UTC, comment #5: 

The file now compiles fine, and appears to run okay, but there is a segmentation fault at the exit of Octave.


octave:1> diary on
octave:2> x = make_int(1.2)
installing integer type at type-id = 56
x =

1

octave:3> y = make_int (1.9)
y =

2

octave:4> z = x + y
z =

3

octave:5> whos
Variables in the current scope:

   Attr Name        Size                     Bytes  Class
   ==== ====        ====                     =====  =====
        x           0x0                          0  integer
        y           0x0                          0  integer
        z           0x0                          0  integer

Total is 0 elements using 0 bytes

octave:6> exit
fatal: caught signal Segmentation fault -- stopping myself...
Segmentation fault


The segmentation fault may have been there for a long time, but it should be fixed.

Rik <rik5>
Group administrator
Wed 14 Feb 2018 09:47:21 PM UTC, comment #4: 

mmmh... the fix for the issue of comment #3 was trivial
and I think this was a bug in make_int.cc unrelated to the interpreter refactoring.




(file #43325)

Carlo de Falco <cdf>
Group Member
Wed 14 Feb 2018 09:41:06 PM UTC, comment #3: 

no, it doesn't do the the right thing.


>> a = make_int (pi)
installing integer type at type-id = 55
a =

3

>> a = make_int (pi)
warning: duplicate type integer
installing integer type at type-id = 55
duplicate unary operator '!' for type 'integer'
fatal: caught signal Abort trap: 6 -- stopping myself...
Abort trap: 6


Carlo de Falco <cdf>
Group Member
Wed 14 Feb 2018 09:37:29 PM UTC, comment #2: 

Thanks,
the attached version gets rid of all
the warnings at compile time, but I'm
not sure yet that it does the right thing.


(file #43324)

Carlo de Falco <cdf>
Group Member
Wed 14 Feb 2018 05:34:38 PM UTC, comment #1: 

@Carlo: Attached is a modified version of make_int.cc which at least compiles.  I mostly updated the #include list and replaced calls to deprecated functions with their new replacements.

There are still a lot of warnings, and I don't know if it does the right thing, but I will leave the rest of the optimization to you.

(file #43315)

Rik <rik5>
Group administrator
Wed 14 Feb 2018 04:10:26 PM UTC, original submission:  

Hi,

While trying to understand how to create a custom octave
class in C++ after the interpreter refactoring,
I noticed that the example Wmake_int.cc" does
not work anymore with the development release.

I'd be happy to help fixing it but I do need directions.

Carlo de Falco <cdf>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #43325:  make_int.cc added by cdf (9KiB - application/octet-stream)
file #43324:  make_int.cc added by cdf (9KiB - application/octet-stream)
file #43315:  make_int.cc added by rik5 (9KiB - text/x-c++src)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by sebald (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by cdf
  • -email is unavailable- added by rik5 (Updated the item)
  • -email is unavailable- added by cdf (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 12 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-02-21 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2018-02-20 jwe StatusConfirmed Ready For Test
    2018-02-19 rik5 StatusReady For Test Confirmed
        Summaryexamples/code/make_int.cc does not compile with development release examples/code/make_int.cc produces a segfault at exit of Octave
    2018-02-16 cdf Carbon-Copy- Added -email is unavailable-
    2018-02-14 cdf Attached File- Added make_int.cc, #43325
        StatusIn Progress Ready For Test
        Assigned toNone cdf
    2018-02-14 cdf Attached File- Added make_int.cc, #43324
    2018-02-14 rik5 Attached File- Added make_int.cc, #43315
        StatusNone In Progress

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code