bugGnash - The GNU Flash player - Bugs: bug #32510, SIGSEGV from advanceLiveChar

 
 

bug #32510: SIGSEGV from advanceLiveChar

Submitted by:  Bastiaan Jacques <bjacques>
Submitted on:  Thu 17 Feb 2011 01:04:11 AM UTC  
 
Category: coreSeverity: 6 - Security
Release: masterStatus: Fixed
Privacy: PublicAssigned to: Benjamin Wolsey <bwy>
Open/Closed: Closed

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)

Mon 21 Feb 2011 08:08:12 PM UTC, comment #17:

Well done!

Bastiaan Jacques <bjacques>
Project Member
Mon 21 Feb 2011 01:32:45 PM UTC, comment #16:

Should be fixed in trunk.

Benjamin Wolsey <bwy>
Project MemberIn charge of this item.
Mon 21 Feb 2011 09:04:36 AM UTC, comment #15:

It seems that a PlaceObject call is adding a child MovieClip to a parent that has already had destroy() called on it and been removed from the list of live chars.

That means that the child MovieClip never has destroy() called on it, so it remains in the list of live chars.

If that's really that case, it's amazing that the bug hasn't appeared elsewhere!

Benjamin Wolsey <bwy>
Project MemberIn charge of this item.
Mon 21 Feb 2011 08:17:24 AM UTC, comment #14:

An even simple XML file attached.

The invalid access is to the character 50, placed at depth 3. The SWF only uses place and remove using PlaceObject2 tags.

(file #22743)

Benjamin Wolsey <bwy>
Project MemberIn charge of this item.
Sat 19 Feb 2011 09:11:58 PM UTC, comment #13:

Attaching swfmill XML and SWF that reproduces the bug.

(file #22733, file #22734)

Benjamin Wolsey <bwy>
Project MemberIn charge of this item.
Sat 19 Feb 2011 02:26:18 PM UTC, comment #12:

The MovieClip does in fact get placed in a DisplayList, but that list is never displayed and is destroyed shortly afterwards on destruction of the MovieClip that owns it.

The real problem appears to be that a MovieClip is destroyed (in the C++ sense) without all its children being destroyed or unloaded (in the Gnash sense). Only unloaded clips are removed from the live character list by movie_root. So when our MovieClip is deleted, its child MovieClips are still in movie_root's list of live chars without anything to mark them.

I still haven't found the circumstances where this can happen, but this crash also happens in 0.8.6 and 0.8.7. The only place where it's actually supposed to happen is on exit, where all GC resources are destroyed en masse on the assumption that no further access will take place, including from the dtors of other GC resources. But this crash does not happen on exit.

When destroy() or unload() is called on a MovieClip, the same functions should be called recursively on all its children, so the current assumption is that somewhere a parent MovieClip is both unmarked and not unloaded.

Benjamin Wolsey <bwy>
Project MemberIn charge of this item.
Fri 18 Feb 2011 01:42:19 PM UTC, comment #11:

The second bug is a MovieClip that is constructed then deleted, apparently without ever appearing in a DisplayList.

Benjamin Wolsey <bwy>
Project MemberIn charge of this item.
Fri 18 Feb 2011 10:21:11 AM UTC, comment #10:

The first bug was due to a character at depth 65535 being left out of the merge, so it was neither present in the display list after looping nor destroyed. That meant it was present in the list of live chars but not marked by any display list, so the GC deleted it.

It looks like a mistake in the comparison predicate (>= 65535 rather than > 65535) caused this one.

The second SWF indeed has the same stack trace, but it's not from an error in merging as mergeDisplayList is never called.

Benjamin Wolsey <bwy>
Project MemberIn charge of this item.
Thu 17 Feb 2011 12:06:33 PM UTC, comment #9:

I get the same stacktrace from:

http://spe.atdmt.com/ds/NMMRTUMISWXP/FY11_Win7_HolCloud_RemCITY/FY11_Win7_HolCloud_RemTokyo_160x600.swf

Bastiaan Jacques <bjacques>
Project Member
Thu 17 Feb 2011 10:28:23 AM UTC, comment #8:

The problem is almost certainly in merging the display list when looping back to the beginning.

Benjamin Wolsey <bwy>
Project MemberIn charge of this item.
Thu 17 Feb 2011 09:54:43 AM UTC, comment #7:

Also same assertion in 0.8.6!

Benjamin Wolsey <bwy>
Project MemberIn charge of this item.
Thu 17 Feb 2011 09:38:27 AM UTC, comment #6:

Same assertion in 0.8.7.

Benjamin Wolsey <bwy>
Project MemberIn charge of this item.
Thu 17 Feb 2011 09:18:08 AM UTC, comment #5:

in 0.8.8 the following assertion fails:

void gnash::DisplayList::mergeDisplayList(gnash::DisplayList&): assertion »itNewEnd == dlistTagsEffectiveZoneEnd(newList._charsByDepth)

Benjamin Wolsey <bwy>
Project MemberIn charge of this item.
Thu 17 Feb 2011 08:17:18 AM UTC, comment #4:

line 1703 of movie_root.cpp has the full story.
#define GNASH_PARANOIA_LEVEL 2
to get more aborts

Sandro Santilli <strk>
Project Member
Thu 17 Feb 2011 08:15:38 AM UTC, comment #3:

So the GC is cleaning a MovieClip that is still in the list of live chars (_liveChars). The _liveChars list is not marked because we intend that MovieClips are removed from it before being destroyed.

Compiling with GNASH_PARANOIA_LEVEL > 0 ought to cause an assertion failure in this case.

Benjamin Wolsey <bwy>
Project MemberIn charge of this item.
Thu 17 Feb 2011 08:14:29 AM UTC, comment #2:

live chars (to be advanced) are not being gc-marked ?

Sandro Santilli <strk>
Project Member
Thu 17 Feb 2011 08:08:57 AM UTC, comment #1:

==8800== Thread 1:
==8800== Invalid read of size 1
==8800== at 0x4165459: gnash::(anonymous namespace)::advanceLiveChar(gnash::MovieClip*) (movie_root.cpp:2527)
==8800== by 0x41653A8: gnash::movie_root::advanceLiveChars() (bind.hpp:253)
==8800== by 0x416CF69: gnash::movie_root::advanceMovie() (movie_root.cpp:884)
==8800== by 0x41752AB: gnash::movie_root::advance() (movie_root.cpp:832)
==8800== by 0x807D72C: gnash::Gui::advanceMovie(bool) (gui.cpp:970)
==8800== by 0x809C648: gnash::Gui::advance_movie(gnash::Gui*) (gui.h:285)
==8800== by 0x4DE1FCB: ??? (in /lib/libglib-2.0.so.0.2600.0)
==8800== by 0x4DE1854: g_main_context_dispatch (in /lib/libglib-2.0.so.0.2600.0)
==8800== by 0x4DE5667: ??? (in /lib/libglib-2.0.so.0.2600.0)
==8800== by 0x4DE5BA6: g_main_loop_run (in /lib/libglib-2.0.so.0.2600.0)
==8800== by 0x4A101D8: gtk_main (in /usr/lib/libgtk-x11-2.0.so.0.2200.0)
==8800== by 0x80972AD: gnash::GtkGui::run() (gtk.cpp:278)
==8800== Address 0x6cfb58b is 179 bytes inside a block of size 324 free'd
==8800== at 0x4026504: operator delete(void*) (vg_replace_malloc.c:387)
==8800== by 0x418657D: gnash::MovieClip::~MovieClip() (MovieClip.cpp:500)
==8800== by 0x48AC3DB: gnash::GC::cleanUnreachable() (GC.cpp:84)
==8800== by 0x48AC44B: gnash::GC::runCycle() (GC.cpp:125)
==8800== by 0x416CF2B: gnash::movie_root::cleanupAndCollect() (GC.h:251)
==8800== by 0x416CF87: gnash::movie_root::advanceMovie() (movie_root.cpp:900)
==8800== by 0x41752AB: gnash::movie_root::advance() (movie_root.cpp:832)
==8800== by 0x807D72C: gnash::Gui::advanceMovie(bool) (gui.cpp:970)
==8800== by 0x809C648: gnash::Gui::advance_movie(gnash::Gui*) (gui.h:285)
==8800== by 0x4DE1FCB: ??? (in /lib/libglib-2.0.so.0.2600.0)
==8800== by 0x4DE1854: g_main_context_dispatch (in /lib/libglib-2.0.so.0.2600.0)
==8800== by 0x4DE5667: ??? (in /lib/libglib-2.0.so.0.2600.0)
==8800==

Benjamin Wolsey <bwy>
Project MemberIn charge of this item.
Thu 17 Feb 2011 01:04:11 AM UTC, original submission:

STR: gui/gnash http://s0.2mdn.net/1327335/18jun10_v2_728x90_24_Bereikbaarheid_trein_RegioUtrecht.swf

(wait approx. 10 seconds)
#0 0x0000003f in ?? ()
#1 0x00437244 in gnash::(anonymous namespace)::advanceLiveChar (mo=0x8280a38)
at /home/bastiaan/gnash/master/libcore/movie_root.cpp:2531
#2 0x004493be in boost::_bi::list1<boost::arg<1> >::operator()<void ()(gnash::MovieClip), boost::_bi::list1<gnash::MovieClip*&> > (this=0xbfffd9c0, f=@0xbfffd9bc, a=...) at /usr/include/boost/bind/bind.hpp:253
#3 0x004449e6 in boost::_bi::bind_t<void, void ()(gnash::MovieClip), boost::_bi::list1<boost::arg<1> > >::operator()<gnash::MovieClip*> (this=0xbfffd9bc, a1=@0x8280dd8) at /usr/include/boost/bind/bind_template.hpp:32
#4 0x0043efa9 in std::for_each<std::_List_iterator<gnash::MovieClip>, boost::_bi::bind_t<void, void ()(gnash::MovieClip*), boost::_bi::list1<boost::arg<1> > > > (__first=, __last=, __f=...)
at /usr/lib/gcc/i686-redhat-linux/4.6.0/../../../../include/c++/4.6.0/bits/stl_algo.h:4275
#5 0x00434b52 in gnash::movie_root::advanceLiveChars (this=0xbfffdfcc)
at /home/bastiaan/gnash/master/libcore/movie_root.cpp:2001
#6 0x0042fcb9 in gnash::movie_root::advanceMovie (this=0xbfffdfcc)
at /home/bastiaan/gnash/master/libcore/movie_root.cpp:884
#7 0x0042fb3a in gnash::movie_root::advance (this=0xbfffdfcc)
at /home/bastiaan/gnash/master/libcore/movie_root.cpp:832
#8 0x080cec09 in gnash::Gui::advanceMovie (this=0x818db88, doDisplay=true)
at /home/bastiaan/gnash/master/gui/gui.cpp:970
#9 0x0810ef52 in gnash::Gui::advance_movie (gui=0x818db88) at /home/bastiaan/gnash/master/gui/gui.h:285
#10 0x00d838fd in g_timeout_dispatch (source=0x826b018, callback=
0x810ef28 <gnash::Gui::advance_movie(gnash::Gui*)>, user_data=0x818db88) at gmain.c:3585
#11 0x00d82192 in g_main_dispatch (context=0x81b65e8) at gmain.c:2149
#12 g_main_context_dispatch (context=0x81b65e8) at gmain.c:2702
#13 0x00d82978 in g_main_context_iterate (context=0x81b65e8, block=1, dispatch=1, self=<value optimized out>)
at gmain.c:2780
#14 0x00d8304b in g_main_loop_run (loop=0x827d248) at gmain.c:2988
#15 0x0679a499 in IA__gtk_main () at gtkmain.c:1237

Original report at https://bugzilla.redhat.com/show_bug.cgi?id=607887

Bastiaan Jacques <bjacques>
Project Member

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #22743:  FY2.xml added by bwy (2KiB - text/xml)
file #22733:  FY.xml added by bwy (3KiB - text/xml)
file #22734:  FY.swf added by bwy (215B - application/x-shockwave-flash)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by strk (Posted a comment)
  • -unavailable- added by bwy (Posted a comment)
  • -unavailable- added by bjacques (Submitted the item)
  •  

    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 logged-in users can vote.

     

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

     

     

    Follow 7 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 21 Feb 2011 08:08:12 PM UTCbjacquesStatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
    Mon 21 Feb 2011 01:32:45 PM UTCbwyStatusNone=>Ready For Test
    Mon 21 Feb 2011 01:18:25 PM UTCbwyAssigned toNone=>bwy
    Mon 21 Feb 2011 08:17:24 AM UTCbwyAttached File-=>Added FY2.xml, #22743
    Sat 19 Feb 2011 09:11:58 PM UTCbwyAttached File-=>Added FY.xml, #22733
      Attached File-=>Added FY.swf, #22734

    Back to the top


    Powered by Savane 3.1-cleanup1