bugGNU Octave - Bugs: bug #50234, c++ file_stat with non existant...

 
 

bug #50234: c++ file_stat with non existant crashes octave in windows

Submitter:  John Donoghue <lostbard>
Submitted:  Mon 06 Feb 2017 01:45:10 AM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Segfault, Bus Error, etc.
Status:  Fixed Assigned to:  None
Originator Name:  JohnD Open/Closed:  * Closed
Release:  * 4.2.0 Operating System:  * Microsoft Windows
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

  Spam posted by _108826
Fri 24 Feb 2017 07:45:58 PM UTC, comment #26: 

I pushed the change on the stable branch here (http://hg.savannah.gnu.org/hgweb/octave/rev/a6e4157694ef).  Unfortunately, this missed the 4.2.1 bug fix release by a few hours.  I suppose this means anyone compiling 4.2.1 on Mac OS is going to need to apply this patch first.

Rik <rik5>
Group administrator
Fri 24 Feb 2017 07:31:43 PM UTC, comment #25: 

Rik, your new patch fixed the problem for me.

Ben Abbott <bpabbott>
Group Member
Fri 24 Feb 2017 07:06:25 PM UTC, comment #24: 

Seems like your C++ linker can't find the compiled code for the ~file_stat() function.  This may be due to over-aggresive optimization (the function is empty, and specified as being "inline", so clang may not be including it in the library of compiled code).  Try the attached patch which gets rid of the inline keyword on the file_stat destructor.

(file #39823)

Rik <rik5>
Group administrator
Fri 24 Feb 2017 06:55:00 PM UTC, comment #23: 

Rik/John, I'm seeing the error below on MacOS.


  CXXLD    libinterp/liboctinterp.la
libtool: warning: '/sw/lib/libpcre.la' seems to be moved
libtool: warning: '/sw/lib/gcc5/lib/gcc/x86_64-apple-darwin15.6.0/5.4.0/../../../libgfortran.la' seems to be moved
libtool: warning: '/sw/lib/gcc5/lib/gcc/x86_64-apple-darwin15.6.0/5.4.0/../../../libquadmath.la' seems to be moved
clang: warning: argument unused during compilation: '-pthread'
clang: warning: argument unused during compilation: '-pthread'
Undefined symbols for architecture x86_64:
  "octave::sys::file_stat::~file_stat()", referenced from:
      octave_fcn_handle::set_fcn(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) in liboctinterp.4.dylib-master.o
      get_file_line(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, unsigned long) in liboctinterp.4.dylib-master.o
      octave::base_parser::frob_function(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, octave_user_function*) in liboctinterp.4.dylib-master.o
      Fautoload(octave_value_list const&, int) in liboctinterp.4.dylib-master.o
      F__magick_write__(octave_value_list const&, int) in liboctinterp.4.dylib-master.o
      F__magick_finfo__(octave_value_list const&, int) in liboctinterp.4.dylib-master.o
      F__mkdir__(octave_value_list const&, int) in liboctinterp.4.dylib-master.o


My understanding of c++ is insufficient for fixing the problem. Can one of you propose a fix?

Ben Abbott <bpabbott>
Group Member
Mon 20 Feb 2017 05:12:33 AM UTC, comment #22: 

I essentially checked in your changeset along with some comments explaining reminding any future programmers not to move these functions back to the header file and citing this bug report (http://hg.savannah.gnu.org/hgweb/octave/rev/525c36d5f985).

Rik <rik5>
Group administrator
Thu 16 Feb 2017 05:25:42 PM UTC, comment #21: 

For reference, I also tried the file_stat code as original, but with the file_name being set in the constructor body with same bad result

John Donoghue <lostbard>
Group Member
Thu 16 Feb 2017 05:19:39 PM UTC, comment #20: 

Ok.  I'm looking at whether some other changes to the constructor and destructor for file_stat might prevent this.  But it takes an hour or so to test each iteration through MXE so maybe by the end of the day I might have something.

Rik <rik5>
Group administrator
Thu 16 Feb 2017 05:03:43 PM UTC, comment #19: 

I got gcc compiled ok and built a fresh install. I few minor changes were needed to Qt and wxwidgets to get them to compile

Running in windows however, the same code still crashes

John Donoghue <lostbard>
Group Member
Tue 14 Feb 2017 11:38:49 PM UTC, comment #18: 

Ive got some changes now that compiled build-gcc 6.3.0 ok - its currently compiling native-gcc, so may work.


To get a --with-gcc work, we may need to implement something similar to the plugins that mxe.cc is using and would be better for testing different versions at least.

John Donoghue <lostbard>
Group Member
Tue 14 Feb 2017 10:23:38 PM UTC, comment #17: 

I'm willing to work on newer compiler versions.

Would it be worthwhile to add an option like --with-gcc=VERSION?  I've considered it in the past but haven't had time to do it.

John W. Eaton <jwe>
Group administrator
Tue 14 Feb 2017 07:00:22 PM UTC, comment #16: 

For comment #13, removing the 'virtual' and it still works

For comment #15, we should be able to try a new compiler although it may take a while to get it working

John Donoghue <lostbard>
Group Member
Tue 14 Feb 2017 04:46:03 PM UTC, comment #15: 

Can we try a newer version of gcc to check if it is the compiler, rather than the code, which is at fault?  Aside from the point about the reference in comment #10, this looks like valid C++ code.

Rik <rik5>
Group administrator
Tue 14 Feb 2017 11:14:12 AM UTC, comment #14: 

Both are needed or it still crashes.

I hadnt tried without the virtual so will try that.

This is all using 4.9.4. We could try gcc 5 or 6 ? and see if there is other any additional issues that show up

John Donoghue <lostbard>
Group Member
Tue 14 Feb 2017 03:39:42 AM UTC, comment #13: 

I don't object to the patch, but since the destructor is trivial and the constructor is pretty simple and many constructors of even greater complexity are defined in header files, we should probably add a note about why these are defined in the .cc file.

Also, do both need to be moved?

Is the virtual declaration for the destructor required?

What version of GCC?  Is it 4.9.4?  Should we update the version used by mxe-octave or does that cause other trouble?

John W. Eaton <jwe>
Group administrator
Mon 13 Feb 2017 09:52:46 PM UTC, comment #12: 

This patch (attached) appears to work for me - not sure why its an issue otherwise.


(file #39750)

John Donoghue <lostbard>
Group Member
Fri 10 Feb 2017 04:28:32 PM UTC, comment #11: 

suggested change appears to make no difference.

John Donoghue <lostbard>
Group Member
Thu 09 Feb 2017 05:56:03 PM UTC, comment #10: 

Confirmed.  For completeness, I booted an old Windows XP VM and ran 4.2.0 to create a .oct file and it segfaults in the manner reported here.

Looking at the code in liboctave/system/file-ops.cc I see


    std::string
    octave::sys::file_ops::tilde_expand (const std::string& name)
    {
      if (name.find ('~') == std::string::npos)
        return name;
      else
        {
          std::string result;


I'm grasping here, but I see that name is passed in by reference, and that the function prototype declares this const which means it is making a promise not to change it.  However, tilde_expand just returns a std::string object.  When a '~' is found, the function declares a temporary "std::string result" and then proceeds to construct that string before ending with "return result;".

On the other hand, if no '~' is found then the function actually passes a reference to a string ("return name;") rather than a string object.  It is possible that eventually the reference is invalid but downstream functions which called tilde_expand continue to try and use it.

As a quick test, one might try altering the return statement to


return std::string (name);




Rik <rik5>
Group administrator
Thu 09 Feb 2017 12:37:12 AM UTC, comment #9: 

I tried that after I first saw it just on an offchance, but the error (and stack trace) was the same

John Donoghue <lostbard>
Group Member
Thu 09 Feb 2017 12:21:28 AM UTC, comment #8: 

Just out of curiosity, what happens if you rewrite it as


  file_stat fs ("nonexistentfile");
  bool ok = fs.exists ();
  return octave_value (ok);


?

John W. Eaton <jwe>
Group administrator
Thu 09 Feb 2017 12:06:59 AM UTC, comment #7: 

After catching the error and looking in gdb, the name looks invalid in update_internal, however looks ok up a step so I think the stack is getting messed up.

Im retrying with some print statements in it

John Donoghue <lostbard>
Group Member
Wed 08 Feb 2017 11:40:31 PM UTC, comment #6: 

The stack trace says that it is crashing in the call to find, here:


    std::string
    octave::sys::file_ops::tilde_expand (const std::string& name)
    {
      if (name.find ('~') == std::string::npos)
        return name;
      else


How?  What is the value of name at this point?

John W. Eaton <jwe>
Group administrator
Wed 08 Feb 2017 07:59:36 PM UTC, comment #5: 

That change does not fix it for me however, that change also needs to be applied to stable as well

John Donoghue <lostbard>
Group Member
Tue 07 Feb 2017 06:58:31 PM UTC, comment #4: 

I saw that - from the change, I don't think it will help but am trying a rebuild to verify.

John Donoghue <lostbard>
Group Member
Tue 07 Feb 2017 06:08:58 PM UTC, comment #3: 

I don't know if it is related, but there was a check-in yesterday, Feb. 6th, that fixes a problem with the stat wrappers.


changeset:   23159:a0eae3977ea6
bookmark:    @
user:        Markus Mützel <markus.muetzel@gmx.de>
date:        Mon Feb 06 16:50:01 2017 +0100
summary:     stat-wrappers.c (assign_stat_fields): Fix copy-paste errors.


Can you try building an MXE version from the development sources with a cset >= 23159?


Rik <rik5>
Group administrator
Tue 07 Feb 2017 05:25:35 PM UTC, comment #2: 

Anyone else running windows able to see the same issue?

John Donoghue <lostbard>
Group Member
Mon 06 Feb 2017 01:47:48 AM UTC, comment #1: 

On a similar run with symbols:



#0  0x00000000196c3bf0 in libstdc++-6!_ZNKSs4findEcy () from C:\Octave\OCTAVE~1.0\bin\libstdc++-6.dll
#1  0x000000006c5f0d78 in octave::sys::file_ops::tilde_expand (name=...)
    at /home/johnd/mxe-octave/build-64/tmp-stable-octave/octave-4.2.0/liboctave/system/file-ops.cc:303
#2  0x000000006c5f3623 in octave::sys::file_stat::update_internal (this=this@entry=0x28bcb3c0, force=force@entry=false)
    at /home/johnd/mxe-octave/build-64/tmp-stable-octave/octave-4.2.0/liboctave/system/file-stat.cc:178
#3  0x000000002cb11c4a in octave::sys::file_stat::file_stat (fl=true, n=..., this=0x28bcb3c0)
    at C:/Octave/OCTAVE~1.0/include/octave-4.2.0/octave/../octave/file-stat.h:218


John Donoghue <lostbard>
Group Member
Mon 06 Feb 2017 01:45:10 AM UTC, original submission:  

Using a oct file created  in windows 7, octave 4.2.0 (tried both w64 and w32) versions

I get a consistent hard crash of octave running the function:


#include <octave/oct.h>
#include <octave/defun-dld.h>
#include <octave/file-stat.h>

DEFUN_DLD(nofile,args,nargout,"no docs")
{
  bool ok = file_stat ("wedonthavefile").exists();
  return octave_value(ok);
}


oct file built as mkoctfile notest.cpp

Run in octave as notest.

STacktrace without any debug symbols compiled in is:


Thread 19 received signal SIGSEGV, Segmentation fault.
[Switching to Thread 10192.0x3318]
0x6c365571 in liboctinterp-4!_ZNSt8_Rb_treeIiSt4pairIKi12octave_valueESt10_Select1stIS3_ESt4lessIiESaIS3_EE4findERS1_ ()
   from C:\Octave\Octave-4.2.0\bin\liboctinterp-4.dll
(gdb) where
#0  0x6c365571 in liboctinterp-4!_ZNSt8_Rb_treeIiSt4pairIKi12octave_valueESt10_Select1stIS3_ESt4lessIiESaIS3_EE4findERS1_ ()
   from C:\Octave\Octave-4.2.0\bin\liboctinterp-4.dll
#1  0x6bca8996 in liboctinterp-4!_ZN20octave_user_functionD2Ev () from C:\Octave\Octave-4.2.0\bin\liboctinterp-4.dll
#2  0x6bca8beb in liboctinterp-4!_ZN20octave_user_functionD0Ev () from C:\Octave\Octave-4.2.0\bin\liboctinterp-4.dll
#3  0x6c365600 in liboctinterp-4!_ZNSt8_Rb_treeIiSt4pairIKi12octave_valueESt10_Select1stIS3_ESt4lessIiESaIS3_EE8_M_eraseEPSt13_Rb_tree_nodeIS3_E () from C:\Octave\Octave-4.2.0\bin\liboctinterp-4.dll
#4  0x6c3655dd in liboctinterp-4!_ZNSt8_Rb_treeIiSt4pairIKi12octave_valueESt10_Select1stIS3_ESt4lessIiESaIS3_EE8_M_eraseEPSt13_Rb_tree_nodeIS3_E () from C:\Octave\Octave-4.2.0\bin\liboctinterp-4.dll
#5  0x6c3655dd in liboctinterp-4!_ZNSt8_Rb_treeIiSt4pairIKi12octave_valueESt10_Select1stIS3_ESt4lessIiESaIS3_EE8_M_eraseEPSt13_Rb_tree_nodeIS3_E () from C:\Octave\Octave-4.2.0\bin\liboctinterp-4.dll
#6  0x6c35ef7a in liboctinterp-4!_ZNSt8_Rb_treeISsSt4pairIKSsN12symbol_table8fcn_infoEESt10_Select1stIS4_ESt4lessISsESaIS4_EE8_M_eraseEPSt13_Rb_tree_nodeIS4_E () from C:\Octave\Octave-4.2.0\bin\liboctinterp-4.dll
#7  0x6c35eecf in liboctinterp-4!_ZNSt8_Rb_treeISsSt4pairIKSsN12symbol_table8fcn_infoEESt10_Select1stIS4_ESt4lessISsESaIS4_EE8_M_eraseEPSt13_Rb_tree_nodeIS4_E () from C:\Octave\Octave-4.2.0\bin\liboctinterp-4.dll
#8  0x6c35eecf in liboctinterp-4!_ZNSt8_Rb_treeISsSt4pairIKSsN12symbol_table8fcn_infoEESt10_Select1stIS4_ESt4lessISsESaIS4_EE8_M_eraseEPSt13_Rb_tree_nodeIS4_E () from C:\Octave\Octave-4.2.0\bin\liboctinterp-4.dll
#9  0x6c35eecf in liboctinterp-4!_ZNSt8_Rb_treeISsSt4pairIKSsN12symbol_table8fcn_infoEESt10_Select1stIS4_ESt4lessISsESaIS4_EE8_M_eraseEPSt13_Rb_tree_nodeIS4_E () from C:\Octave\Octave-4.2.0\bin\liboctinterp-4.dll
#10 0x6c35eecf in liboctinterp-4!_ZNSt8_Rb_treeISsSt4pairIKSsN12symbol_table8fcn_infoEESt10_Select1stIS4_ESt4lessISsESaIS4_EE8_M_eraseEPSt13_Rb_tree_nodeIS4_E () from C:\Octave\Octave-4.2.0\bin\liboctinterp-4.dll
#11 0x6c35eecf in liboctinterp-4!_ZNSt8_Rb_treeISsSt4pairIKSsN12symbol_table8fcn_infoEESt10_Select1stIS4_ESt4lessISsESaIS4_EE8_M_eraseEPSt13_Rb_tree_nodeIS4_E () from C:\Octave\Octave-4.2.0\bin\liboctinterp-4.dll
#12 0x6c35eecf in liboctinterp-4!_ZNSt8_Rb_treeISsSt4pairIKSsN12symbol_table8fcn_infoEESt10_Select1stIS4_ESt4lessISsESaIS4_EE8_M_eraseEPSt13_Rb_tree_nodeIS4_E () from C:\Octave\Octave-4.2.0\bin\liboctinterp-4.dll
#13 0x6c1068a5 in liboctinterp-4!_ZN12symbol_table8fcn_info12fcn_info_rep36mark_subfunction_in_scope_as_privateEiRKSs ()
   from C:\Octave\Octave-4.2.0\bin\liboctinterp-4.dll
#14 0x6b8811b4 in ?? () from C:\Octave\Octave-4.2.0\bin\liboctinterp-4.dll
#15 0x6b881325 in ?? () from C:\Octave\Octave-4.2.0\bin\liboctinterp-4.dll
#16 0x6b881433 in ?? () from C:\Octave\Octave-4.2.0\bin\liboctinterp-4.dll
#17 0x77d06b8e in ntdll!RtlDecompressBuffer () from C:\WINDOWS\SYSTEM32\ntdll.dll
#18 0x77cd5ade in ntdll!RtlAnsiCharToUnicodeChar () from C:\WINDOWS\SYSTEM32\ntdll.dll
#19 0x77ce7efc in ntdll!LdrShutdownProcess () from C:\WINDOWS\SYSTEM32\ntdll.dll
#20 0x00000000 in ?? ()


John Donoghue <lostbard>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #39823:  inline.diff added by rik5 (396B - text/x-diff)
file #39750:  oct_no_crash.patch added by lostbard (2KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by _108826 (Posted a comment)
  • -email is unavailable- added by bpabbott (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by lostbard (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 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-02-24 rik5 Open/ClosedOpen Closed
    2017-02-24 rik5 Attached File- Added inline.diff, #39823
    2017-02-24 bpabbott Open/ClosedClosed Open
    2017-02-21 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2017-02-20 rik5 StatusConfirmed Ready For Test
    2017-02-13 lostbard Attached File- Added oct_no_crash.patch, #39750
    2017-02-09 rik5 StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code