[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Gnash-commit] gnash ChangeLog server/Property.cpp server/Prop... [relea
From: |
Sandro Santilli |
Subject: |
[Gnash-commit] gnash ChangeLog server/Property.cpp server/Prop... [release_0_8_2_rc1] |
Date: |
Wed, 27 Feb 2008 17:39:31 +0000 |
CVSROOT: /sources/gnash
Module name: gnash
Branch: release_0_8_2_rc1
Changes by: Sandro Santilli <strk> 08/02/27 17:39:31
Modified files:
. : ChangeLog
server : Property.cpp Property.h
testsuite/actionscript.all: Object.as
Log message:
don't go into an infinite loop if a getter or setter gets or sets
the member it targets. Fixes bug #22420.
CVSWeb URLs:
http://cvs.savannah.gnu.org/viewcvs/gnash/ChangeLog?cvsroot=gnash&only_with_tag=release_0_8_2_rc1&r1=1.5711.2.27&r2=1.5711.2.28
http://cvs.savannah.gnu.org/viewcvs/gnash/server/Property.cpp?cvsroot=gnash&only_with_tag=release_0_8_2_rc1&r1=1.6&r2=1.6.2.1
http://cvs.savannah.gnu.org/viewcvs/gnash/server/Property.h?cvsroot=gnash&only_with_tag=release_0_8_2_rc1&r1=1.18&r2=1.18.2.1
http://cvs.savannah.gnu.org/viewcvs/gnash/testsuite/actionscript.all/Object.as?cvsroot=gnash&only_with_tag=release_0_8_2_rc1&r1=1.38&r2=1.38.2.1
Patches:
Index: ChangeLog
===================================================================
RCS file: /sources/gnash/gnash/ChangeLog,v
retrieving revision 1.5711.2.27
retrieving revision 1.5711.2.28
diff -u -b -r1.5711.2.27 -r1.5711.2.28
--- ChangeLog 27 Feb 2008 10:15:41 -0000 1.5711.2.27
+++ ChangeLog 27 Feb 2008 17:39:30 -0000 1.5711.2.28
@@ -1,4 +1,14 @@
-2008-02-26 Benjamin Wolsey <address@hidden>
+2008-02-27 Sandro Santilli <address@hidden>
+
+ * server/Property.{cpp,h}: don't go into an infinite loop
+ if a getter or setter gets or sets the member it targets.
+ Fixes bug #22420.
+ * testsuite/actionscript.all/Object.as: add test for recursive
+ getter/setter. Gnash fails at NOT protecting from infinite
+ recursion non-trival call graphs in SWF7+ (but is compatible
+ with SWF6 in that reguard).
+
+2008-02-27 Benjamin Wolsey <address@hidden>
* libbase/config.cpp: drop unused file.
* libbase/Makefile.am: drop config.cpp.
Index: server/Property.cpp
===================================================================
RCS file: /sources/gnash/gnash/server/Property.cpp,v
retrieving revision 1.6
retrieving revision 1.6.2.1
diff -u -b -r1.6 -r1.6.2.1
--- server/Property.cpp 21 Jan 2008 20:55:47 -0000 1.6
+++ server/Property.cpp 27 Feb 2008 17:39:31 -0000 1.6.2.1
@@ -26,6 +26,19 @@
Property::getDelayedValue(const as_object& this_ptr) const
{
const as_accessors* a = boost::get<const as_accessors>(&mBound);
+
+ // Don't recursively invoke a getter
+ //
+ // NOTE: it seems recursion protection is handled differently
+ // when the SWF is targetted at player 7 or higher.
+ // See actionscript.all/Object.as
+ //
+ as_accessors::ScopedLock lock(*a);
+ if ( ! lock.obtained() )
+ {
+ return a->underlyingValue;
+ }
+
as_environment env;
fn_call fn(const_cast<as_object*>(&this_ptr), &env, 0, 0);
if (mDestructive)
@@ -56,11 +69,29 @@
void
Property::setDelayedValue(as_object& this_ptr, const as_value& value)
{
- const as_accessors* a = boost::get<const as_accessors>(&mBound);
+ log_debug("setDelayedValue: %s", value.to_debug_string().c_str());
+ as_accessors* a = boost::get<as_accessors>(&mBound);
+
+ // Don't recursively invoke a setter
+ //
+ // NOTE: it seems recursion protection is handled differently
+ // when the SWF is targetted at player 7 or higher.
+ // In particular, it is not avoided unless the setter
+ // calls self.. See actionscript.all/Object.as
+ //
+ as_accessors::ScopedLock lock(*a);
+ if ( ! lock.obtained() )
+ {
+ a->underlyingValue = value;
+ return;
+ }
+
+
as_environment env;
env.push(value);
fn_call fn(&this_ptr, &env, 1, 0);
(*a->mSetter)(fn);
+
return;
// TODO: Push value
@@ -105,8 +136,7 @@
case 2: // Getter/setter
{
const as_accessors& a = boost::get<as_accessors>(mBound);
- if (a.mGetter) a.mGetter->setReachable();
- if (a.mSetter) a.mSetter->setReachable();
+ a.markReachableResources();
break;
}
default:
@@ -115,4 +145,12 @@
}
}
+void
+as_accessors::markReachableResources() const
+{
+ if (mGetter) mGetter->setReachable();
+ if (mSetter) mSetter->setReachable();
+ underlyingValue.setReachable();
+}
+
} // namespace gnash
Index: server/Property.h
===================================================================
RCS file: /sources/gnash/gnash/server/Property.h,v
retrieving revision 1.18
retrieving revision 1.18.2.1
diff -u -b -r1.18 -r1.18.2.1
--- server/Property.h 21 Jan 2008 20:55:47 -0000 1.18
+++ server/Property.h 27 Feb 2008 17:39:31 -0000 1.18.2.1
@@ -15,7 +15,7 @@
// along with this program; if not, write to the Free Software
// Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
-/* $Id: Property.h,v 1.18 2008/01/21 20:55:47 rsavoye Exp $ */
+/* $Id: Property.h,v 1.18.2.1 2008/02/27 17:39:31 strk Exp $ */
#ifndef GNASH_PROPERTY_H
#define GNASH_PROPERTY_H
@@ -24,11 +24,13 @@
#include "gnashconfig.h"
#endif
-#include <boost/variant.hpp>
-
#include "as_prop_flags.h"
#include "as_value.h"
#include "string_table.h"
+#include "log.h"
+
+#include <boost/variant.hpp>
+#include <cassert>
namespace gnash {
@@ -39,12 +41,56 @@
class as_accessors
{
public:
+
+ /// For SWF6 (not higher) a setter would not be invoked
+ /// while being set. This ScopedLock helps marking a
+ /// Getter-Setter as being invoked in an exception-safe
+ /// manner.
+ class ScopedLock {
+ const as_accessors& a;
+ bool obtainedLock;
+
+ ScopedLock(ScopedLock&);
+ ScopedLock& operator==(ScopedLock&);
+ public:
+
+ ScopedLock(const as_accessors& na) : a(na)
+ {
+ if ( a.beingAccessed ) obtainedLock=false;
+ else {
+ a.beingAccessed = true;
+ obtainedLock = true;
+ }
+ }
+
+ ~ScopedLock() { if ( obtainedLock) a.beingAccessed = false; }
+
+ /// Return true if the lock was obtained
+ //
+ /// If false is returned, we're being called recursively,
+ /// which means we should set the underlyingValue instead
+ /// of calling the setter (for SWF6, again).
+ ///
+ bool obtained() const { return obtainedLock; }
+ };
+
+ friend class ScopedLock;
+
as_function* mGetter;
as_function* mSetter;
+ as_value underlyingValue;
+
as_accessors(as_function* getter, as_function* setter) :
mGetter(getter),
- mSetter(setter)
+ mSetter(setter), underlyingValue(), beingAccessed(false)
{/**/}
+
+ void markReachableResources() const;
+
+private:
+
+ mutable bool beingAccessed;
+
};
/// An abstract property
@@ -186,6 +232,7 @@
// Destructive are always overwritten.
if (mDestructive)
{
+ gnash::log_debug("destructive getter/setter, value %s",
value.to_debug_string().c_str());
mDestructive = false;
mBound = value;
}
Index: testsuite/actionscript.all/Object.as
===================================================================
RCS file: /sources/gnash/gnash/testsuite/actionscript.all/Object.as,v
retrieving revision 1.38
retrieving revision 1.38.2.1
diff -u -b -r1.38 -r1.38.2.1
--- testsuite/actionscript.all/Object.as 24 Oct 2007 21:55:33 -0000
1.38
+++ testsuite/actionscript.all/Object.as 27 Feb 2008 17:39:31 -0000
1.38.2.1
@@ -20,7 +20,7 @@
// compile this test case with Ming makeswf, and then
// execute it like this gnash -1 -r 0 -v out.swf
-rcsid="$Id: Object.as,v 1.38 2007/10/24 21:55:33 strk Exp $";
+rcsid="$Id: Object.as,v 1.38.2.1 2008/02/27 17:39:31 strk Exp $";
#include "check.as"
@@ -307,6 +307,79 @@
check_equals(c, 7);
check(o.addProperty != Object.prototype.addProperty );
+// recursive setter
+mem_setter = function(x)
+{
+ this.mem2 = x;
+ this.mem = x;
+};
+mem_getter = function()
+{
+ return this.mem;
+};
+o = {};
+o.addProperty('mem', mem_getter, mem_setter);
+check_equals(typeof(o.mem), 'undefined');
+o.mem = 3; // watch out for recursion !
+check_equals(o.mem, 3);
+check_equals(o.mem2, 3);
+
+// Test double-recursion of setter:
+// setter1 triggers setter2
+
+mem1_getter = function() { return this.mem1; };
+mem1_setter = function(x)
+{
+ if ( this.setterCalls > 2 )
+ {
+ return;
+ }
+ ++this.setterCalls;
+ o2.mem2 = x;
+ this.mem1 = x;
+};
+
+mem2_getter = function() { return this.mem2; };
+mem2_setter = function(x)
+{
+ if ( this.setterCalls > 2 )
+ {
+ return;
+ }
+ ++this.setterCalls;
+ o1.mem1 = x;
+ this.mem2 = x;
+};
+
+o1 = {}; o1.addProperty('mem1', mem1_getter, mem1_setter);
+o2 = {}; o2.addProperty('mem2', mem2_getter, mem2_setter);
+
+o1.setterCalls = o2.setterCalls = 0; // reset counters
+o1.mem1 = 3;
+#if OUTPUT_VERSION == 6
+ check_equals(o1.setterCalls, 1);
+ check_equals(o2.setterCalls, 1);
+#else
+ // SWF7+ doesn't protect recursion..
+ xcheck_equals(o1.setterCalls, 3);
+ xcheck_equals(o2.setterCalls, 3);
+#endif
+check_equals(o1.mem1, 3);
+check_equals(o1.mem1, 3);
+
+o1.setterCalls = o2.setterCalls = 0; // reset counters
+o2.mem2 = 6;
+#if OUTPUT_VERSION == 6
+ check_equals(o1.setterCalls, 1);
+ check_equals(o2.setterCalls, 1);
+#else
+ // SWF7+ doesn't protect recursion..
+ xcheck_equals(o1.setterCalls, 3);
+ xcheck_equals(o2.setterCalls, 3);
+#endif
+check_equals(o1.mem1, 6);
+check_equals(o2.mem2, 6);
+
// Object.addProperty wasn't in SWF5
#endif // OUTPUT_VERSION > 5
@@ -455,8 +528,11 @@
#endif // OUTPUT_VERSION > 5
-#if OUTPUT_VERSION > 5
-totals(156);
-#else
+#if OUTPUT_VERSION <= 5
totals(63);
#endif
+
+#if OUTPUT_VERSION >= 6
+totals(167);
+#endif
+
- [Gnash-commit] gnash ChangeLog server/Property.cpp server/Prop... [release_0_8_2_rc1],
Sandro Santilli <=