bugGnash - The GNU Flash player - Bugs: bug #23489, recursion testcase crashes Gnash

 
 

bug #23489: recursion testcase crashes Gnash

Submitter:  Benjamin Wolsey <bwy>
Submitted:  Wed 04 Jun 2008 07:59:01 PM UTC
   
 
Category:  ActionScript Severity:  5 - Blocker
Release:  None Status:  Fixed
Privacy:  Public Assigned to:  strk
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 31 Aug 2008 02:41:48 PM UTC, comment #8: 

Cool. Fixed.

Benjamin Wolsey <bwy>
Group Member
Fri 29 Aug 2008 10:10:29 PM UTC, comment #7: 

I've committed use of SafeStack in as_environment.
It pushed me further so I also changed stack management
to drop the stack smashing protection.
We still log an error but don't try to recover, fixing
tests in Function.as showing "no-fix" being the correct
behaviour.

I'm sure there are many more pending incompatibilities
in stack management: namely all the places in which
we aim at optimization deferring dropping stack elements
to the end of operations. Any operation involving
calls to user-defined function may trigger the call
with too much elements on the stack, thus preventing
code exploiting stack smashing from working correctly.

I guess we can postpone fixing this till we have testcases
showing the problem (like getter-setter making use of
stack element not explicitly pushed).

Sandro Santilli <strk>
Group Member
Thu 28 Aug 2008 01:17:00 AM UTC, comment #6: 

I'll work on using SafeStack in as_environment, and on
having top(), bottom() and drop() be immune to out-of-range
(ie: return undefined or simply keep going).
Should simplify things.

Beside, SafeStack allows returning by-ref on pop() too,
which would avoid some copies. Still, dangers would still
be pending doing so as code like:

  as_value& tmp = env.pop();
  env.push(as_value(2)); // <-- overrides tmp above

Sandro Santilli <strk>
Group Member
Thu 28 Aug 2008 01:01:15 AM UTC, comment #5: 

Since we're talking about it:
SafeStack only misses the "pad" feature we have
for the vector-based current stack.
The 'pad' feature in turn is only used to have
'undefined' returned in place of any missing
stack value when an opcode needs them and we have not enough.

First our way to pad is bogus in that the PP (at least AVM1)
has been found to allow stack smashing, so for instance
you can have a called function consume the stack value
in the context of the caller. This means we shoudn't
really pad 'in-between' (ie: since start of our current
"relative" start).

Second, if we never set a limit (bottom) on the stack
we may as well only return as_value() [ or a default T
for the safe-stack case ] when top() or pop() are
out of range.

Sandro Santilli <strk>
Group Member
Thu 28 Aug 2008 12:46:08 AM UTC, comment #4: 

The attached patch fixes the problem, and shows a very dangerous spot of gnash: the VM stack.
I belive Chad's "SafeStack" was exactly designed to work around such bugs, that is invalidation of references.
In a few words:
  1) ActionSetMember takes the reference of a value on stack
  2) The reference is passed around
  3) Eventually, a function call (getter-setter invocation)
     pushes something on the stack
  4) Incidentally, the push-on-stack triggers reallocation
     so that the reference taken before ends up pointing
     to arbitrary memory.

The patch simply copies the as_value in the opcode handler,
but there are sooo many cases in which we use reference and
hardly control whether or not something is pushed on the
stack before we've done using them that it's likely much
better changing the as_environment stack to be a SafeStack...

Beside, Chad plan was to use Machine for both AVM1 and AVM2
so any move toward use of the new classes should be good.

PS: I'm out of office, so if anyone else wants to commit this please go ahead.


(file #16373)

Sandro Santilli <strk>
Group Member
Fri 08 Aug 2008 05:54:42 PM UTC, comment #3: 

This is still a problem. The rest of the backtrace is here:

    at /usr/include/boost/variant/detail/visitation_impl.hpp:203
#4  0x00002b5721c27056 in gnash::as_value::operator= (
    this=<value optimized out>, v=<value optimized out>)
    at /usr/include/boost/variant/variant.hpp:1738
#5  0x00002b5721bf87dc in gnash::Property::setDelayedValue (
    this=<value optimized out>, this_ptr=@0x94d0c0, value=@0x94d6c0)
    at /home/benjamin/Downloads/gnash/trunk/libcore/Property.h:162
#6  0x00002b5721bf9df1 in gnash::Property::setValue (this=0x94db20,
    this_ptr=@0x37a1, value=@0x94d6c0)
    at /home/benjamin/Downloads/gnash/trunk/libcore/Property.cpp:190
#7  0x00002b5721c2101e in gnash::as_object::set_member_default (this=0x94d0c0,
    key=361, val=@0x94d6c0, nsname=0, ifFound=false)
    at /home/benjamin/Downloads/gnash/trunk/libcore/as_object.cpp:590
#8  0x00002b5721c1842d in gnash::as_object::set_member (this=0x37a1,
    key=14241, val=@0x6, nsname=18446744073709551615, ifFound=false)
    at /home/benjamin/Downloads/gnash/trunk/libcore/as_object.h:278
#9  0x00002b5721df69fe in gnash::SWF::SWFHandlers::ActionSetMember (
    thread=@0x7fff8992af70)
    at /home/benjamin/Downloads/gnash/trunk/libcore/vm/ASHandlers.cpp:3397
#10 0x00002b5721de572b in gnash::SWF::SWFHandlers::execute (
    this=<value optimized out>, type=<value optimized out>,
    thread=@0x7fff8992af70)
    at /home/benjamin/Downloads/gnash/trunk/libcore/vm/ASHandlers.cpp:459

Boost hocus-pocus doesn't make the cause of the error very obvious.

Benjamin Wolsey <bwy>
Group Member
Wed 04 Jun 2008 08:52:08 PM UTC, comment #2: 

I have no idea when this started - I only found the AS code while looking for recursion tests.

Don't know about 0.8.3 either.

Benjamin Wolsey <bwy>
Group Member
Wed 04 Jun 2008 08:23:55 PM UTC, comment #1: 

Sounds like a regression, any idea since when ?
Is 0.8.3 affected ?

Sandro Santilli <strk>
Group Member
Wed 04 Jun 2008 07:59:01 PM UTC, original submission:  

file #15128:  recursiveGetterSetter2.as from bug #22420 causes gnash to fail an immensely long boost assertion.

Just for fun:

lt-gtk-gnash: /usr/include/boost/variant/detail/visitation_impl.hpp:203: typename Visitor::result_type boost::detail::variant::visitation_impl(int, int, Visitor&, VPCV, mpl_::true_, NBF, W*, S*) [with W = boost::detail::variant::visitation_impl(int, int, Visitor&, VoidPtrCV, mpl_::false_, NoBackupFlag, Which*, step0*) [with Which = boost::variant<T0, T1, T2, T3, T4, T5, T6, T7, T8, T9, T10, T11, T12, T13, T14, T15, T16, T17, T18, T19>::internal_apply_visitor_impl(int, int, Visitor&, VoidPtrCV) [with Visitor = boost::variant<boost::blank, double, bool, boost::intrusive_ptr<gnash::as_object>, gnash::as_value::CharacterProxy, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_>::assigner, VoidPtrCV = const void*, T0_ = boost::blank, T1 = double, T2 = bool, T3 = boost::intrusive_ptr<gnash::as_object>, T4 = gnash::as_value::CharacterProxy, T5 = std::basic_string<char, std::char_traits<char>, std::allocator<char> >, T6 = boost::detail::variant::void_, T7 = boost::detail::variant::void_, T8 = boost::detail::variant::void_, T9 = boost::detail::variant::void_, T10 = boost::detail::variant::void_, T11 = boost::detail::variant::void_, T12 = boost::detail::variant::void_, T13 = boost::detail::variant::void_, T14 = boost::detail::variant::void_, T15 = boost::detail::variant::void_, T16 = boost::detail::variant::void_, T17 = boost::detail::variant::void_, T18 = boost::detail::variant::void_, T19 = boost::detail::variant::void_]::first_which, step0 = boost::variant<T0, T1, T2, T3, T4, T5, T6, T7, T8, T9, T10, T11, T12, T13, T14, T15, T16, T17, T18, T19>::internal_apply_visitor_impl(int, int, Visitor&, VoidPtrCV) [with Visitor = boost::variant<boost::blank, double, bool, boost::intrusive_ptr<gnash::as_object>, gnash::as_value::CharacterProxy, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_>::assigner, VoidPtrCV = const void*, T0_ = boost::blank, T1 = double, T2 = bool, T3 = boost::intrusive_ptr<gnash::as_object>, T4 = gnash::as_value::CharacterProxy, T5 = std::basic_string<char, std::char_traits<char>, std::allocator<char> >, T6 = boost::detail::variant::void_, T7 = boost::detail::variant::void_, T8 = boost::detail::variant::void_, T9 = boost::detail::variant::void_, T10 = boost::detail::variant::void_, T11 = boost::detail::variant::void_, T12 = boost::detail::variant::void_, T13 = boost::detail::variant::void_, T14 = boost::detail::variant::void_, T15 = boost::detail::variant::void_, T16 = boost::detail::variant::void_, T17 = boost::detail::variant::void_, T18 = boost::detail::variant::void_, T19 = boost::detail::variant::void_]::first_step, Visitor = boost::variant<boost::blank, double, bool, boost::intrusive_ptr<gnash::as_object>, gnash::as_value::CharacterProxy, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_>::assigner, VoidPtrCV = const void*, NoBackupFlag = boost::variant<boost::blank, double, bool, boost::intrusive_ptr<gnash::as_object>, gnash::as_value::CharacterProxy, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_>::has_fallback_type_]::next_which, S = boost::detail::variant::visitation_impl(int, int, Visitor&, VoidPtrCV, mpl_::false_, NoBackupFlag, Which*, step0*) [with Which = boost::variant<T0, T1, T2, T3, T4, T5, T6, T7, T8, T9, T10, T11, T12, T13, T14, T15, T16, T17, T18, T19>::internal_apply_visitor_impl(int, int, Visitor&, VoidPtrCV) [with Visitor = boost::variant<boost::blank, double, bool, boost::intrusive_ptr<gnash::as_object>, gnash::as_value::CharacterProxy, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_>::assigner, VoidPtrCV = const void*, T0_ = boost::blank, T1 = double, T2 = bool, T3 = boost::intrusive_ptr<gnash::as_object>, T4 = gnash::as_value::CharacterProxy, T5 = std::basic_string<char, std::char_traits<char>, std::allocator<char> >, T6 = boost::detail::variant::void_, T7 = boost::detail::variant::void_, T8 = boost::detail::variant::void_, T9 = boost::detail::variant::void_, T10 = boost::detail::variant::void_, T11 = boost::detail::variant::void_, T12 = boost::detail::variant::void_, T13 = boost::detail::variant::void_, T14 = boost::detail::variant::void_, T15 = boost::detail::variant::void_, T16 = boost::detail::variant::void_, T17 = boost::detail::variant::void_, T18 = boost::detail::variant::void_, T19 = boost::detail::variant::void_]::first_which, step0 = boost::variant<T0, T1, T2, T3, T4, T5, T6, T7, T8, T9, T10, T11, T12, T13, T14, T15, T16, T17, T18, T19>::internal_apply_visitor_impl(int, int, Visitor&, VoidPtrCV) [with Visitor = boost::variant<boost::blank, double, bool, boost::intrusive_ptr<gnash::as_object>, gnash::as_value::CharacterProxy, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_>::assigner, VoidPtrCV = const void*, T0_ = boost::blank, T1 = double, T2 = bool, T3 = boost::intrusive_ptr<gnash::as_object>, T4 = gnash::as_value::CharacterProxy, T5 = std::basic_string<char, std::char_traits<char>, std::allocator<char> >, T6 = boost::detail::variant::void_, T7 = boost::detail::variant::void_, T8 = boost::detail::variant::void_, T9 = boost::detail::variant::void_, T10 = boost::detail::variant::void_, T11 = boost::detail::variant::void_, T12 = boost::detail::variant::void_, T13 = boost::detail::variant::void_, T14 = boost::detail::variant::void_, T15 = boost::detail::variant::void_, T16 = boost::detail::variant::void_, T17 = boost::detail::variant::void_, T18 = boost::detail::variant::void_, T19 = boost::detail::variant::void_]::first_step, Visitor = boost::variant<boost::blank, double, bool, boost::intrusive_ptr<gnash::as_object>, gnash::as_value::CharacterProxy, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_>::assigner, VoidPtrCV = const void*, NoBackupFlag = boost::variant<boost::blank, double, bool, boost::intrusive_ptr<gnash::as_object>, gnash::as_value::CharacterProxy, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_>::has_fallback_type_]::next_step, Visitor = boost::variant<boost::blank, double, bool, boost::intrusive_ptr<gnash::as_object>, gnash::as_value::CharacterProxy, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_>::assigner, VPCV = const void*, NBF = boost::variant<boost::blank, double, bool, boost::intrusive_ptr<gnash::as_object>, gnash::as_value::CharacterProxy, std::basic_string<char, std::char_traits<char>, std::allocator<char> >, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_, boost::detail::variant::void_>::has_fallback_type_]: Zusicherung »false« nicht erfüllt.

Benjamin Wolsey <bwy>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #16373:  CopyValueBeforePushing.patch added by strk (424B - application/octet-stream)

 

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 (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 10 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2008-08-31 bwy StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2008-08-29 strk StatusIn Progress Ready For Test
    2008-08-28 strk StatusConfirmed In Progress
    2008-08-28 strk Attached File- Added CopyValueBeforePushing.patch, #16373
    2008-08-26 strk StatusNone Confirmed
        Assigned toNone strk
    2008-08-08 bwy Severity4 - Important 5 - Blocker
    2008-06-04 strk CategoryNone ActionScript
        Severity3 - Normal 4 - Important

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code