Changeset 39524 in webkit

Timestamp:

Dec 30, 2008 10:49:34 PM (15 years ago)

Author:

oliver@apple.com

Message:

<​https://bugs.webkit.org/show_bug.cgi?id=23049> [jsfunfuzz] With blocks do not correctly protect their scope object
<rdar://problem/6469742> Crash in JSC::TypeInfo::hasStandardGetOwnPropertySlot() running jsfunfuzz

Reviewed by Darin Adler

The problem that caused this was that with nodes were not correctly protecting
the final object that was placed in the scope chain. We correct this by forcing
the use of a temporary register (which stops us relying on a local register
protecting the scope) and changing the behaviour of op_push_scope so that it
will store the final scope object.

Location:

trunk

Files:

• JavaScriptCore/ChangeLog (modified) (1 diff)
• JavaScriptCore/bytecompiler/BytecodeGenerator.cpp (modified) (1 diff)
• JavaScriptCore/interpreter/Interpreter.cpp (modified) (3 diffs)
• JavaScriptCore/interpreter/Interpreter.h (modified) (1 diff)
• JavaScriptCore/jit/JIT.cpp (modified) (1 diff)
• JavaScriptCore/parser/Nodes.cpp (modified) (1 diff)
• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/js/resources/with-scope-gc.js (added)
• LayoutTests/fast/js/with-scope-gc-expected.txt (added)
• LayoutTests/fast/js/with-scope-gc.html (added)

trunk/JavaScriptCore/ChangeLog

@@ +1 @@
+2008-12-30  Oliver Hunt  <oliver@apple.com>
+
+        Reviewed by Darin Adler.
+
+        <https://bugs.webkit.org/show_bug.cgi?id=23049> [jsfunfuzz] With blocks do not correctly protect their scope object
+        <rdar://problem/6469742> Crash in JSC::TypeInfo::hasStandardGetOwnPropertySlot() running jsfunfuzz
+
+        The problem that caused this was that with nodes were not correctly protecting
+        the final object that was placed in the scope chain.  We correct this by forcing
+        the use of a temporary register (which stops us relying on a local register
+        protecting the scope) and changing the behaviour of op_push_scope so that it
+        will store the final scope object.
+
+        * bytecompiler/BytecodeGenerator.cpp:
+        (JSC::BytecodeGenerator::emitPushScope):
+        * interpreter/Interpreter.cpp:
+        (JSC::Interpreter::privateExecute):
+        (JSC::Interpreter::cti_op_push_scope):
+        * interpreter/Interpreter.h:
+        * jit/JIT.cpp:
+        (JSC::JIT::privateCompileMainPass):
+        * parser/Nodes.cpp:
+        (JSC::WithNode::emitBytecode):
+
 2008-12-30  Cameron Zwarich  <cwzwarich@uwaterloo.ca>
 

trunk/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

@@ -1396 +1396 @@
 RegisterID* BytecodeGenerator::emitPushScope(RegisterID* scope)
 {
+    ASSERT(scope->isTemporary());
     ControlFlowContext context;
     context.isFinallyBlock = false;

trunk/JavaScriptCore/interpreter/Interpreter.cpp

@@ -3640 +3640 @@
 
            Converts register scope to object, and pushes it onto the top
-           of the current scope chain.
+           of the current scope chain.  The contents of the register scope
+           are replaced by the result of toObject conversion of the scope.
         */
         int scope = (++vPC)->u.operand;
@@ -3647 +3648 @@
         CHECK_FOR_EXCEPTION();
 
+        callFrame[scope] = o;
         callFrame->setScopeChain(callFrame->scopeChain()->push(o));
 
@@ -5739 +5741 @@
 }
 
-void Interpreter::cti_op_push_scope(STUB_ARGS)
+JSObject* Interpreter::cti_op_push_scope(STUB_ARGS)
 {
     BEGIN_STUB_FUNCTION();
 
     JSObject* o = ARG_src1->toObject(ARG_callFrame);
-    CHECK_FOR_EXCEPTION_VOID();
+    CHECK_FOR_EXCEPTION();
     ARG_callFrame->setScopeChain(ARG_callFrame->scopeChain()->push(o));
+    return o;
 }
 

trunk/JavaScriptCore/interpreter/Interpreter.h

@@ -256 +256 @@
         static JSPropertyNameIterator* JIT_STUB cti_op_get_pnames(STUB_ARGS);
         static JSValue* JIT_STUB cti_op_next_pname(STUB_ARGS);
-        static void JIT_STUB cti_op_push_scope(STUB_ARGS);
+        static JSObject* JIT_STUB cti_op_push_scope(STUB_ARGS);
         static void JIT_STUB cti_op_pop_scope(STUB_ARGS);
         static JSValue* JIT_STUB cti_op_typeof(STUB_ARGS);

trunk/JavaScriptCore/jit/JIT.cpp

@@ -1039 +1039 @@
             emitPutJITStubArgFromVirtualRegister(currentInstruction[1].u.operand, 1, X86::ecx);
             emitCTICall(Interpreter::cti_op_push_scope);
+            emitPutVirtualRegister(currentInstruction[1].u.operand);
             NEXT_OPCODE(op_push_scope);
         }

trunk/JavaScriptCore/parser/Nodes.cpp

@@ -2039 +2039 @@
 RegisterID* WithNode::emitBytecode(BytecodeGenerator& generator, RegisterID* dst)
 {
-    RefPtr<RegisterID> scope = generator.emitNode(m_expr.get()); // scope must be protected until popped
+    RefPtr<RegisterID> scope = generator.newTemporary();
+    generator.emitNode(scope.get(), m_expr.get()); // scope must be protected until popped
     generator.emitExpressionInfo(m_divot, m_expressionLength, 0);
     generator.emitPushScope(scope.get());

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2008-12-30  Oliver Hunt  <oliver@apple.com>
+
+        Reviewed by Darin Adler.
+
+        <https://bugs.webkit.org/show_bug.cgi?id=23049> [jsfunfuzz] With blocks do not correctly protect their scope object
+        <rdar://problem/6469742> Crash in JSC::TypeInfo::hasStandardGetOwnPropertySlot() running jsfunfuzz
+
+        Tests to ensure we correctly protect the scope object from GC.
+
+        * fast/js/resources/with-scope-gc.js: Added.
+        * fast/js/with-scope-gc-expected.txt: Added.
+        * fast/js/with-scope-gc.html: Added.
+
 2008-12-30  Simon Fraser  <simon.fraser@apple.com>