Re: [Gnash-commit] /srv/bzr/gnash/avm2 r9685: Fix segfault caused by null objects in the scope stack.

[strk]

On Sat, Nov 15, 2008 at 11:41:54AM +0800, Tom Stellard wrote:
> On Sat, Nov 15, 2008 at 4:36 AM, strk <address@hidden> wrote:
> > On Sat, Nov 15, 2008 at 02:19:01AM +0800, Tom Stellard wrote:
> >> ------------------------------------------------------------
> >> revno: 9685
> >> committer: Tom Stellard <address@hidden>
> >> branch nick: gnash_dev
> >> timestamp: Sat 2008-11-15 02:19:01 +0800
> >> message:
> >>   Fix segfault caused by null objects in the scope stack.
> >> modified:
> >>   libcore/as_environment.cpp
> >>   libcore/vm/Machine.cpp
> >
> >> === modified file 'libcore/as_environment.cpp'
> >> --- a/libcore/as_environment.cpp      2008-11-06 19:04:35 +0000
> >> +++ b/libcore/as_environment.cpp      2008-11-14 18:19:01 +0000
> >> @@ -679,6 +679,7 @@
> >>                               {
> >>                                       // const_cast needed due to 
> >> non-const as_object::get_member
> >>                                       as_object* obj = 
> >> const_cast<as_object*>((*scopeStack)[i-1].get());
> >> +                                     if(!obj) continue;
> >
> > Shouldn't we avoid putting NULLs in the scope stack instead ?
> > How did it get there ?
> >
> > --strk;
> >
> 
> NULLs are added to the scope stack when AVM2 can't find the object it
> is looking for.  This usually happens when there is an unimplemented
> actionscript class, or when AVM2 didn't create an object when it was
> supposed to.  I think I could probably add code in push_scope_stack()
> that prevents NULL objects from being added to the scope stack.

The aim here is to find a programmatic error.
Pushing a NULL in the scope stack would be a programmatic error,
so silently behaving in a weird way is what you want to avoid.
The best would be to log an AS error from the caller, whenever
it's about to push a NULL.
In this way, you can further inspect the problem producing a
focused testcase.

Doing so, I found out what the behaviour for AVM1 was in this case.
AVM1 can push an object to the scope-stack for a block of code,
using 'with':

        with (myObject)
        {
                doThis();
                doThat();
        }

By having the scope pusher abort you can easily get a backtrace
and find the caller. By having the caller log_aserror and avoid
the push you can easily find movies doing so. Once you find the
movie you can check the expected behaviour.
In the 'with (undefined)' case, for example, the expected
behaviour was to completely skip the inside block!
I would have had an hard time figuring if I didn't have an AS
error logged, and an abort to remind me I should have avoided
the call completely.

--strk;