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

 
 

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

Submitter:  None
Submitted:  Tue 10 Nov 2015 06:49:13 PM UTC
   
 
Category:  Plotting with OpenGL Severity:  3 - Normal
Priority:  5 - Normal Item Group:  None
Status:  Fixed Assigned to:  None
Originator Name:  Clinton Winant Originator Email:  -email is unavailable-
Open/Closed:  * Closed Release:  * 4.0.0
Operating System:  * GNU/Linux Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

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>
Group 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>
Group Member
Sat 02 Jan 2016 08:20:57 PM UTC, comment #21: 

Panxto:

I remember seeing something like


if (condition)
  error (message);
else
  do_something_else ();


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


if (condition)
  error (message);

do_something_else ();


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

John W. Eaton <jwe>
Group 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


buffsize *= 2;


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,


starting_img_size = sqrt ((4 * 1024 * 1024) / 75) = 236 x 236
img_size_1 = sqrt ((8 * 1024 * 1024) / 75) = 334 x 334
img_size_2 = sqrt ((16 * 1024 * 1024) / 75) = 473 x 473
img_size_3 = sqrt ((32 * 1024 * 1024) / 75) = 669 x 669



Rik <rik5>
Group 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>
Group 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:


pcolor (rand (200));
## pdf terminal
print -dpdflatexstandalone try.tex
## others based on eps terminal
terms = {".png", ".jpg", ".ps", ".pdf"}
cellfun (@(s) print (["try" s]), terms)



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

After what?


(file #35903)

Pantxo Diribarne <pantxo>
Group 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>
Group 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>
Group 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>
Group 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>
Group 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


pcolor (rand (200))
print tst.pdf
GL2PS info: OpenGL feedback buffer overflow
GL2PS info: OpenGL feedback buffer overflow
rm: cannot remove `/tmp/oct-aPeWF3.ps': No such file or directory
rm: cannot remove `/tmp/oct-aPeWF3.ps': No such file or directory



Rik <rik5>
Group 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>
Group 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:


cd doc/interpreter
hg stat -un *.eps | xargs rm
cd ../..
make V=1 -j1



Rik <rik5>
Group 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

pcolor (rand (200));
print try.eps


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>
Group 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


+    else if (state == GL2PS_ERROR)
+      error ("gl2ps-renderer::draw_axes: gl2psEndPage returned GL2PS_ERROR");


Should the error message actually be


+      error ("gl2ps-renderer::draw_axes: gl2psEndViewport returned GL2PS_ERROR");


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?


pcolor (rand (200));
print try.png


I still get an all white plot.

Rik <rik5>
Group 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):


gl2psBeginPage ()
  // First axes
  gl2psBeginViewport ()
    // draw_axes
  state = gl2psEndViewport () // Check this one for errors and warnings

  ...

  // Nth axes
  gl2psBeginViewport ()
    // draw_axes
  state = gl2psEndViewport () // Check this one for errors and warnings
state = gl2psEndPage () // Not meaningful


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>
Group Member
Wed 25 Nov 2015 06:30:57 PM UTC, comment #6: 

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


while (state == GL2PS_OVERFLOW)
{
   bufsize += 1024*1024;
   gl2psBeginPage (..., bufsize,...);
   draw_scene_with_opengl
   state = gl2psEndPage ();
}


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.


pcolor (rand (200));
print try.png


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


while(state == GL2PS_OVERFLOW){
  buffsize += 1024*1024;
  gl2psBeginPage(file, "gl2psTest", viewport, format, sort, options,
                 GL_RGBA, 0, NULL, nbcol, nbcol, nbcol,
                 buffsize, fp, file);
  display();
  state = gl2psEndPage();
}


I modified this to


while(state == GL2PS_OVERFLOW){
  buffsize += 1024;
  gl2psBeginPage("test", "gl2psTestSimple", NULL, GL2PS_EPS, GL2PS_SIMPLE_SORT,
                 GL2PS_DRAW_BACKGROUND | GL2PS_USE_CURRENT_VIEWPORT,
                 GL_RGBA, 0, NULL, 0, 0, 0, buffsize, fp, "out.eps");
  display();
  state = gl2psEndPage();
  printf ("Final state: %d\n", state);
}


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>
Group 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>
Group 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>
Group 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. 


buffsize += 1024*1024;


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


sqrt (1024*1024 / 75) == 118


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>
Group 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

graphics_toolkit("qt")
pcolor(rand(200))
print try.png

GL2PS info: OpenGL feedback buffer overflow


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 Files:
   
   
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
  • -email is unavailable- added by godfrey (Updated the item)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by pantxo
  • -email is unavailable- added by pantxo (Updated the item)
  • -email is unavailable- added by rik5
  • -email is unavailable- added by johasixt
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by ederag (Posted a comment)
  •  

    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 15 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-01-31 rik5 Open/ClosedOpen Closed
    2016-01-17 rik5 StatusIn Progress Fixed
    2016-01-06 pantxo Attached File- Added overflow10.patch, #35931
    2016-01-02 godfrey Attached File- Added print_fail_020116, #35904
    2016-01-02 pantxo Attached File- Added overflow9.patch, #35903
    2015-12-31 pantxo Attached File- Added overflow.patch, #35898
    2015-12-02 rik5 StatusReady For Test In Progress
    2015-11-30 rik5 StatusConfirmed Ready For Test
    2015-11-30 pantxo Attached File- Added overflow3.patch, #35603
        Carbon-Copy- Added -email is unavailable-
    2015-11-26 pantxo Attached File- Added overflow.patch, #35569
    2015-11-25 rik5 Carbon-Copy- Added pantxo
    2015-11-25 johasixt Carbon-Copy- Added johasixt
    2015-11-10 rik5 StatusNone Confirmed
        SummaryError using print GL2PS info: OpenGL feedback buffer overflow when printing

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code