bugGNU Octave - Bugs: bug #44662, [OF io] csv2cell.cc miscompiled...

 
 

bug #44662: [OF io] csv2cell.cc miscompiled with Octave built for 64-bit windows

Submitter:  Philip Nienhuis <philipnienhuis>
Submitted:  Sun 29 Mar 2015 06:04:21 PM UTC
   
 
Category:  Configuration and Build System Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Segfault, Bus Error, etc.
Status:  None Assigned to:  None
Originator Name:  Philip Nienhuis Open/Closed:  * Closed
Release:  * other Operating System:  * Microsoft Windows
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sat 02 May 2015 12:40:37 PM UTC, comment #10: 

Closing this bug report in favor of bug #44998.

It seems to be a miscompilation on the Windows side; a cross-built csv2cell.oct (built by mxe-octave) runs fine once on Windows.

Philip Nienhuis <philipnienhuis>
Group Member
Fri 01 May 2015 09:52:27 PM UTC, comment #9: 

Changing "category" to "configuration and build system"

Philip Nienhuis <philipnienhuis>
Group Member
Fri 01 May 2015 09:51:40 PM UTC, comment #8: 

I'm convinced this is a mingw compiler issue.

These days OF packages can be cross-built as well on Linux. It turns out that a cross-built csv2cell() (with the --enable-windows-64 flag) runs fine on win64.

Philip Nienhuis <philipnienhuis>
Group Member
Sun 19 Apr 2015 08:55:08 AM UTC, comment #7: 

The copy & crash issue is related to patch #8656 (see comment #4)

Philip Nienhuis <philipnienhuis>
Group Member
Sat 18 Apr 2015 03:58:05 PM UTC, comment #6: 

I'm quite convinced that is something with the --enable-windows-64 MXE configure option that underlies the problems.
If I do:

mkoctfile -v csv2cell.csv


... and then try to copy the output on the screen using Ctrl-C, Octave crashes.

Philip Nienhuis <philipnienhuis>
Group Member
Tue 31 Mar 2015 12:04:04 AM UTC, comment #5: 

csvconcat is failing?  There isn't much in that file.

I worry a bit about these lines in csvconcat.cc:


/* Output real value */
char tmp[20];
sprintf(tmp, "%g", c(i, j).double_value());
word += tmp;


Twenty characters isn't much wider than a double translated to decimal notation.  It shouldn't be a problem, though.  There might be a C++ streams method for converting the double_value() without having to use the stack:

http://en.cppreference.com/w/cpp/string/basic_string/to_string

but maybe that is slower.

In any case, I'd say continue adding "std::cout <<" throughout the file with various messages and there is a good chance it will pinpoint where the failure occurs.  csvconcat.cc can't go very far before failure.

Dan Sebald <sebald>
Mon 30 Mar 2015 04:48:27 PM UTC, comment #4: 

With a 64-bit indexing Octave it's the same problem :-(

AFAICS the compiled functions num2col, col2num, xmlread and xmlwrite do work fine, however.

So the problem seems to be confined to the csvcell, cell2csv, csvconcat and csvexplode functions.

Philip Nienhuis <philipnienhuis>
Group Member
Mon 30 Mar 2015 06:49:35 AM UTC, comment #3: 

Hmm, yeah, cell2csv() is a much less potentially problematic because it isn't using the stack to the degree csv2cell() is.  With cell2cvs() one can comment out most of the code and slowly add lines back in to see where the function is crashing.  However, I'm wondering if the issue is related to the addition of package object code in the non-64-bit addressing environment.  Are other functions in the package working as expected?

Dan Sebald <sebald>
Sun 29 Mar 2015 08:03:11 PM UTC, comment #2: 

Thanks for quick reply!

Yes, the actual failure occurs probably a lot earlier up in the code.

But I'm most of all puzzled why it works well on 64-bit Linux, with a 32-bit Octave and with an earlier Octave 64-bit indexing development version on Windows, but not with --enable-windows-64 (= compiled for 64 bit Windows but no 64-bit indexing).

In the mean time it turns out that csv2cell()'s sibling cell2csv() (also in io package) crashes as well.

The only other thing I could do is build a 64-bit indexing Octave-4.0.0+ and see what happens then.

Philip Nienhuis <philipnienhuis>
Group Member
Sun 29 Mar 2015 07:06:57 PM UTC, comment #1: 

Here is the documentation on the streams getline function:

http://www.cplusplus.com/reference/istream/istream/getline/

Hard to debug without it failing here, but just a couple suggestions to look at from your perspective.

The fact that the exact location of failure changes based upon what lines you add or remove suggests to me the bug (probably a memory violation) occurs sooner in the file.

One thing that stands out is that in this code:


/* Buffers */
192 char line [MAXSTRINGLENGTH];
193 std::string str, word;
194 bool inside = false;
195
196 /* Read headerlines */
197 for (long ii=0; ii < hlines; ii++)
198 {
199 fd.getline (line, MAXSTRINGLENGTH);
200 if (fd.tellg () >= fdend || fd.fail ())
201 return octave_value (Cell (0, 0));
202 }


the character buffer length is MAXSTRINGLENGTH, rather than MAXSTRINGLENGTH+1.  The documentation suggests that a '\0' is added to the end.  So if the file in question is, say by mistake, a binary file or happens to have a line greater than MAXSTRINGLENGTH will fd.getline() write past the end of the line[] buffer?  Try making the code:


char line [MAXSTRINGLENGTH+1];


and see if that helps.

Another question is whether the last line of this code:


260 fd.getline (line, MAXSTRINGLENGTH);
261 while (fd.fail ())
262 {
263 fd.clear ();
264 str += line;
265 fd.getline (line, MAXSTRINGLENGTH);
266 }
267 str += line;


does anything.  Probably a nicer way of programming these loops is more along the lines:


while (fd.fail ())
{
    fd.getline (line, MAXSTRINGLENGTH);
    if (fd.gcount () > 0)
        str += line;
}


using the function described in the documentation.  Then there isn't any code need before and after the loop.

Dan Sebald <sebald>
Sun 29 Mar 2015 06:04:21 PM UTC, original submission:  

(overflow from bug #44493, after trying some time to sort things out myself)

With an Octave cross-compiled for 64-bit Windows (--enable-windows-64, not --enable-64), it seems csv2cell.cc in the io package gets miscompiled. When trying to run it, it crashes and takes Octave with it. I've built csv2cell.cc simply by "mkoctfile csv2cell.cc", like is done during io package installation.

When building csv2cell.cc on Windows 7 (64-bit) with a 32-bit Octave (e.g., 4.0.0-rc2) and an older 4.1.0+ with 64-bit indexing from Feb. 10, csv2cell.cc runs fine. Same for building csv2cell.cc on 64-bit Linux.

By inserting "std::cout <<" statements I could figure out that the crash occurs as soon as a 'while (fd.fail())' on L.207 is executed ("fd" = file pointer to .csv file). That may be a red herring as inspecting fd.fail() in a "std::cout <<" statement before the while loop gives no errors. After commenting out the while loop, the crash occurs as soon as some other variable "str" is assigned a new value. That's as far as I got.

My C++ skills are too little to get any further without help. Anyone who can give me a clue?

csv2cell.cc is here:
http://hg.code.sf.net/p/octave/io/file/a920f7408781/src/csv2cell.cc (but may need a password)
Access through http from octave.sf.net | code and then clicking the io repo in the page (http://sourceforge.net/p/octave/_list/hg) I get a "500" error in http://sourceforge.net/p/octave/io/ci/default/tree/


Thanks

Philip Nienhuis <philipnienhuis>
Group 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 sebald (Posted a comment)
  • -email is unavailable- added by philipnienhuis (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 2 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2015-05-02 philipnienhuis Open/ClosedOpen Closed
    2015-05-01 philipnienhuis CategoryOctave Package Configuration and Build System

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code