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

 
 

bug #32510: SIGSEGV from advanceLiveChar

Submitter:  Bastiaan Jacques <bjacques>
Submitted:  Thu 17 Feb 2011 01:04:11 AM UTC
   
 
Category:  core Severity:  6 - Security
Release:  master Status:  Fixed
Privacy:  Public Assigned to:  bwy
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

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

Well done!

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

Should be fixed in trunk.

Benjamin Wolsey <bwy>
Group Member
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>
Group Member
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>
Group Member
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>
Group Member
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>
Group Member
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>
Group Member
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>
Group Member
Thu 17 Feb 2011 12:06:33 PM UTC, comment #9: 
Bastiaan Jacques <bjacques>
Group 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>
Group Member
Thu 17 Feb 2011 09:54:43 AM UTC, comment #7: 

Also same assertion in 0.8.6!

Benjamin Wolsey <bwy>
Group Member
Thu 17 Feb 2011 09:38:27 AM UTC, comment #6: 

Same assertion in 0.8.7.

Benjamin Wolsey <bwy>
Group Member
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>
Group Member
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>
Group 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>
Group Member
Thu 17 Feb 2011 08:14:29 AM UTC, comment #2: 

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

Sandro Santilli <strk>
Group 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>
Group Member
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>
Group Member

 

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

Attach Files:
   
   
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
  • -email is unavailable- added by strk (Posted a comment)
  • -email is unavailable- added by bwy (Posted a comment)
  • -email is unavailable- added by bjacques (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 logged-in users can vote.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2011-02-21 bjacques StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2011-02-21 bwy StatusNone Ready For Test
    2011-02-21 bwy Assigned toNone bwy
    2011-02-21 bwy Attached File- Added FY2.xml, #22743
    2011-02-19 bwy Attached File- Added FY.xml, #22733
        Attached File- Added FY.swf, #22734

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code