[jsfunfuzz] Assertion + incorrect behaviour with dynamically created local variable...
authoroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 1 Jan 2009 08:22:40 +0000 (08:22 +0000)
committeroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 1 Jan 2009 08:22:40 +0000 (08:22 +0000)
<https://bugs.webkit.org/show_bug.cgi?id=23063>

Reviewed by Cameron Zwarich

Eval inside a catch block attempts to use the catch block's static scope in
an unsafe way by attempting to add new properties to the scope.  This patch
fixes this issue simply by preventing the catch block from using a static
scope if it contains an eval.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@39534 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JavaScriptCore/ChangeLog
JavaScriptCore/parser/Grammar.y
JavaScriptCore/parser/Nodes.cpp
JavaScriptCore/parser/Nodes.h
LayoutTests/ChangeLog
LayoutTests/fast/js/eval-var-decl-expected.txt
LayoutTests/fast/js/resources/eval-var-decl.js

index 2af6d7d..b653304 100644 (file)
@@ -1,5 +1,23 @@
 2008-12-31  Oliver Hunt  <oliver@apple.com>
 
+        Reviewed by Cameron Zwarich.
+
+        [jsfunfuzz] Assertion + incorrect behaviour with dynamically created local variable in a catch block
+        <https://bugs.webkit.org/show_bug.cgi?id=23063>
+
+        Eval inside a catch block attempts to use the catch block's static scope in
+        an unsafe way by attempting to add new properties to the scope.  This patch
+        fixes this issue simply by preventing the catch block from using a static
+        scope if it contains an eval.
+
+        * parser/Grammar.y:
+        * parser/Nodes.cpp:
+        (JSC::TryNode::emitBytecode):
+        * parser/Nodes.h:
+        (JSC::TryNode::):
+
+2008-12-31  Oliver Hunt  <oliver@apple.com>
+
         Reviewed by Gavin Barraclough.
 
         [jsfunfuzz] Computed exception offset wrong when first instruction is attempt to resolve deleted eval
index ed8075c..3f6e3e5 100644 (file)
@@ -1139,20 +1139,20 @@ ThrowStatement:
 ;
 
 TryStatement:
-    TRY Block FINALLY Block             { $$ = createNodeDeclarationInfo<StatementNode*>(new TryNode(GLOBAL_DATA, $2.m_node, GLOBAL_DATA->propertyNames->nullIdentifier, 0, $4.m_node),
+    TRY Block FINALLY Block             { $$ = createNodeDeclarationInfo<StatementNode*>(new TryNode(GLOBAL_DATA, $2.m_node, GLOBAL_DATA->propertyNames->nullIdentifier, false, 0, $4.m_node),
                                                                                          mergeDeclarationLists($2.m_varDeclarations, $4.m_varDeclarations),
                                                                                          mergeDeclarationLists($2.m_funcDeclarations, $4.m_funcDeclarations),
                                                                                          $2.m_features | $4.m_features,
                                                                                          $2.m_numConstants + $4.m_numConstants);
                                           DBG($$.m_node, @1, @2); }
-  | TRY Block CATCH '(' IDENT ')' Block { $$ = createNodeDeclarationInfo<StatementNode*>(new TryNode(GLOBAL_DATA, $2.m_node, *$5, $7.m_node, 0),
+  | TRY Block CATCH '(' IDENT ')' Block { $$ = createNodeDeclarationInfo<StatementNode*>(new TryNode(GLOBAL_DATA, $2.m_node, *$5, ($7.m_features & EvalFeature) != 0, $7.m_node, 0),
                                                                                          mergeDeclarationLists($2.m_varDeclarations, $7.m_varDeclarations),
                                                                                          mergeDeclarationLists($2.m_funcDeclarations, $7.m_funcDeclarations),
                                                                                          $2.m_features | $7.m_features | CatchFeature,
                                                                                          $2.m_numConstants + $7.m_numConstants);
                                           DBG($$.m_node, @1, @2); }
   | TRY Block CATCH '(' IDENT ')' Block FINALLY Block
-                                        { $$ = createNodeDeclarationInfo<StatementNode*>(new TryNode(GLOBAL_DATA, $2.m_node, *$5, $7.m_node, $9.m_node),
+                                        { $$ = createNodeDeclarationInfo<StatementNode*>(new TryNode(GLOBAL_DATA, $2.m_node, *$5, ($7.m_features & EvalFeature) != 0, $7.m_node, $9.m_node),
                                                                                          mergeDeclarationLists(mergeDeclarationLists($2.m_varDeclarations, $7.m_varDeclarations), $9.m_varDeclarations),
                                                                                          mergeDeclarationLists(mergeDeclarationLists($2.m_funcDeclarations, $7.m_funcDeclarations), $9.m_funcDeclarations),
                                                                                          $2.m_features | $7.m_features | $9.m_features | CatchFeature,
index 821a6d1..e4b4ea7 100644 (file)
@@ -2331,7 +2331,13 @@ RegisterID* TryNode::emitBytecode(BytecodeGenerator& generator, RegisterID* dst)
         RefPtr<Label> handlerEndLabel = generator.newLabel();
         generator.emitJump(handlerEndLabel.get());
         RefPtr<RegisterID> exceptionRegister = generator.emitCatch(generator.newTemporary(), tryStartLabel.get(), tryEndLabel.get());
-        generator.emitPushNewScope(exceptionRegister.get(), m_exceptionIdent, exceptionRegister.get());
+        if (m_catchHasEval) {
+            RefPtr<RegisterID> dynamicScopeObject = generator.emitNewObject(generator.newTemporary());
+            generator.emitPutById(dynamicScopeObject.get(), m_exceptionIdent, exceptionRegister.get());
+            generator.emitMove(exceptionRegister.get(), dynamicScopeObject.get());
+            generator.emitPushScope(exceptionRegister.get());
+        } else
+            generator.emitPushNewScope(exceptionRegister.get(), m_exceptionIdent, exceptionRegister.get());
         generator.emitNode(dst, m_catchBlock.get());
         generator.emitPopScope();
         generator.emitLabel(handlerEndLabel.get());
index 491f34b..988258e 100644 (file)
@@ -2015,12 +2015,13 @@ namespace JSC {
 
     class TryNode : public StatementNode {
     public:
-        TryNode(JSGlobalData* globalData, StatementNode* tryBlock, const Identifier& exceptionIdent, StatementNode* catchBlock, StatementNode* finallyBlock) JSC_FAST_CALL
+        TryNode(JSGlobalData* globalData, StatementNode* tryBlock, const Identifier& exceptionIdent, bool catchHasEval, StatementNode* catchBlock, StatementNode* finallyBlock) JSC_FAST_CALL
             : StatementNode(globalData)
             , m_tryBlock(tryBlock)
             , m_exceptionIdent(exceptionIdent)
             , m_catchBlock(catchBlock)
             , m_finallyBlock(finallyBlock)
+            , m_catchHasEval(catchHasEval)
         {
         }
 
@@ -2034,6 +2035,7 @@ namespace JSC {
         Identifier m_exceptionIdent;
         RefPtr<StatementNode> m_catchBlock;
         RefPtr<StatementNode> m_finallyBlock;
+        bool m_catchHasEval;
     };
 
     class ParameterNode : public ParserRefCounted {
index 7072b15..fd3f690 100644 (file)
@@ -1,5 +1,18 @@
 2008-12-31  Oliver Hunt  <oliver@apple.com>
 
+        Reviewed by Cameron Zwarich.
+
+        [jsfunfuzz] Assertion + incorrect behaviour with dynamically created local variable in a catch block
+        <https://bugs.webkit.org/show_bug.cgi?id=23063>
+
+        Add tests for variable declaration inside eval inside a catch block.
+
+        * fast/js/eval-var-decl-expected.txt:
+        * fast/js/resources/eval-var-decl.js:
+        (try.thirdEvalResult):
+
+2008-12-31  Oliver Hunt  <oliver@apple.com>
+
         Reviewed by Gavin Barraclough.
 
         [jsfunfuzz] Computed exception offset wrong when first instruction is attempt to resolve deleted eval
index 2058399..5b8eb49 100644 (file)
@@ -7,6 +7,7 @@ PASS hasOwnProperty("foo") is true
 PASS hasOwnProperty("bar") is true
 PASS firstEvalResult is true
 PASS secondEvalResult is false
+PASS thirdEvalResult is true
 PASS successfullyParsed is true
 
 TEST COMPLETE
index 58f2728..11ccae4 100644 (file)
@@ -15,4 +15,12 @@ shouldBeTrue("firstEvalResult");
 var secondEvalResult = eval('delete x; var result = hasOwnProperty("x"); var x = 3; result');
 shouldBeFalse("secondEvalResult");
 
+var thirdEvalResult = false;
+try {
+    thirdEvalResult = (function(){ var x=false; try { throw ""; } catch (e) { eval("var x = true;"); } return x; })();
+} catch (e) {
+    thirdEvalResult = "Threw exception!";
+}
+shouldBeTrue("thirdEvalResult");
+
 var successfullyParsed = true;