From d019ec96d99e5a42fe71235eee0e6de837028bee Mon Sep 17 00:00:00 2001 From: "oliver@apple.com" Date: Wed, 31 Dec 2008 06:49:34 +0000 Subject: [PATCH] [jsfunfuzz] With blocks do not correctly protect their scope object 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. git-svn-id: https://svn.webkit.org/repository/webkit/trunk@39524 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- JavaScriptCore/ChangeLog | 24 +++++++++++ JavaScriptCore/bytecompiler/BytecodeGenerator.cpp | 1 + JavaScriptCore/interpreter/Interpreter.cpp | 9 +++-- JavaScriptCore/interpreter/Interpreter.h | 2 +- JavaScriptCore/jit/JIT.cpp | 1 + JavaScriptCore/parser/Nodes.cpp | 3 +- LayoutTests/ChangeLog | 13 ++++++ LayoutTests/fast/js/resources/with-scope-gc.js | 49 +++++++++++++++++++++++ LayoutTests/fast/js/with-scope-gc-expected.txt | 9 +++++ LayoutTests/fast/js/with-scope-gc.html | 13 ++++++ 10 files changed, 119 insertions(+), 5 deletions(-) create mode 100644 LayoutTests/fast/js/resources/with-scope-gc.js create mode 100644 LayoutTests/fast/js/with-scope-gc-expected.txt create mode 100644 LayoutTests/fast/js/with-scope-gc.html diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog index 52d93e7..55ee52b 100644 --- a/JavaScriptCore/ChangeLog +++ b/JavaScriptCore/ChangeLog @@ -1,3 +1,27 @@ +2008-12-30 Oliver Hunt + + Reviewed by Darin Adler. + + [jsfunfuzz] With blocks do not correctly protect their scope object + 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 Reviewed by Sam Weinig. diff --git a/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp b/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp index b3dcdd2..7409cfe 100644 --- a/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp +++ b/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp @@ -1395,6 +1395,7 @@ RegisterID* BytecodeGenerator::emitConstruct(RegisterID* dst, RegisterID* func, RegisterID* BytecodeGenerator::emitPushScope(RegisterID* scope) { + ASSERT(scope->isTemporary()); ControlFlowContext context; context.isFinallyBlock = false; m_scopeContextStack.append(context); diff --git a/JavaScriptCore/interpreter/Interpreter.cpp b/JavaScriptCore/interpreter/Interpreter.cpp index 5caffc3..e93cd3f 100644 --- a/JavaScriptCore/interpreter/Interpreter.cpp +++ b/JavaScriptCore/interpreter/Interpreter.cpp @@ -3639,13 +3639,15 @@ JSValue* Interpreter::privateExecute(ExecutionFlag flag, RegisterFile* registerF /* push_scope scope(r) 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; JSValue* v = callFrame[scope].jsValue(callFrame); JSObject* o = v->toObject(callFrame); CHECK_FOR_EXCEPTION(); + callFrame[scope] = o; callFrame->setScopeChain(callFrame->scopeChain()->push(o)); ++vPC; @@ -5738,13 +5740,14 @@ JSValue* Interpreter::cti_op_next_pname(STUB_ARGS) return temp; } -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; } void Interpreter::cti_op_pop_scope(STUB_ARGS) diff --git a/JavaScriptCore/interpreter/Interpreter.h b/JavaScriptCore/interpreter/Interpreter.h index 8b71bd0..f90b23b 100644 --- a/JavaScriptCore/interpreter/Interpreter.h +++ b/JavaScriptCore/interpreter/Interpreter.h @@ -255,7 +255,7 @@ namespace JSC { static JSValue* JIT_STUB cti_op_throw(STUB_ARGS); 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); static JSValue* JIT_STUB cti_op_is_undefined(STUB_ARGS); diff --git a/JavaScriptCore/jit/JIT.cpp b/JavaScriptCore/jit/JIT.cpp index 30ea338..3573f6ab 100644 --- a/JavaScriptCore/jit/JIT.cpp +++ b/JavaScriptCore/jit/JIT.cpp @@ -1038,6 +1038,7 @@ void JIT::privateCompileMainPass() case op_push_scope: { emitPutJITStubArgFromVirtualRegister(currentInstruction[1].u.operand, 1, X86::ecx); emitCTICall(Interpreter::cti_op_push_scope); + emitPutVirtualRegister(currentInstruction[1].u.operand); NEXT_OPCODE(op_push_scope); } case op_pop_scope: { diff --git a/JavaScriptCore/parser/Nodes.cpp b/JavaScriptCore/parser/Nodes.cpp index bb88482..4d4406d 100644 --- a/JavaScriptCore/parser/Nodes.cpp +++ b/JavaScriptCore/parser/Nodes.cpp @@ -2038,7 +2038,8 @@ void WithNode::releaseNodes(NodeReleaser& releaser) RegisterID* WithNode::emitBytecode(BytecodeGenerator& generator, RegisterID* dst) { - RefPtr scope = generator.emitNode(m_expr.get()); // scope must be protected until popped + RefPtr 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()); RegisterID* result = generator.emitNode(dst, m_statement.get()); diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index 7325bbb..0b598a3 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,16 @@ +2008-12-30 Oliver Hunt + + Reviewed by Darin Adler. + + [jsfunfuzz] With blocks do not correctly protect their scope object + 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 Fix spurious test failures by rounding floating point values to diff --git a/LayoutTests/fast/js/resources/with-scope-gc.js b/LayoutTests/fast/js/resources/with-scope-gc.js new file mode 100644 index 0000000..82ca1cf --- /dev/null +++ b/LayoutTests/fast/js/resources/with-scope-gc.js @@ -0,0 +1,49 @@ +description( +'Tests to make sure that dynamic scope objects are correctly protected from GC. To pass we need to not crash.' +); + +function gc() +{ + if (this.GCController) + GCController.collect(); + else + for (var i = 0; i < 10000; ++i) // Allocate a sufficient number of objects to force a GC. + ({}); +} + +(function() { + try { + // Immediate value for scope + with(1) { gc(); a; } + } catch(e) { + } +})(); + +(function() { + try { + // Real object for scope + var local; + with (local = {}) { + z=null; + {}; {}; [1,2,3,4*{}]; // Clobber any temporaries the scope may exist in + gc(); + b; + } + } catch (e) { + } +})(); + +(function() { + try { + // Test catch blocks for the heck of it + try { + throw 1; + } catch(e) { + gc(); + b; + } + } catch (e) { + } +})(); + +var successfullyParsed = true; diff --git a/LayoutTests/fast/js/with-scope-gc-expected.txt b/LayoutTests/fast/js/with-scope-gc-expected.txt new file mode 100644 index 0000000..38cc3cf --- /dev/null +++ b/LayoutTests/fast/js/with-scope-gc-expected.txt @@ -0,0 +1,9 @@ +Tests to make sure that dynamic scope objects are correctly protected from GC. To pass we need to not crash. + +On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". + + +PASS successfullyParsed is true + +TEST COMPLETE + diff --git a/LayoutTests/fast/js/with-scope-gc.html b/LayoutTests/fast/js/with-scope-gc.html new file mode 100644 index 0000000..248806c --- /dev/null +++ b/LayoutTests/fast/js/with-scope-gc.html @@ -0,0 +1,13 @@ + + + + + + + +

+
+ + + + -- 1.8.3.1