bugGNU Octave - Bugs: bug #53653, colloc crash

 
 

bug #53653: colloc crash

Submitter:  Ceral Paquet <octavebugs>
Submitted:  Sat 14 Apr 2018 12:08:32 PM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Segfault, Bus Error, etc.
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * 4.2.1 Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 15 Apr 2018 05:10:34 PM UTC, comment #6: 

I pushed a final cset to add input validation BIST tests (http://hg.savannah.gnu.org/hgweb/octave/rev/75eea4768e24).

I will file a separate report about potentially eliminating assert statements.

Rik <rik5>
Group administrator
Sun 15 Apr 2018 12:49:11 AM UTC, comment #5: 

Clearly this isn't a frequently-used function.  I added 4 BIST tests for functionality all around n = 1.  There are no tests yet for the rest of the input validation or for more extended cases (n > 1).

Rik <rik5>
Group administrator
Sat 14 Apr 2018 11:12:17 PM UTC, comment #4: 

Should we add some tests to call this function?

Mike Miller <mtmiller>
Group Member
Sat 14 Apr 2018 05:51:08 PM UTC, comment #3: 

Oops.  Thanks for the fix.  Amazing that it took 8 years to discover this mistake.

I'm neutral on keeping vs. removing asserts.  If we aren't getting reports about them, then probably they can be removed.  But it also doesn't cost much to keep them.

John W. Eaton <jwe>
Group administrator
Sat 14 Apr 2018 04:15:11 PM UTC, comment #2: 

Adding jwe to the CC list since it appears that it as this cset that introduced the problem


changeset:   10603:d909c4c14b63
user:        John W. Eaton <jwe@octave.org>
date:        Tue May 04 13:00:00 2010 -0400
summary:     convert villad functions to C++


For reference, this isn't actually a segfault due to a memory overrun, but rather the failure of a C assert statement.  Using the 4.3.90 release candidate,


octave:1> colloc (1)
octave-cli: liboctave/numeric/CollocWt.cc:156: bool jcobi(octave_idx_type, octave_idx_type, octave_idx_type, double, double, double*, double*, double*, double*): Assertion `nt > 1' failed.
fatal: caught signal Aborted -- stopping myself...
Abort


At least the message is clear about where the issue is and what the problem is.  Looking at the code around line 156 it is


static bool
jcobi (octave_idx_type n, octave_idx_type n0, octave_idx_type n1,
       double alpha, double beta, double *dif1, double *dif2,
       double *dif3, double *root)
{
  assert (n0 == 0 || n0 == 1);
  assert (n1 == 0 || n1 == 1);

  octave_idx_type nt = n + n0 + n1;

  assert (nt > 1);


nt is 1 in this case which is the problem.  On the other hand, the original Fortran for this routine which was converted in cset 10603 is


-C
-      IF ((N + N0 + N1) .LT. 1) THEN
-        IER   = 7
-        LSTOP = .TRUE.
-        CALL VILERR(IER,LSTOP)
-      ELSE
-      END IF
-C


It seems like the actual error condition is


  if (nt < 1) then


which can be negated for the assert statement to


assert (nt >= 1);


I checked in a small cset to do that: http://hg.savannah.gnu.org/hgweb/octave/rev/db31e068f4db.

I suppose the next question is whether we really need assert statements here.  Wouldn't it be better to call liboctave_error_handler and allow the interpreter to continue rather than crashing out of the program?

Maybe we should grep for other asserts in liboctave.

Rik <rik5>
Group administrator
Sat 14 Apr 2018 12:20:58 PM UTC, comment #1: 

Confirmed.  It doesn't crash back in version 3.2.4, but from 3.4.3 to the present it does segfault.

Rik <rik5>
Group administrator
Sat 14 Apr 2018 12:08:32 PM UTC, original submission:  

colloc(1) crashes octave.

Ceral Paquet <octavebugs>

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by rik5
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by octavebugs (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 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-04-15 rik5 StatusConfirmed Fixed
        Open/ClosedOpen Closed
    2018-04-14 rik5 Carbon-Copy- Added jwe
    2018-04-14 rik5 CategoryNone Interpreter
        Item GroupNone Segfault, Bus Error, etc.
        StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code