bugGNU Octave - Bugs: bug #46417, GL2PS info: OpenGL feedback buffer...

 
 

bug #46417: GL2PS info: OpenGL feedback buffer overflow when printing

Submitted by:  None
Submitted on:  Tue 10 Nov 2015 06:49:13 PM UTC  
 
Category: Plotting with OpenGLSeverity: 3 - Normal
Priority: 5 - NormalItem Group: None
Status: FixedAssigned to: None
Originator Name: Clinton WinantOriginator Email: -unavailable-
Open/Closed: ClosedRelease: 4.0.0
Operating System: GNU/Linux

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 Jump to the original submission)

Sun 17 Jan 2016 01:10:00 AM UTC, comment #23:

Finally done with this overflow bug! I checked in your changes here (http://hg.savannah.gnu.org/hgweb/octave/rev/8c9755d29d2a). Closing report.

Rik <rik5>
Project Administrator
Wed 06 Jan 2016 08:06:05 AM UTC, comment #22:

I attached a new patch.
@Rik: I followed your proposition, the buffer is 4Mb large by default and is increased twice after each overflow. This makes the original example work without overflow. You now have to use "pcolor (rand (300))" to test the overflow behavior.

@jwe: indeed there used to be an unnecessary "else" statement in the patch from comment #15. It is now removed.

(file #35931)

Pantxo Diribarne <pantxo>
Project Member
Sat 02 Jan 2016 08:20:57 PM UTC, comment #21:

Panxto:

I remember seeing something like

Since error can't return now, it's OK and preferred to write

It's not terribly important, but it can reduce the amount of indentation a little bit.

John W. Eaton <jwe>
Project Administrator
Sat 02 Jan 2016 06:28:03 PM UTC, comment #20:

@Panxto: I think you're correct that the starting buffer size of 1MB is way too small. How about a starting size of 4MB?

Another question is whether the buffer size should grow linearly or exponentially? If the first round fails at 4MB is it really likely to succed at 5 MB? or should we be increasing by a scale factor like 1.5 or 2 each time through the loop? From my earlier experiments, a pcolor plot requires a fixed 75 bytes per pixel. Given that this is a 2-D image, a linear growth in the length of a side of the image requires a squared growth in memory requirements. If we change the code to

Then Octave will be growing linearly in image size. I think this will also help the underlying malloc() call to have memory requirements that are aligned to powers of 2.

To make it concrete,

Rik <rik5>
Project Administrator
Sat 02 Jan 2016 04:46:36 PM UTC, comment #19:

@Pantxo: I tried your latest patch. It applies without errors to
the current dev system on Fedora 23 and seems to work as before
except that the seg fault that always occurs on my system
(Intel HD6000 graphics) has changed.

I have included the bt of the seg fault in case you want
to take a look. The fault is in mesa code, which should not
be failing.

In any case this patch seems like a good idea and should
be pushed unless there are other problems on other systems.

(file #35904)

Michael Godfrey <godfrey>
Project Member
Sat 02 Jan 2016 07:53:17 AM UTC, comment #18:

@ederag: I am at 21011:d9f4e4961e09 on default.

@jwe: I was looking for a way to discard previous content of the temporary file while keeping access to the original file pointer that I had passed to unwind_protect. You are right that fseek+ftruncate is simpler but I didn't know of ftruncate hence the use of freopen.
I didn't know about gnulib::tmpfile either and it seams to do exactly what I want without bothering with fclose/unlink in unwind_protect. Thanks
I attached an updated patch. You need to run "bootstrap" first as I added gnulib::ftruncate.
I used the following for tests:

>> The error function no longer returns so you don't need an "else" after that

After what?

(file #35903)

Pantxo Diribarne <pantxo>
Project Member
Thu 31 Dec 2015 11:07:59 PM UTC, comment #17:

Why do you need freopen here? Wouldn't it be sufficient to just use fclose and the fopen? In any case, there does appear to be a gnulib replacement for freopen, so just add the freopen module to bootstrap.conf and use gnulib::freopen.

Or, instead of reopening the file, can't you just reset the position to the beginning when you try the operation again with a larger buffer? Or, if you need to truncate the file, use ftruncate. There is a gnulib module that appears to provide that function for Windows, so portability shouldn't be an issue.

The error function no longer returns so you don't need an "else" after that.

Also, why not use gnulib::tmpfile? That gives you a FILE* to a temporary file that will be deleted automatically when it is closed or when the program terminates. Then you don't need to bother with the unlink step yourself.

John W. Eaton <jwe>
Project Administrator
Thu 31 Dec 2015 06:58:29 PM UTC, comment #16:

@Pantxo, trying to test your patch, against current 21020:7ebc9f38b312, some hunks failed.
What is your base ?

ederag <ederag>
Thu 31 Dec 2015 11:48:26 AM UTC, comment #15:

I found the time to write a new patch (between two diaper changes). It works for ps/eps, pdf, pdflatexstandalone and png. Feel free to test other formats.

Things that remain to be done:

  • I used bare c "freopen" function. How do I import gnulib version?
  • I'd like to also increase the size of the base buffer so that those overflow don't happen too often: 1Mb is ridiculously small as compared to the amount of memory people have on their computers. 5Mb? 10Mb?

(file #35898)

Pantxo Diribarne <pantxo>
Project Member
Wed 02 Dec 2015 04:59:11 PM UTC, comment #14:

Seems like a reasonable idea to hide the gl2ps stream from other callers until it is guaranteed to be valid, and then just copy the data over.

Rik <rik5>
Project Administrator
Wed 02 Dec 2015 03:58:35 PM UTC, comment #13:

Ok so it looks like there is no easy way out now we use the viewport feature:

  • gl2psEndPage produces corrupted streams instead of untouched ones (as it does in 4.0 without the use of viewports)
  • our printing process involves multiple manipulations of streams including conversion/deletion of eventually corrupted ones.

I have an idea that should work and be less intrusive than the previous one (no need to change _osmesa_print_): use a dummy file stream as gl2ps argument and then copy the content of that file stream to the pipe stream (provided by __osmes_print or gl2ps_print) after a successfull run.

Pantxo Diribarne <pantxo>
Project Member
Mon 30 Nov 2015 10:12:36 PM UTC, comment #12:

I tried overflow3.patch and it works if I print to eps, but not if I go to pdf. The file is created, but it is all white. I also get the following interesting error messages

Rik <rik5>
Project Administrator
Mon 30 Nov 2015 09:14:08 PM UTC, comment #11:

@Rik: I did not notice that some of the images in the doc did not build anymore.

I attached a patch in which I changed the interface to glps_renderer: it is glps_renderer::draw that is responsible for opening pipe streams (as many as needed to ensure we don't use a corrupted one) and the first argument to glps_renderer constructor is removed.

As I had to change _osmesa_print_ which used to provide its own file stream, I would like to have Andy's opinion, I may have missed something. Ccing him.

(file #35603)

Pantxo Diribarne <pantxo>
Project Member
Mon 30 Nov 2015 12:58:52 AM UTC, comment #10:

@Pantxo: Can you build the documentation anymore? It seems that some of the images, such as spchol, had overflows. Now when I try to regenerate them I get errors in texindex. It looks to me like we will need to find a solution for this problem.

Steps to reproduce:

Rik <rik5>
Project Administrator
Fri 27 Nov 2015 12:17:00 PM UTC, comment #9:

@Rik: yes both :-). The error message should be changed and I also obtain a white image but heavier (closer to the expected result).

Try

and open the eps in an editor. You'll see that the prolog has been written 3 times: we get 2 overflows before the buffer is sufficiently large. If you delete the 2 superfluous prologs (sections starting with "%!PS-Adobe-3.0 EPSF-3.0") and save you'll get a valid eps.

The issue is that each time we retry to print after increasing the buffer size, we use the same stream which is thus corrupted. I don't know how to fix this without changing the interface to gl2ps::rend which expect the caller (e.g. _osmesa_print_)) to provide the stream as argument.

Pantxo Diribarne <pantxo>
Project Member
Fri 27 Nov 2015 05:40:51 AM UTC, comment #8:

@Pantxo: I think a partial fix is still better than no solution. In your patch, I see

Should the error message actually be

since that was the gl2ps function that was most recently executed?

Also, with your patch applied do you get the correct output from the test code?

I still get an all white plot.

Rik <rik5>
Project Administrator
Thu 26 Nov 2015 11:04:39 PM UTC, comment #7:

@Rik: the structure we are using now in default is the same as in gl2psTest.c, that is why you observe the same behavior in Octave and this example. As you noted "gl2psEndPage ()" always returns GL2PS_NO_FEEDBACK because we just ended the viewport using "gl2psEndViewport ()". The latter returns what we used to expect (see comments in http://hg.savannah.gnu.org/hgweb/octave/rev/395140e53656):

Detecting the overflow from "gl2psEndViewport ()" works well (see attached patch), the problem is how to handle corrupted processes: at the time we detect the overflow, the stream (a file or a process to which we pipe gl2ps output) is already corrupted. We can't simply rewind and overwrite it ...

(file #35569)

Pantxo Diribarne <pantxo>
Project Member
Wed 25 Nov 2015 06:30:57 PM UTC, comment #6:

The issue may reside in gl2ps. Octave is essentially doing the following

I ran Octave under gdb and the problem is that gl2psEndPage is always returning GL2PS_NO_FEEDBACK (the feedback buffer is empty) rather than GL2PS_OVERFLOW (the size of the feedback buffer given to gl2psBeginPage is not large enough).

The example code should require three trips through the loop.

I do get messages printed to stderr about GL2PS_OVERFLOW for the first two function calls so gl2ps knows about the error, but is not reporting it back correctly upstream. I don't know if this is a problem with gl2ps or with the way we are calling it. I suspect maybe the latter because I downloaded the code for gl2ps and the sample test file is written as

I modified this to

Now when I run the simple test program it spits out a bunch of GL2PS_OVERFLOWs, but eventually ends with success. It never returns GL2PS_NO_FEEDBACK.

If I modify gl2psTest.c in the same way then I can get GL2PS_NO_FEEDBACK returns, but eventually the function succeeds.

It seems long, but might have to build a version of the gl2ps library with debugging symbols included and then run all of Octave under it to see what is going on.

Rik <rik5>
Project Administrator
Wed 25 Nov 2015 05:47:52 PM UTC, comment #5:

I'm adding Pantxo to the list since he has more experience with gl2ps.

Rik <rik5>
Project Administrator
Thu 19 Nov 2015 06:56:50 PM UTC, comment #4:

@Rik, isn't that the purpose of
while (state == GL2PS_OVERFLOW) ?
Shouldn't it increase the buffer size until not overflowing ?
The printed page is white, so it does not succeed at all.
There might be something else going on.

ederag <ederag>
Thu 19 Nov 2015 04:17:09 PM UTC, comment #3:

Can the original reporter upload the problem jpg file and the command used to read and print it? I want to verify what buffersize might be required to fix this issue.

Rik <rik5>
Project Administrator
Tue 10 Nov 2015 11:35:34 PM UTC, comment #2:

Confirmed. I used the pcolor example from comment #1 to verify. The issue is in gl2ps-renderer.cc.

Apparently each pixel in a surface object takes about 75 bytes so the current buffer size can only handle

I verified that pcolor objects below this range work fine.

Ideally, we could use some property from the graphic object being drawn to determine the size of the buffer. Failing that, we could just increase this value. 1024*1024 is only 1 MB which isn't all that much on most machines these days.

Rik <rik5>
Project Administrator
Tue 10 Nov 2015 09:06:03 PM UTC, comment #1:

With the dev version (20693:050d8fc28fe8), this error message can be reproduced by

But with graphics_toolkit("gnuplot") it prints fine.
Are you really using gnuplot ?

ederag <ederag>
Tue 10 Nov 2015 06:49:13 PM UTC, original submission:

with graphics_toolkit ("gnuplot");

print -djpeg threebath.jpg

produces the correct jpg file,

without invoking gnuplot, the same command produces:

GL2PS info: OpenGL feedback buffer overflow
GL2PS info: OpenGL feedback buffer overflow
GL2PS info: OpenGL feedback buffer overflow
GL2PS info: OpenGL feedback buffer overflow

Anonymous

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #35931:  overflow10.patch added by pantxo (6KiB - application/x-download)
file #35904:  print_fail_020116 added by godfrey (34KiB - application/octet-stream)
file #35903:  overflow9.patch added by pantxo (6KiB - application/x-download)
file #35898:  overflow.patch added by pantxo (6KiB - text/x-diff)
file #35603:  overflow3.patch added by pantxo (7KiB - application/x-download)
file #35569:  overflow.patch added by pantxo (3KiB - application/x-download)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by godfrey (Updated the item)
  • -unavailable- added by jwe (Posted a comment)
  • -unavailable- added by pantxo
  • -unavailable- added by pantxo (Updated the item)
  • -unavailable- added by rik5
  • -unavailable- added by johasixt
  • -unavailable- added by rik5 (Posted a comment)
  • -unavailable- added by ederag (Posted a comment)
  •  

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

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 15 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 31 Jan 2016 05:04:29 AM UTCrik5Open/ClosedOpen=>Closed
    Sun 17 Jan 2016 01:10:00 AM UTCrik5StatusIn Progress=>Fixed
    Wed 06 Jan 2016 08:06:05 AM UTCpantxoAttached File-=>Added overflow10.patch, #35931
    Sat 02 Jan 2016 04:46:36 PM UTCgodfreyAttached File-=>Added print_fail_020116, #35904
    Sat 02 Jan 2016 07:53:17 AM UTCpantxoAttached File-=>Added overflow9.patch, #35903
    Thu 31 Dec 2015 11:48:26 AM UTCpantxoAttached File-=>Added overflow.patch, #35898
    Wed 02 Dec 2015 04:59:11 PM UTCrik5StatusReady For Test=>In Progress
    Mon 30 Nov 2015 10:12:36 PM UTCrik5StatusConfirmed=>Ready For Test
    Mon 30 Nov 2015 09:14:08 PM UTCpantxoAttached File-=>Added overflow3.patch, #35603
      Carbon-Copy-=>Added -unavailable-
    Thu 26 Nov 2015 11:04:39 PM UTCpantxoAttached File-=>Added overflow.patch, #35569
    Wed 25 Nov 2015 05:47:52 PM UTCrik5Carbon-Copy-=>Added pantxo
    Wed 25 Nov 2015 03:30:55 PM UTCjohasixtCarbon-Copy-=>Added johasixt
    Tue 10 Nov 2015 11:35:34 PM UTCrik5StatusNone=>Confirmed
      SummaryError using print=>GL2PS info: OpenGL feedback buffer overflow when printing

    Back to the top


    Powered by Savane 3.1-cleanup1