[Gnash-commit] /srv/bzr/gnash/trunk r12253: Some VM operations relied on the order of evaluation of function arguments,

[Benjamin Wolsey]

------------------------------------------------------------
revno: 12253 [merge]
committer: Benjamin Wolsey <address@hidden>
branch nick: trunk
timestamp: Tue 2010-06-15 09:23:24 +0200
message:
  Some VM operations relied on the order of evaluation of function arguments,
  which is not specified by the C++ standard and varies between compilers. This
  commit fixes those problems.
modified:
  libcore/vm/ASHandlers.cpp

=== modified file 'libcore/vm/ASHandlers.cpp'
--- a/libcore/vm/ASHandlers.cpp 2010-05-20 10:19:03 +0000
+++ b/libcore/vm/ASHandlers.cpp 2010-06-15 06:40:04 +0000
@@ -890,10 +890,10 @@
     assert(thread.atActionTag(SWF::ACTION_EQUAL)); // 0x0E
 #endif
 
-    as_value& op1 = env.top(0);
-    as_value& op2 = env.top(1);
+    const double op1 = env.top(0).to_number();
+    const double op2 = env.top(1).to_number();
 
-    env.top(1).set_bool(op1.to_number() == op2.to_number());
+    env.top(1).set_bool(op2 == op1);
 
     // Flash4 used 1 and 0 as return from this tag
     if ( env.get_version() < 5 ) convertToNumber(env.top(1), getVM(env));
@@ -904,10 +904,13 @@
 void
 ActionLessThan(ActionExec& thread)
 {
-
     as_environment& env = thread.env;
     
-    env.top(1).set_bool(env.top(1).to_number() < env.top(0).to_number());
+    // NB: this unusual order is correct!
+    const double d2 = env.top(1).to_number();
+    const double d1 = env.top(0).to_number();
+
+    env.top(1).set_bool(d2 < d1);
 
     // Flash4 used 1 and 0 as return from this tag
     if ( env.get_version() < 5 ) convertToNumber(env.top(1), getVM(env));
@@ -918,9 +921,9 @@
 void
 ActionLogicalAnd(ActionExec& thread)
 {
-
     as_environment& env = thread.env;
     
+    // Note: the order of evaluation of the && operands is specified.
     env.top(1).set_bool(env.top(1).to_bool() && env.top(0).to_bool());
     env.drop(1);
 }
@@ -928,9 +931,9 @@
 void
 ActionLogicalOr(ActionExec& thread)
 {
-
     as_environment& env = thread.env;
     
+    // Note: the order of evaluation of the || operands is specified.
     env.top(1).set_bool(env.top(1).to_bool() || env.top(0).to_bool());
     env.drop(1);
 }
@@ -1177,9 +1180,11 @@
 {
     as_environment& env = thread.env;
     const int version = getSWFVersion(env);
-    env.top(1).set_string(env.top(1).to_string(version) +
-            env.top(0).to_string(version));
-    
+
+    const std::string& op1 = env.top(0).to_string(version);
+    const std::string& op2 = env.top(1).to_string(version);
+
+    env.top(1).set_string(op2 + op1);
     env.drop(1);
 }
 
@@ -1444,7 +1449,9 @@
     as_environment& env = thread.env;
     
     const int ver = env.get_version();
-    env.top(1).set_bool(env.top(1).to_string(ver) < env.top(0).to_string(ver));
+    const std::string& op1 = env.top(0).to_string(ver);
+    const std::string& op2 = env.top(1).to_string(ver);
+    env.top(1).set_bool(op2 < op1);
     env.drop(1);
 }
 
@@ -3364,20 +3371,18 @@
 {
     // Just swap the operator and invoke ActionNewLessThan
     as_environment& env = thread.env;
-    as_value tmp = env.top(1);
-    env.top(1) = env.top(0);
-    env.top(0) = tmp;
+    std::swap(env.top(1), env.top(0));
     ActionNewLessThan(thread);
 }
 
 void
 ActionStringGreater(ActionExec& thread)
 {
-    
     as_environment& env = thread.env;
     
-    // No need to use to_string() here, this is a swf7 opcode
-    env.top(1).set_bool(env.top(1).to_string() > env.top(0).to_string());
+    const std::string& op1 = env.top(0).to_string();
+    const std::string& op2 = env.top(1).to_string();
+    env.top(1).set_bool(op2 > op1);
     env.drop(1);
 }