bugGNU Octave - Bugs: bug #53330, [octave forge] (control) several...

 
 

bug #53330: [octave forge] (control) several functions crash Octave with SIGABRT from invalid free

Submitter:  Doug Stewart <dastew>
Submitted:  Mon 12 Mar 2018 08:34:02 PM UTC
   
 
Category:  Octave Package Severity:  4 - Important
Priority:  5 - Normal Item Group:  Regression
Status:  Fixed Assigned to:  None
Originator Name:  doug Stewart 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 02 Jan 2019 04:44:13 AM UTC, comment #15: 

Mike and Doug look occupied at the moment, so to make do until the next release of the control package, I made a replacement for dare.m that does not use the slicot library, but octave's own qz algorithm.

That should patch also dlqe and dlqr. To install I overwrote dare.m in the following location on my system:

/usr/share/octave/packages/control-3.1.0/dare.m

If you try it, comments welcome.  Enjoy.



octave:1> pkg load control
octave:2> dlqr(eye(4),eye(4),eye(4),eye(4))
ans =

   0.61803   0.00000  -0.00000   0.00000
  -0.00000   0.61803  -0.00000   0.00000
  -0.00000   0.00000   0.61803   0.00000
  -0.00000   0.00000  -0.00000   0.61803





(file #45832)

James B. Rawlings <jbraw>
Thu 20 Dec 2018 07:38:05 PM UTC, comment #14: 

Doug - nothing needs to be done here, this bug is fixed. Orion and James are asking you to make a new release of the control package, either 3.1.1 or 3.2.0, whatever it will be. Just update the DESCRIPTION and NEWS file and request a release on the sourceforge tracker, I need to do the same for the signal package.

Mike Miller <mtmiller>
Group Member
Thu 20 Dec 2018 07:23:18 PM UTC, comment #13: 

Mike : What needs to be done here.
I will try and do it if you guide me.

Doug Stewart <dastew>
Thu 20 Dec 2018 06:40:01 PM UTC, comment #12: 

With the release of Ubuntu 18.10, octave 4.4.1 is the default.

I understand that you have a fix for dlqr.  Can you please release it so that we don't have to knock off our own version.

Thanks.


telemark:4> octave
octave: X11 DISPLAY environment variable not set
octave: disabling GUI features
GNU Octave, version 4.4.1
Copyright (C) 2018 John W. Eaton and others.
This is free software; see the source code for copying conditions.
There is ABSOLUTELY NO WARRANTY; not even for MERCHANTABILITY or
FITNESS FOR A PARTICULAR PURPOSE.  For details, type 'warranty'.

Octave was configured for "x86_64-pc-linux-gnu".

Additional information about Octave is available at https://www.octave.org.

Please contribute if you find this software useful.
For more information, visit https://www.octave.org/get-involved.html

Read https://www.octave.org/bugs.html to learn how to submit bug reports.
For information about changes from previous versions, type 'news'.

octave:1> pkg load control
octave:2> dlqr(eye(4),eye(4),eye(4),eye(4))
free(): invalid next size (fast)
fatal: caught signal Aborted -- stopping myself...
Aborted (core dumped)

James B. Rawlings <jbraw>
Thu 08 Nov 2018 11:36:04 PM UTC, comment #11: 

can we please get a new release with these fixes?

Orion Poplawski <opoplawski>
Tue 20 Mar 2018 03:25:17 PM UTC, comment #10: 

I'm surprised that it ever worked with OCTAVE_LOCAL_BUFFER (bool, ...) since the size of bool in C++ is typically 1 byte, but Fortran 77 requires that LOGICAL is the same size as INTEGER and REAL, which would typically be 4 bytes.

John W. Eaton <jwe>
Group administrator
Tue 13 Mar 2018 08:49:41 PM UTC, comment #9: 

Doug - try to remember post the URLs to the commits that fix an issue.

In this case

Mike Miller <mtmiller>
Group Member
Tue 13 Mar 2018 08:48:26 PM UTC, comment #8: 

Looks good to me, closing.

Mike Miller <mtmiller>
Group Member
Tue 13 Mar 2018 08:17:47 PM UTC, comment #7: 

This can be closed    --- no more hard crashes.

This same bug might affect other packages.

Doug Stewart <dastew>
Tue 13 Mar 2018 06:35:05 PM UTC, comment #6: 

Thank you Mike.
I will do the changes and report back.
give me a day or two.

Doug Stewart <dastew>
Tue 13 Mar 2018 06:25:32 PM UTC, comment #5: 

For full exhibition, I debugged this by installing the libblas3-dbgsym and liblapack3-dbgsym packages from Debian, which add debug symbols for their internal functions. I also installed the lapack Debian source package so I could get exact line numbers.

I then ran Octave with valgrind again and got the exact LAPACK source file and line number where the invalid write took place (dgees.f:408). And it was obvious that it was trying to write into the LOGICAL BWORK array that was passed in.

Mike Miller <mtmiller>
Group Member
Tue 13 Mar 2018 06:21:49 PM UTC, comment #4: 

The integers are not the problem. I see that jwe went though last year and fixed all of the integer references in the control package.

I think the remaining problem is the Fortran LOGICAL arrays called BWORK.

When I make the following changes to two of the SLICOT wrapper functions, the tests for h2syn now pass without crashing Octave.


diff --git a/src/sl_sb10ed.cc b/src/sl_sb10ed.cc
--- a/src/sl_sb10ed.cc
+++ b/src/sl_sb10ed.cc
@@ -47,7 +47,7 @@ extern "C"
                   double& TOL,
                   F77_INT* IWORK,
                   double* DWORK, F77_INT& LDWORK,
-                  bool* BWORK,
+                  F77_LOGICAL* BWORK,
                   F77_INT& INFO);
 }

@@ -113,7 +113,7 @@ For internal use only.")

         OCTAVE_LOCAL_BUFFER (F77_INT, iwork, liwork);
         OCTAVE_LOCAL_BUFFER (double, dwork, ldwork);
-        OCTAVE_LOCAL_BUFFER (bool, bwork, 2*n);
+        OCTAVE_LOCAL_BUFFER (F77_LOGICAL, bwork, 2*n);

         // error indicator
         F77_INT info;
diff --git a/src/sl_sb10hd.cc b/src/sl_sb10hd.cc
--- a/src/sl_sb10hd.cc
+++ b/src/sl_sb10hd.cc
@@ -47,7 +47,7 @@ extern "C"
                   double& TOL,
                   F77_INT* IWORK,
                   double* DWORK, F77_INT& LDWORK,
-                  bool* BWORK,
+                  F77_LOGICAL* BWORK,
                   F77_INT& INFO);
 }

@@ -110,7 +110,7 @@ For internal use only.")

         OCTAVE_LOCAL_BUFFER (F77_INT, iwork, max (2*n, n*n));
         OCTAVE_LOCAL_BUFFER (double, dwork, ldwork);
-        OCTAVE_LOCAL_BUFFER (bool, bwork, 2*n);
+        OCTAVE_LOCAL_BUFFER (F77_LOGICAL, bwork, 2*n);

         // error indicator
         F77_INT info;


I think there must be some optimization going on with the case of a C++ bool and the changes that were made to the OCTAVE_LOCAL_BUFFER macro. You must use the F77_LOGICAL type now for any Fortran LOGICAL arguments, return values, or in this case, scratch arrays, that are passed in to a Fortran routine.

Make that change to all of the SLICOT C++ wrapper routines and you should be ok.

Mike Miller <mtmiller>
Group Member
Tue 13 Mar 2018 12:46:48 PM UTC, comment #3: 

One more thought.
Default is now 64 bit. The same make file is used to install control pkg. on stable and on default.

Are we sure that the .cc code and the fortran code is being compiled properly using the same make file for both stable and default?


Doug Stewart <dastew>
Tue 13 Mar 2018 01:13:21 AM UTC, comment #2: 

@Mike
   If you run the same test in stable it does not crash---- so if you stop at
 the line that calls the fortran code and see what size the variables are in
 the stable version , and do the same in default version, then you should
  be able to see where the problem is.
 
This is what i would do if I knew how :-(
thanks for helping!

Doug

Doug Stewart <dastew>
Mon 12 Mar 2018 10:50:06 PM UTC, comment #1: 

I'm retitling the bug report to update the description and make it easier to find.

I can confirm this error with control 3.1.0 on Octave's default branch. This does not affect Octave 4.2.1.

The error message indicates that libc's internal heap bookkeeping slots have been messed with. So gdb isn't going to be much help here, since it just reports the state of the program when the crash occurs, which is already too late.

The valgrind output is more helpful, but what's needed next is some reading of the code at the faulty module.

The following command shows the quoted Valgrind excerpts. I can provide the full Valgrind log if needed.


$ LD_LIBRARY_PATH=/usr/lib/x86_64-linux-gnu/blas:/usr/lib/x86_64-linux-gnu/lapack OMP_NUM_THREADS=1 ./run-octave -cli -valgrind -W -q --eval 'pkg load control; test h2syn'
...
==1907== Invalid write of size 4
==1907==    at 0xB26D08F: dgees_ (in /usr/lib/x86_64-linux-gnu/lapack/liblapack.so.3.7.1)
==1907==    by 0x1C660213: sb02rd_ (SB02RD.f:802)
==1907==    by 0x1C68DE52: sb10vd_ (SB10VD.f:332)
==1907==    by 0x1C67FD05: sb10hd_ (SB10HD.f:373)
==1907==    by 0x1C5D7F24: F__sl_sb10hd__(octave_value_list const&, int) (sl_sb10hd.cc:120)
...
==1907==  Address 0x1b19153c is 0 bytes after a block of size 12 alloc'd
==1907==    at 0x4C2D91F: operator new[](unsigned long) (vg_replace_malloc.c:423)
==1907==    by 0x1C5D7DA0: make_unique<bool []> (unique_ptr.h:831)
==1907==    by 0x1C5D7DA0: F__sl_sb10hd__(octave_value_list const&, int) (sl_sb10hd.cc:113)
...
==1907== Invalid read of size 4
==1907==    at 0xB3B4C44: dtrsen_ (in /usr/lib/x86_64-linux-gnu/lapack/liblapack.so.3.7.1)
==1907==    by 0xB26D15A: dgees_ (in /usr/lib/x86_64-linux-gnu/lapack/liblapack.so.3.7.1)
==1907==    by 0x1C660213: sb02rd_ (SB02RD.f:802)
==1907==    by 0x1C68DE52: sb10vd_ (SB10VD.f:332)
==1907==    by 0x1C67FD05: sb10hd_ (SB10HD.f:373)
==1907==    by 0x1C5D7F24: F__sl_sb10hd__(octave_value_list const&, int) (sl_sb10hd.cc:120)
...
==1907==  Address 0x1b19153c is 0 bytes after a block of size 12 alloc'd
==1907==    at 0x4C2D91F: operator new[](unsigned long) (vg_replace_malloc.c:423)
==1907==    by 0x1C5D7DA0: make_unique<bool []> (unique_ptr.h:831)
==1907==    by 0x1C5D7DA0: F__sl_sb10hd__(octave_value_list const&, int) (sl_sb10hd.cc:113)
...
valgrind: m_mallocfree.c:307 (get_bszB_as_is): Assertion 'bszB_lo == bszB_hi' failed.
valgrind: Heap block lo/hi size mismatch: lo = 80, hi = 0.
This is probably caused by your program erroneously writing past the
end of a heap block and corrupting heap metadata.  If you fix any
invalid writes reported by Memcheck, this assertion failure will
probably go away.  Please try that before reporting this as a bug.
...


If I get a few minutes I will look into the "__sl_sb10hd__" routine, but you are probably right that this will end up being related to Octave's changes with respect to Fortran integer sizes.

If your code is using octave_idx_type to pass integers to Fortran, it may need to be updated to use the F77_INT type. And that may require Octave 4.2 or later, unless you want to try to make the code conditional to continue supporting older versions of Octave.

Mike Miller <mtmiller>
Group Member
Mon 12 Mar 2018 08:34:02 PM UTC, original submission:  


This is different than a seg. fault. It does not say panic trying to save etc.

This is probably related to
http://lists.gnu.org/archive/html/octave-maintainers/2017-01/msg00011.html

This only happens with default and control-3.x.x
To see the first example install control pkg. and then run
test h2syn
or
test bstmodred



 The crash occures at lin 141 in h2syn.m
   It is a call to a .cc file which then calls a fortran subruotine.
  [ak, bk, ck, dk, rcond] = _sl_sb10hd_ (a, b, c, d, ncon, nmeas);
  The input variables are
  a  double 6x6
  b  double 6x5
  c  double 5x6
  d  double 5x5
  ncon double 1x1            function
  nmeas double 1x1           function


It also occures in line 281 of bstmodred. m
 [ar, br, cr, dr, nr, hsv, ns] = _sl_ab09hd_ (a, b, c, d, dt, scaled, job, nr, ordsel, alpha, beta, ...
                                            tol1, tol2);


It also crashes in hinfsyn.m at line 241
       [ak, bk, ck, dk, rcond] = _sl_sb10fd_ (a, b, c, d, ncon, nmeas, gmax);
 

It also crashes in moen4.m at line 240
  [sys, x0, info] = _slicot_identification_ ("moen4", nargout, varargin{:});
 
It also crashes in ncfsyn.m at line 383
[AK, BK, CK, DK, RCOND] = _sl_sb10id_ (A, B, C, D, FACTOR);

and at 296
  [ak, bk, ck, dk, rcond] = _sl_sb10id_ (a, b, c, d, factor);

And probably more places. I stopped testing, and figured that if we fix these the rest might be fixed.

Any help would be appreciated.
If you tell me what needs changed I can do the rest.

Doug

Doug Stewart <dastew>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #45832:  dare.m added by jbraw (4KiB - text/x-objcsrc)

 

Carbon-Copy List
  • -email is unavailable- added by jbraw (Posted a comment)
  • -email is unavailable- added by opoplawski (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by dastew (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 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-02-18 mmuetzel Dependencies- bugs #55712 is dependent
    2019-01-02 jbraw Attached File- Added dare.m, #45832
    2018-12-20 mtmiller Carbon-CopyRemoved 80942 -
    2018-07-02 mtmiller Dependencies- bugs #54221 is dependent
    2018-03-13 mtmiller StatusConfirmed Fixed
        Open/ClosedOpen Closed
    2018-03-12 mtmiller Severity3 - Normal 4 - Important
        StatusNone Confirmed
        SummaryHard crash when testing control pkg. [octave forge] (control) several functions crash Octave with SIGABRT from invalid free

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code