bugGNU Octave - Bugs: bug #60473, rmdir called with empty string...

 
 

bug #60473: rmdir called with empty string causes gui to crash on windows

Submitted by:  Nicholas Jankowski <nrjank>
Submitted on:  Thu 29 Apr 2021 05:14:18 AM UTC  
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Segfault, Bus Error, etc.
Status:  Fixed Assigned to:  None
Originator Name:  Nicholas Jankowski Open/Closed:  Closed
Release:  6.2.0 Operating System:  Microsoft Windows

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)

Fri 30 Apr 2021 02:51:10 PM UTC, comment #7: 

FYI pulled down the latest version of Octave 6.2.1 [hg id: 6fc423987872, Fri, Apr 30, 2021] off the buildbot and can confirm on windows that I no longer get the crash on rmdir("").  absent any other concerns i think the bug can be closed as fixed.

Nicholas Jankowski <nrjank>
Project Member
Thu 29 Apr 2021 08:19:00 PM UTC, comment #6: 

The interpreter notifies the GUI when a directory is removed.  Then a Qt signal is emitted and the editor function that responds to the signal was trying to remove double quote characters (if any) at the beginning and end of the file.  But there was no check there for the file name being empty so there was an attempt to index a QString object with an out of bounds index and that's what caused the crash.

John W. Eaton <jwe>
Project Administrator
Thu 29 Apr 2021 07:00:05 PM UTC, comment #5: 

@ttl:  no.  i tried this a few times with the editor closed, open with files, open with no open files, docked, floating, etc., to see if any changed the behavior.  in all cases rmdir("") crashed (not just closed nicely) the entire gui

Nicholas Jankowski <nrjank>
Project Member
Thu 29 Apr 2021 06:05:58 PM UTC, comment #4: 

The GUI checks whether open editor files are located in the  removed directories and thus should be silently closed before the editor shows the warning dialog that the files were externally removed. Were there any open files in the editor when rmdir("") triggered the crash?

Torsten Lilge <ttl>
Project Member
Thu 29 Apr 2021 05:23:10 PM UTC, comment #3: 

will see if the fix makes a difference on my windows build after the next buildbot cycle.

curious, i did see that the number of previous rmdir bugs referred to closing editor tabs, and your patch seems to be tied to an editor fix, but in my case the editor wasn't (obviously) involved.   it's still related under the hood and causes a general Qt crash of the whole gui?

Nicholas Jankowski <nrjank>
Project Member
Thu 29 Apr 2021 10:42:32 AM UTC, comment #2: 

We might also be able to improve the crash behavior.  For me, the stack trace is

#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007ff55ba75537 in __GI_abort () at abort.c:79
#2  0x00007ff55a211bf1 in QMessageLogger::fatal(char const*, ...) const () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#3  0x00007ff55a211040 in qt_assert(char const*, char const*, int) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#4  0x00007ff55e0e275a in QString::at (this=0x7ffdb2643b10, i=0) at /usr/include/x86_64-linux-gnu/qt5/QtCore/qstring.h:1071
#5  QString::at (i=<optimized out>, this=<optimized out>) at /usr/include/x86_64-linux-gnu/qt5/QtCore/qstring.h:1071
#6  octave::file_editor::handle_file_remove (this=0x55fba7404a80, old_name=..., new_name=...) at /home/jwe/src/octave/libgui/src/m-editor/file-editor.cc:1141

so maybe we could make the qt_assert or QMessageLogger::fatal functions provide more info before calling abort?

John W. Eaton <jwe>
Project Administrator
Thu 29 Apr 2021 10:37:48 AM UTC, comment #1: 

I pushed the following changeset on stable and merged with default:

http://hg.savannah.gnu.org/hgweb/octave/rev/6fc423987872

It fixes the problem for me with my Debian builds.

John W. Eaton <jwe>
Project Administrator
Thu 29 Apr 2021 05:14:18 AM UTC, original submission:  

Octave for windows, 64bit, zip package installer. verified on 6.2.0 and 6.2.1 (build hg id: 50aa945836c3 Mon, Apr 26, 2021).  not set up to build so have not verified on default branch. 

stumbled upon this in bug #60472, where pkg was reliably crashing to desktop. turns out some odd behavior was calling rmdir("","s").

can reliably recreate crash by loading the GUI and running:

rmdir("")

assuming it's a crash and not just a forced close or exit because octave doesn't retain any recent command history after the last normal session.

note that this doesn't happen when run from the CLI.  in than case you just get:

Nicholas Jankowski <nrjank>
Project Member

 

(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 rik5 (Updated the item)
  • -email is unavailable- added by ttl (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by nrjank (Submitted the item)
  •  

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

    Only project members can vote.

     

     

     

    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-04-30 rik5 StatusReady For Test => Fixed
        Open/ClosedOpen => Closed
    2021-04-29 jwe StatusNone => Ready For Test

    Back to the top


    Powered by Savane 3.7