It seems nobody read this mail, anyway here's a quick update. I've committed the "push_before" approach for stack underrun fixes. top() and pop() methods are still there, so the possible corruptions are all still pending.
--strk; On Thu, Dec 14, 2006 at 03:45:02PM +0100, strk wrote: > This is a long mail, if you want to jump to the point scroll > down to the *final question* line :) > > While fixing handling of malformed SWF produced by Ming (delete/delete2). > I found out that our current handling of "Stack underrun" seems to differ > from the one in the MM player. I'm saying this because it is weird that > nobody noticed the Ming bug in the past, so I'm sure the MM player > was handling that specific malformation in a different way. > > In particular, when Gnash is in a stack underrun condition, that > means it expects to find N elements on the stack but only finds > N-M items there. In this case Gnash pushes M undefined values there. > > Since TAG arguments are put on the stack in reverse order (1 argument > last), this results in pushing undefined values as first argument > rather then as last. > > With ActionDelete (0x3A), this results in a different handling of the bogus > SWF. > ActionDelete is supposed to do: > > // Stack: OBJECT, FIELD > pop FIELD > pop OBJECT > delete OBJECT.FIELD > > With the bogus Ming version, the stack lacks the OBJECT part, as in: > > // Stack: FIELD > pop FIELD -- ok! > pop OBJECT -- not there! > > What happens with the MM player (most likely) is that the second > pop returns an UNDEFINED value, and this might also be what Gnash > was doing some time ago (in 0.7.1 - before stack underrun handling > was introduced). So for MM player this becomes: > > // Stack: FIELD > pop FIELD -- ok ! > pop OBJECT -- get UNDEFINED > delete FIELD (undefined object == _root) > > With the new stack underrun handlign code in Gnash, this becomes: > > // Stack: FIELD > <stack underrun detected, fixing> > // Stack: FIELD, UNDEFINED > pop FIELD -- get UNDEFINED: wrong ! > pop OBJECT -- get FIELD: wrong ! > delete FIELD.UNDEFINED : wrong ! > > > The whole problem with Gnash on this aspect is use of > as_environment::top(N) to get elements of the stack rather > then use of ::pop() and ::push(). > > top(N) returns an as_value by reference. Previous versions > of Gnash were returning a reference to a static as_value, initially > undefined (!!). That was dangerous and confusing for many reasons, > including the fact that some tags handler might have changed that > static variable value from undefined to something else when > trying to set the return value (remember, tag handlers often > use top(x).set_y(y) to return!). Also, that never fixed the stack. > > I guess top(N) is used trying to be more performant and avoid > continuos memory allocation/deallocation due to pop/push on > the stack, but to handle malformed SWF this seems to get > more complex. > > In particular, if we change the fix_stack_underrun() function > to add undefined values *before* the exiting ones, as in: > > // Stack: FIELD > <stack underrun detected, fixing> > // Stack: UNDEFINED, FIELD > > Any preexisting reference to top(0) would silently now point > to memory address that might as well been deallocated > (think about reallocating an array). The result might be a *disaster*. > Actually, now that I think about this, if the array is really deallocated > the same problem applies now, when pushing elements *after* it. > > *final question* > > So, the final question at this point is: do you think it would be a big > problem to remove the top(x) method from as_environment and force use > of pop() instead ? > > pop() would return by value, while top(x) returns by reference opening > a can of worms, but debatebly being faster. > > Comments ? > > > _______________________________________________ > Gnash-dev mailing list > [email protected] > http://lists.gnu.org/mailman/listinfo/gnash-dev -- /"\ ASCII Ribbon Campaign \ / Respect for low technology. X Keep e-mail messages readable by any computer system. / \ Keep it ASCII. _______________________________________________ Gnash-dev mailing list [email protected] http://lists.gnu.org/mailman/listinfo/gnash-dev
