bugGNU Octave - Bugs: bug #44545, control package: impulse causes...

 
 

bug #44545: control package: impulse causes segfault when built with 64-bit indexing

Submitter:  Mike Miller <mtmiller>
Submitted:  Sun 15 Mar 2015 03:04:37 AM UTC
   
 
Category:  Octave Package Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Segfault, Bus Error, etc.
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * other Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sat 21 Mar 2015 06:20:36 PM UTC, comment #7: 

Yes, works for me with 64-bit Octave 4.0.0-rc1. All unit tests pass. Thanks!

FYI I see one warning testing with Octave 4.0.0-rc1, you can address it if you want, it only warns oncer per user session:


>> test h2syn
warning: nfields is obsolete and will be removed from a future version of Octave; please use numfields instead
PASSES 2 out of 2 tests


Mike Miller <mtmiller>
Group Member
Sat 21 Mar 2015 03:45:02 PM UTC, comment #6: 

I've just replaced int by octave_idx_type in all the C++ files and pushed the changes. It works for my 32bit Octave 3.8.2. Could you please check whether the control package passes its tests without segmentation faults on Octave with 64bit indices?

Lukas Reichlin <paramaniac>
Sat 21 Mar 2015 02:03:34 PM UTC, comment #5: 

Yes, octave_idx_type is a signed integer. It is a stand-in replacement for whatever type is needed to interface with Fortran functions expecing an integer type. So it defaults to int, but is int64_t if Octave is built with --enable-64 (and -fdefault-integer-8 is passed to the Fortran compiler).

Mike Miller <mtmiller>
Group Member
Sat 21 Mar 2015 12:58:42 PM UTC, comment #4: 

Thanks for your example!
Is octave_idx_type a signed or unsigned integer?
If octave_idx_type is an unsigned integer, replacing int by octave_idx_type could be problematic.

Lukas Reichlin <paramaniac>
Fri 20 Mar 2015 04:54:05 AM UTC, comment #3: 

Here's an example diff of the changes needed to make this work. I was able to get the simple impulse() example working with these minimal changes applied to mb05nd and td04ad.

(file #33409)

Mike Miller <mtmiller>
Group Member
Sun 15 Mar 2015 05:11:48 PM UTC, comment #2: 

Yes, I don't have a patch at the moment but I can definitely take a look at coming up with an example for this case. I probably won't be able to get to this for a while though.

I wouldn't call this a high priority, I just figured out how to build Octave with 64-bit indexes and was trying a few packages to see if they work.

Mike Miller <mtmiller>
Group Member
Sun 15 Mar 2015 04:10:13 PM UTC, comment #1: 

Hi Mike

Could you please point out where I need to change the file sl_td04ad.cc from the control package? I will then adapt the remaining C++ files accordingly.

Best regards,
Lukas

Lukas Reichlin <paramaniac>
Sun 15 Mar 2015 03:04:37 AM UTC, original submission:  

Calling the impulse function results in a segmentation fault when the control package is installed and used with Octave built with 64-bit indexing:


octave:1> s = tf (1, [1 1]);
octave:2> impulse (s)

Program received signal SIGSEGV, Segmentation fault.
0x00007fffe535e85e in td04ad_ ()
   from /home/mike/octave/control-2.8.0/x86_64-unknown-linux-gnu-api-v49+/__control_slicot_functions__.oct
(gdb) bt
#0  0x00007fffe535e85e in td04ad_ ()
   from /home/mike/octave/control-2.8.0/x86_64-unknown-linux-gnu-api-v49+/__control_slicot_functions__.oct
#1  0x00007fffe5274690 in F__sl_td04ad__ (args=..., nargout=<optimized out>)
    at sl_td04ad.cc:118
...


This is likely due to the C++ calls into the slicot library not using the octave_idx_type where it is needed. The slicot library is built correctly, the '-fdefault-integer-8' option is correctly passed via mkoctfile.

Mike Miller <mtmiller>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #33409:  control-idx.diff added by mtmiller (5KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by paramaniac (Posted a comment)
  • -email is unavailable- added by mtmiller (Submitted the item)
  • -email is unavailable- added by mtmiller
  •  

    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 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2015-03-21 mtmiller StatusPostponed Fixed
        Open/ClosedOpen Closed
    2015-03-20 mtmiller Attached File- Added control-idx.diff, #33409
    2015-03-15 mtmiller StatusNone Postponed
    2015-03-15 mtmiller Carbon-Copy- Added paramaniac

    Back to the top

    Powered by Savane 3.13-caa5.
    Corresponding source code