Bug 24575

Summary: evince crashed with SIGSEGV in CairoOutputDev::restoreState()
Product: poppler Reporter: Pedro Villavicencio <pvillavi>
Component: cairo backendAssignee: poppler-bugs <poppler-bugs>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: medium CC: davidben
Version: unspecified   
Hardware: x86 (IA32)   
OS: All   
URL: https://bugs.edge.launchpad.net/ubuntu/+source/poppler/+bug/430887
Whiteboard:
i915 platform: i915 features:
Attachments: Do not restoreState on empty stack

Description Pedro Villavicencio 2009-10-16 12:13:42 UTC
this report has been filed here:

https://bugs.edge.launchpad.net/ubuntu/+source/poppler/+bug/430887

"Evince crashes while loading the attached document (also available for now at http://www.orm.com.br/projetos/tvliberal/pdf/LISTADEPRE%C3%87OS_abrilsemtembro.pdf).
If the thumbnail pane is enabled, evince crashes after displaying (apparently corrupted) the 3rd page; if the pane is closed before opening the file, evince only crashes if I scroll down and try to display the 3rd page or any later.
Even in the latter case and before scrolling down, the error output already contains plenty of errors. IOW while seemingly decoding the first page."

".
Thread 3 (process 3547):
#0  0x0027d422 in __kernel_vsyscall ()
No symbol table info available.
#1  0x00b3cba6 in *__GI___poll (fds=0xbd1ff4, nfds=9, timeout=9)
    at ../sysdeps/unix/sysv/linux/poll.c:87
	resultvar = <value optimized out>
	oldtype = 0
	result = <value optimized out>
#2  0x00f2c52b in IA__g_poll (fds=0xa096e50, nfds=9, timeout=9)
    at /build/buildd/glib2.0-2.21.6/glib/gpoll.c:127
No locals.
#3  0x00f1f5fb in g_main_context_iterate (context=0xa0ac058, 
    block=<value optimized out>, dispatch=1, self=0xa094050)
    at /build/buildd/glib2.0-2.21.6/glib/gmain.c:2904
	max_priority = 2147483647
	timeout = 9
	some_ready = <value optimized out>
	nfds = 9
	allocated_nfds = <value optimized out>
	fds = <value optimized out>
	__PRETTY_FUNCTION__ = "g_main_context_iterate"
#4  0x00f1fc2f in IA__g_main_loop_run (loop=0xa096220)
    at /build/buildd/glib2.0-2.21.6/glib/gmain.c:2799
	self = (GThread *) 0xa094050
	__PRETTY_FUNCTION__ = "IA__g_main_loop_run"
#5  0x003fc6f9 in IA__gtk_main ()
    at /build/buildd/gtk+2.0-2.17.11/gtk/gtkmain.c:1205
	tmp_list = (GList *) 0xa0a14e4
	functions = (GList *) 0x0
	init = (GtkInitFunction *) 0x0
	loop = (GMainLoop *) 0xa096220
#6  0x08080aa2 in main (argc=1, argv=0xbfc3d654) at main.c:497
	context = <value optimized out>
	args = (GHashTable *) 0xa0e16f0
	error = (GError *) 0x0
.
Thread 2 (process 3549):
#0  0x0027d422 in __kernel_vsyscall ()
No symbol table info available.
#1  0x00144142 in pthread_cond_timedwait@@GLIBC_2.3.2 ()
   from /lib/tls/i686/cmov/libpthread.so.0
No symbol table info available.
#2  0x0088315e in g_cond_timed_wait_posix_impl (cond=0xfffffdfc, 
    entered_mutex=0x1, abs_time=0xb63bf288)
    at /build/buildd/glib2.0-2.21.6/gthread/gthread-posix.c:242
	result = <value optimized out>
	end_time = {tv_sec = 1253118030, tv_nsec = 894223000}
	timed_out = <value optimized out>
	__PRETTY_FUNCTION__ = "g_cond_timed_wait_posix_impl"
#3  0x00ef6cac in g_async_queue_pop_intern_unlocked (queue=0xa29a0c0, 
    try=<value optimized out>, end_time=0xb63bf288)
    at /build/buildd/glib2.0-2.21.6/glib/gasyncqueue.c:365
	retval = <value optimized out>
	__PRETTY_FUNCTION__ = "g_async_queue_pop_intern_unlocked"
#4  0x00ef6db1 in IA__g_async_queue_timed_pop (queue=0xa29a0c0, 
    end_time=0xb63bf288)
    at /build/buildd/glib2.0-2.21.6/glib/gasyncqueue.c:491
	retval = <value optimized out>
	__PRETTY_FUNCTION__ = "IA__g_async_queue_timed_pop"
#5  0x00f47b1e in g_thread_pool_thread_proxy (data=0xa2e16f0)
    at /build/buildd/glib2.0-2.21.6/glib/gthreadpool.c:121
	task = <value optimized out>
	pool = <value optimized out>
#6  0x00f4657f in g_thread_create_proxy (data=0xa221fc8)
    at /build/buildd/glib2.0-2.21.6/glib/gthread.c:635
	__PRETTY_FUNCTION__ = "g_thread_create_proxy"
#7  0x0013f80e in start_thread () from /lib/tls/i686/cmov/libpthread.so.0
No symbol table info available.
#8  0x00b4a7ee in clone () at ../sysdeps/unix/sysv/linux/i386/clone.S:130
No locals.
.
Thread 1 (process 3548):
#0  CairoOutputDev::restoreState (this=0xa30cbf8, state=0xb65046a0)
    at CairoOutputDev.cc:274
	ms = (CairoOutputDev::MaskStack *) 0x0
#1  0x0103c06d in Gfx::restoreState (this=0xb6504540) at Gfx.cc:4716
No locals.
#2  0x0103c09d in Gfx::opRestore (this=0xb6504540, args=0xb6e58dc4, numArgs=0)
    at Gfx.cc:842
No locals.
#3  0x0103c576 in Gfx::execOp (this=0xb6504540, cmd=0xb6e58f64, 
    args=0xb6e58dc4, numArgs=0) at Gfx.cc:790
	op = <value optimized out>
	name = 0xb6504ec0 "Q"
	argPtr = (Object *) 0xb6e58dc4
	i = 0
#4  0x0103cb99 in Gfx::go (this=0xb6504540, topLevel=1) at Gfx.cc:661
	timer = {start_time = {tv_sec = 1253118018, tv_usec = 461139}, 
  end_time = {tv_sec = 17296957, tv_usec = 18173940}, active = 1}
	obj = {type = objCmd, {booln = -1236250944, intg = -1236250944, 
    real = 0.015064974964878686, string = 0xb6504ec0, name = 0xb6504ec0 "Q", 
    array = 0xb6504ec0, dict = 0xb6504ec0, stream = 0xb6504ec0, ref = {
      num = -1236250944, gen = 1066326626}, cmd = 0xb6504ec0 "Q"}}
	numArgs = 0
	i = <value optimized out>
	lastAbortCheck = 0
	args = {{type = objNone, {booln = -1236250960, intg = -1236250960, 
      real = 0.015064974964878658, string = 0xb6504eb0, 
      name = 0xb6504eb0 "q", array = 0xb6504eb0, dict = 0xb6504eb0, 
      stream = 0xb6504eb0, ref = {num = -1236250960, gen = 1066326626}, 
      cmd = 0xb6504eb0 "q"}}, {type = objNone, {booln = 0, intg = 0, 
      real = 0, string = 0x0, name = 0x0, array = 0x0, dict = 0x0, 
      stream = 0x0, ref = {num = 0, gen = 0}, cmd = 0x0}}, {type = objNone, {
      booln = 0, intg = 0, real = 0, string = 0x0, name = 0x0, array = 0x0, 
      dict = 0x0, stream = 0x0, ref = {num = 0, gen = 0}, cmd = 0x0}}, {
    type = objNone, {booln = 97967555, intg = 97967555, 
      real = 0.015064969828798241, string = 0x5d6ddc3, 
      name = 0x5d6ddc3 <Address 0x5d6ddc3 out of bounds>, array = 0x5d6ddc3, 
      dict = 0x5d6ddc3, stream = 0x5d6ddc3, ref = {num = 97967555, 
        gen = 1066326626}, 
      cmd = 0x5d6ddc3 <Address 0x5d6ddc3 out of bounds>}}, {type = objNone, {
      booln = -1709993466, intg = -1709993466, real = 0.05171116000000002, 
      string = 0x9a139206, 
      name = 0x9a139206 <Address 0x9a139206 out of bounds>, 
      array = 0x9a139206, dict = 0x9a139206, stream = 0x9a139206, ref = {
        num = -1709993466, gen = 1068136930}, 
      cmd = 0x9a139206 <Address 0x9a139206 out of bounds>}}, {type = objNone, 
    {booln = 369075268, intg = 369075268, real = 0.14094782900000002, 
      string = 0x15ffa444, 
      name = 0x15ffa444 <Address 0x15ffa444 out of bounds>, 
      array = 0x15ffa444, dict = 0x15ffa444, stream = 0x15ffa444, ref = {
        num = 369075268, gen = 1069681300}, 
      cmd = 0x15ffa444 <Address 0x15ffa444 out of bounds>}}, {type = objNone, 
    {booln = 9695410, intg = 9695410, real = 0.29999995285448755, 
      string = 0x93f0b2, 
      name = 0x93f0b2 "Aïÿÿ\215\205üþÿÿ\211\205Ìþÿÿ\211D$\004\213U\020\211\024$\213E\034\213U\b蹸ÿÿ\213M\bÇ\205pÿÿÿ", array = 0x93f0b2, dict = 0x93f0b2, 
      stream = 0x93f0b2, ref = {num = 9695410, gen = 1070805811}, 
      cmd = 0x93f0b2 "Aïÿÿ\215\205üþÿÿ\211\205Ìþÿÿ\211D$\004\213U\020\211\024$\213E\034\213U\b蹸ÿÿ\213M\bÇ\205pÿÿÿ"}}, {type = objNone, {
      booln = -1998197456, intg = -1998197456, real = 0.49443210000000004, 
      string = 0x88e5ed30, 
      name = 0x88e5ed30 <Address 0x88e5ed30 out of bounds>, 
      array = 0x88e5ed30, dict = 0x88e5ed30, stream = 0x88e5ed30, ref = {
        num = -1998197456, gen = 1071621318}, 
      cmd = 0x88e5ed30 <Address 0x88e5ed30 out of bounds>}}, {type = objNone, 
    {booln = 9970129, intg = 9970129, real = 243.65795926774311, 
      string = 0x9821d1, name = 0x9821d1 "", array = 0x9821d1, 
      dict = 0x9821d1, stream = 0x9821d1, ref = {num = 9970129, 
        gen = 1080980750}, cmd = 0x9821d1 ""}}, {type = objNone, {
      booln = 1175103052, intg = 1175103052, real = 321.07159999999999, 
      string = 0x460aa64c, 
      name = 0x460aa64c <Address 0x460aa64c out of bounds>, 
      array = 0x460aa64c, dict = 0x460aa64c, stream = 0x460aa64c, ref = {
        num = 1175103052, gen = 1081348389}, 
      cmd = 0x460aa64c <Address 0x460aa64c out of bounds>}}, {type = objNone, 
    {booln = 584115553, intg = 584115553, real = 103.96650000000001, 
      string = 0x22d0e561, 
      name = 0x22d0e561 <Address 0x22d0e561 out of bounds>, 
      array = 0x22d0e561, dict = 0x22d0e561, stream = 0x22d0e561, ref = {
        num = 584115553, gen = 1079639515}, 
      cmd = 0x22d0e561 <Address 0x22d0e561 out of bounds>}}, {type = objNone, 
    {booln = 18894849, intg = 18894849, real = 103.9664919677307, 
      string = 0x1205001, name = 0x1205001 <Address 0x1205001 out of bounds>, 
      array = 0x1205001, dict = 0x1205001, stream = 0x1205001, ref = {
        num = 18894849, gen = 1079639515}, 
      cmd = 0x1205001 <Address 0x1205001 out of bounds>}}, {type = objNone, {
      booln = 0, intg = 0, real = 3.1829936864479085e-313, string = 0x0, 
      name = 0x0, array = 0x0, dict = 0x0, stream = 0x0, ref = {num = 0, 
        gen = 15}, cmd = 0x0}}, {type = objNone, {booln = 0, intg = 0, 
      real = 1.0609978954826362e-313, string = 0x0, name = 0x0, array = 0x0, 
      dict = 0x0, stream = 0x0, ref = {num = 0, gen = 5}, cmd = 0x0}}, {
    type = objNone, {booln = 0, intg = 0, real = 4.147511597173144e-305, 
      string = 0x0, name = 0x0, array = 0x0, dict = 0x0, stream = 0x0, ref = {
        num = 0, gen = 12394484}, cmd = 0x0}}, {type = objNone, {booln = 0, 
      intg = 0, real = -5.2919738973059189e-47, string = 0x0, name = 0x0, 
      array = 0x0, dict = 0x0, stream = 0x0, ref = {num = 0, 
        gen = -1236052504}, cmd = 0x0}}, {type = objNone, {booln = 0, 
      intg = 0, real = 1.1594192354523474e-303, string = 0x0, name = 0x0, 
      array = 0x0, dict = 0x0, stream = 0x0, ref = {num = 0, gen = 17396064}, 
      cmd = 0x0}}, {type = objNone, {booln = 0, intg = 0, 
      real = 2.1104108483123122e-305, string = 0x0, name = 0x0, array = 0x0, 
      dict = 0x0, stream = 0x0, ref = {num = 0, gen = 11379646}, cmd = 0x0}}, 
  {type = objNone, {booln = 0, intg = 0, real = -4.3908512752983352e-47, 
      string = 0x0, name = 0x0, array = 0x0, dict = 0x0, stream = 0x0, ref = {
        num = 0, gen = -1236268280}, cmd = 0x0}}, {type = objNone, {
      booln = 0, intg = 0, real = -5.2919738973059189e-47, string = 0x0, 
      name = 0x0, array = 0x0, dict = 0x0, stream = 0x0, ref = {num = 0, 
        gen = -1236052504}, cmd = 0x0}}, {type = objNone, {booln = 0, 
      intg = 0, real = 1.1617500003191337e-303, string = 0x0, name = 0x0, 
      array = 0x0, dict = 0x0, stream = 0x0, ref = {num = 0, gen = 17399416}, 
      cmd = 0x0}}, {type = objNone, {booln = 0, intg = 0, 
      real = -3.0209853257936153e-44, string = 0x0, name = 0x0, array = 0x0, 
      dict = 0x0, stream = 0x0, ref = {num = 0, gen = -1226469640}, 
      cmd = 0x0}}, {type = objNone, {booln = 0, intg = 0, real = 0, 
      string = 0x0, name = 0x0, array = 0x0, dict = 0x0, stream = 0x0, ref = {
        num = 0, gen = 0}, cmd = 0x0}}, {type = objNone, {booln = 0, 
      intg = 0, real = -4.3908512752983352e-47, string = 0x0, name = 0x0, 
      array = 0x0, dict = 0x0, stream = 0x0, ref = {num = 0, 
        gen = -1236268280}, cmd = 0x0}}, {type = objNone, {booln = 0, 
      intg = 0, real = 1.9423837874685077e-303, string = 0x0, name = 0x0, 
      array = 0x0, dict = 0x0, stream = 0x0, ref = {num = 0, gen = 18173940}, 
      cmd = 0x0}}, {type = objNone, {booln = 0, intg = 0, 
      real = -3.0212248055116396e-44, string = 0x0, name = 0x0, array = 0x0, 
      dict = 0x0, stream = 0x0, ref = {num = 0, gen = -1226469528}, 
      cmd = 0x0}}, {type = objNone, {booln = 0, intg = 0, real = 0, 
      string = 0x0, name = 0x0, array = 0x0, dict = 0x0, stream = 0x0, ref = {
        num = 0, gen = 0}, cmd = 0x0}}, {type = objNone, {booln = 0, 
      intg = 0, real = 1.0185579796633307e-312, string = 0x0, name = 0x0, 
      array = 0x0, dict = 0x0, stream = 0x0, ref = {num = 0, gen = 48}, 
      cmd = 0x0}}, {type = objNone, {booln = 0, intg = 0, 
      real = 8.2234488263383384e-304, string = 0x0, name = 0x0, array = 0x0, 
      dict = 0x0, stream = 0x0, ref = {num = 0, gen = 16911299}, cmd = 0x0}}, 
  {type = objNone, {booln = 0, intg = 0, real = 0, string = 0x0, name = 0x0, 
      array = 0x0, dict = 0x0, stream = 0x0, ref = {num = 0, gen = 0}, 
      cmd = 0x0}}, {type = objNone, {booln = 0, intg = 0, real = 0, 
      string = 0x0, name = 0x0, array = 0x0, dict = 0x0, stream = 0x0, ref = {
        num = 0, gen = 0}, cmd = 0x0}}, {type = objNone, {booln = 0, 
      intg = 0, real = -4.1867832706744579e-151, string = 0x0, name = 0x0, 
      array = 0x0, dict = 0x0, stream = 0x0, ref = {num = 0, 
        gen = -1598689905}, cmd = 0x0}}, {type = objNone, {booln = 0, 
      intg = 0, real = 0, string = 0x0, name = 0x0, array = 0x0, dict = 0x0, 
      stream = 0x0, ref = {num = 0, gen = 0}, cmd = 0x0}}}
#5  0x0103fb99 in Gfx::display (this=0xb6504540, obj=0xb6e59064, topLevel=1)
    at Gfx.cc:630
	obj2 = {type = objNone, {booln = 0, intg = 0, 
    real = 9.0016266882163851e-304, string = 0x0, name = 0x0, array = 0x0, 
    dict = 0x0, stream = 0x0, ref = {num = 0, gen = 17023213}, cmd = 0x0}}
	i = <value optimized out>
#6  0x0108a930 in Page::displaySlice (this=0xa2b96f0, out=0xa30cbf8, 
    hDPI=12.100840336134455, vDPI=12.100840336134455, rotate=0, 
    useMediaBox=0, crop=1, sliceX=0, sliceY=0, sliceW=100, sliceH=141, 
    printing=0, catalog=0xa30cb10, abortCheckCbk=0, abortCheckCbkData=0x0, 
    annotDisplayDecideCbk=0, annotDisplayDecideCbkData=0x0) at Page.cc:474
	gfx = (Gfx *) 0xb6504540
	obj = {type = objStream, {booln = -1236052504, intg = -1236052504, 
    real = 7.7783837678119691e-292, string = 0xb65355e8, 
    name = 0xb65355e8 "\b?\025\001\003", array = 0xb65355e8, 
    dict = 0xb65355e8, stream = 0xb65355e8, ref = {num = -1236052504, 
      gen = 58657919}, cmd = 0xb65355e8 "\b?\025\001\003"}}
	i = <value optimized out>
#7  0x0017eae1 in _poppler_page_render_to_pixbuf (page=0xa302c80, src_x=0, 
    src_y=0, src_width=100, src_height=141, scale=0.16806722689075632, 
    rotation=0, printing=0, pixbuf=0xa302960) at poppler-page.cc:778
	data = {
  cairo_data = 0xb6523c10 "ÌâþÿÌâþÿÌâþÿÌâþÿÌâþÿÌâþÿÌâþÿg¤þÿ\004hÿÿ\nhÿÿ\rmÿÿ\020mþÿ\023pÿÿ\025süÿ\026týÿ", surface = 0xb65040a8, cairo = 0xb65041b0}
#8  0x009bbb8d in make_thumbnail_for_page (
    poppler_page=<value optimized out>, rc=<value optimized out>, width=100, 
    height=141) at ev-poppler.cc:1393
	pixbuf = (GdkPixbuf *) 0x0
#9  0x009bbc73 in pdf_document_thumbnails_get_thumbnail (
    document_thumbnails=0xa1ee750, rc=0xa302ca0, border=1)
    at ev-poppler.cc:1456
	poppler_page = (PopplerPage *) 0xa302c80
	pixbuf = <value optimized out>
	border_pixbuf = <value optimized out>
	width = 100
	height = 141
#10 0x00c24d62 in ev_document_thumbnails_get_thumbnail (document=0xa1ee750, 
    rc=0xa302ca0, border=1) at ev-document-thumbnails.c:44
	__PRETTY_FUNCTION__ = "ev_document_thumbnails_get_thumbnail"
#11 0x00e3db65 in ev_job_thumbnail_run (job=0xa2d6288) at ev-jobs.c:678
	rc = (EvRenderContext *) 0xa302ca0
	page = <value optimized out>
#12 0x00e3b0a1 in ev_job_run (job=0xa2d6288) at ev-jobs.c:212
No locals.
#13 0x00e3ebe8 in ev_job_thread_proxy (data=0x0) at ev-job-scheduler.c:183
	job = (EvSchedulerJob *) 0xa3c0218
#14 0x00f4657f in g_thread_create_proxy (data=0xa298388)
    at /build/buildd/glib2.0-2.21.6/glib/gthread.c:635
	__PRETTY_FUNCTION__ = "g_thread_create_proxy"
#15 0x0013f80e in start_thread () from /lib/tls/i686/cmov/libpthread.so.0
No symbol table info available.
#16 0x00b4a7ee in clone () at ../sysdeps/unix/sysv/linux/i386/clone.S:130
No locals."
Comment 1 David Benjamin 2009-11-20 23:19:01 UTC
Created attachment 31363 [details] [review]
Do not restoreState on empty stack

This PDF is horrendously malformed. As far as I can tell, several of the pages have command streams missing spaces and who knows what else. Even acroread will display an error. That said, acroread does not crash and does a much better job rendering it than we do.

The confused command stream results in a restoreState without the right number of saveStates. The above patch causes that to result in a warning.

We probably want to try being lenient about parsing the command stream to allow one to omit the spaces before/after commands? I suspect this is what acroread is doing. This may be tricky as some commands (d0 and d1) end in numbers.
Comment 2 Albert Astals Cid 2009-11-22 10:58:34 UTC
Yes that'd would be the end goal, we already do that this space thing when parsing obj number, see XRef.cc line 963

About this patch, i'd prefer to move it to the CairoOutputDev, the other *OutputDev already do this check, and in a regular project i'd suggest "upstreaming" the checks but here i prefer not to touch existing code to ease future merges with xpdf code (if he ever does a new release) so moving to CairoOutputDev is as good for fixing the bug and eases a bit the merging so if you agree Carlos or I can commit the fix there.
Comment 3 Carlos Garcia Campos 2009-11-22 11:03:11 UTC
Yes, go ahead. 
Comment 4 Albert Astals Cid 2009-11-22 11:20:47 UTC
Pushed, should we open a different bug for the rendering thing?
Comment 5 David Benjamin 2009-11-22 20:22:49 UTC
It might be worth checking the PDF more carefully if it's really just a spaces thing. I see a block

442.5542 466.4069 l
442.4207 466.2510 442.2421 466.0923 442.0182 465.9284 c
441.7940 46.63657 441.5888 465.6430 441.4008 465.5602 c
441.4008 466.2391 l
441.7194 466.4217 441.9995 466.6445 442.2436 466.9070 c
442.4862 467.1711 442.6580 46
h2.71 442.7566 46
h6777 c
443.1314 46
h6777 l
h
h
444.6473 466.9030 m

which only makes sense as far as argument counts go if we just ignore the h2.71 and h6777 tokens. A first attempt at quickly implementing something like that didn't seem to make the PDF render right. Though, I could have decompressed it wrong --- what I did was rather hacky. Is there a tool somewhere that will decode all the streams of a PDF so that one may easily view the command streams in plain text?
Comment 6 Albert Astals Cid 2009-11-23 13:12:52 UTC
you can try podofobrowser, i'm working on a "uncompress" tool based on poppler since we have all the bits, just never found the time :D
Comment 7 Carlos Garcia Campos 2009-11-25 05:59:03 UTC
Now, I see the error message with any PDF document:

"Error: restoreState on an empty state stack"

so, I'm not sure the patch is right. 
Comment 8 David Benjamin 2009-11-25 08:00:45 UTC
It looks like that got introduced when porting the patch down to CairoOutputDev. I get this error on a random PDF with master, but not the original patch.

I think it's because we call state->restore() before the output device sees anything.

 void Gfx::restoreState() {
   state = state->restore();
   out->restoreState(state);
 } 
Comment 9 Carlos Garcia Campos 2009-11-25 08:47:57 UTC
What about just something like this?

-  mask = ms->mask;
-  maskStack = ms->next;
-  delete ms;
+  if (ms) {
+    mask = ms->mask;
+    maskStack = ms->next;
+    delete ms;
+  }

that's what we already do with other stacks. 
Comment 10 Carlos Garcia Campos 2009-11-27 10:52:57 UTC
Fianlly fixed it in git master and poppler-0.12 branch. 
Comment 11 David Benjamin 2009-11-27 12:51:53 UTC
I'm... not convinced this actually works. I think the patch as it is still can cause a crash.

Say we're at a point where the state stack is empty, but we have set a mask. So out->mask is not null. Then we call Gfx::restoreState. In the current code, we call CairoOutputDev::restoreState which reaches this code:

  MaskStack* ms = maskStack;
  if (mask)
    cairo_pattern_destroy(mask);

  if (ms) {
    mask = ms->mask;
    maskStack = ms->next;
    delete ms;
  }

This frees the current mask, but fails to replace it with a different one because ms is NULL. Future uses of mask will then access freed memory and cause problems.

We could instead write

  if (mask) {
    cairo_pattern_destroy(mask);
    mask = NULL;
  }

which wouldn't crash, but then an erroneous restoreState would clear your mask --- something I'm not convinced is desirable.

The main problem being that, if we do a Gfx::restoreState on an empty stack, it really should be a no-op[1]. However, without the original patch of checking in the generic layer, we still call an OutputDev::restoreState, which is then responsible for enforcing this. This is messy, and, more importantly, actually difficult to detect without extra state. In both the case of restoring to an empty stack and restoring to a 1-element stack, OutputDev::restoreState receives the same GfxState object and has no way of distinguishing between the two without maintaining its own state, which seems rather poor.

In the case of CairoOutputDev, on any restoreState, we forcibly pop the mask, cairo context and cairo_shape context[2].

I think it's worth adding the check to the output-independent part even if future merges with xpdf may be somewhat of a nuisance. Especially if it turns out acroread does something fancy on empty restoreState[1], we'll need to add code there anyway. Also, it looks like the last xpdf release was almost 3 years ago.


[1] Or maybe a restore to default state? Might be worth seeing exactly what acroread does here.

[2] Actually, I think the logic here doesn't work if cairo_shape is introduced/removed on anything other than the bottom-most stack. I'll try to come up with a test case and file a bug if it indeed doesn't work.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.