Revision: 15079
Author: [email protected]
Date: Wed Jun 12 04:02:51 2013
Log: Allocate generator result objects before unwinding try handlers
When a generator suspends, it saves its state out to the heap and
unwinds try handlers but doesn't pop anything off the stack. Instead it
relies on no GC happening between the suspend and the return from the
generator. However this was not the case: boxing the result object
could cause GC, which would try to traverse the stack but would
misinterpret words from unwound try handlers as heap objects.
This CL changes to allocate the result objects before the suspend. It
also removes the generators-iteration skip introduced in r15065.
[email protected]
TEST=mjsunit/harmony/generators-iteration
BUG=
Review URL: https://codereview.chromium.org/16801006
http://code.google.com/p/v8/source/detail?r=15079
Modified:
/branches/bleeding_edge/src/arm/full-codegen-arm.cc
/branches/bleeding_edge/src/full-codegen.cc
/branches/bleeding_edge/src/full-codegen.h
/branches/bleeding_edge/src/ia32/full-codegen-ia32.cc
/branches/bleeding_edge/src/x64/full-codegen-x64.cc
/branches/bleeding_edge/test/mjsunit/mjsunit.status
=======================================
--- /branches/bleeding_edge/src/arm/full-codegen-arm.cc Mon Jun 10 08:47:23
2013
+++ /branches/bleeding_edge/src/arm/full-codegen-arm.cc Wed Jun 12 04:02:51
2013
@@ -1991,8 +1991,12 @@
VisitForStackValue(expr->expression());
switch (expr->yield_kind()) {
- case Yield::INITIAL:
- case Yield::SUSPEND: {
+ case Yield::SUSPEND:
+ // Pop value from top-of-stack slot; box result into result register.
+ EmitCreateIteratorResult(false);
+ __ push(result_register());
+ // Fall through.
+ case Yield::INITIAL: {
VisitForStackValue(expr->generator_object());
__ CallRuntime(Runtime::kSuspendJSGeneratorObject, 1);
__ ldr(context_register(),
@@ -2001,12 +2005,8 @@
Label resume;
__ CompareRoot(result_register(), Heap::kTheHoleValueRootIndex);
__ b(ne, &resume);
- if (expr->yield_kind() == Yield::SUSPEND) {
- EmitReturnIteratorResult(false);
- } else {
- __ pop(result_register());
- EmitReturnSequence();
- }
+ __ pop(result_register());
+ EmitReturnSequence();
__ bind(&resume);
context()->Plug(result_register());
@@ -2018,7 +2018,10 @@
__ mov(r1,
Operand(Smi::FromInt(JSGeneratorObject::kGeneratorClosed)));
__ str(r1, FieldMemOperand(result_register(),
JSGeneratorObject::kContinuationOffset));
- EmitReturnIteratorResult(true);
+ // Pop value from top-of-stack slot, box result into result register.
+ EmitCreateIteratorResult(true);
+ EmitUnwindBeforeReturn();
+ EmitReturnSequence();
break;
}
@@ -2048,10 +2051,10 @@
// try { received = yield result.value }
__ bind(&l_try);
- __ pop(r0); // result.value
+ EmitCreateIteratorResult(false); // pop and box to
r0
__ PushTryHandler(StackHandler::CATCH, expr->index());
const int handler_size = StackHandlerConstants::kSize;
- __ push(r0); // result.value
+ __ push(r0); // result
__ ldr(r3, MemOperand(sp, (0 + 1) * kPointerSize + handler_size));
// g
__ push(r3); // g
__ CallRuntime(Runtime::kSuspendJSGeneratorObject, 1);
@@ -2059,7 +2062,8 @@
MemOperand(fp, StandardFrameConstants::kContextOffset));
__ CompareRoot(r0, Heap::kTheHoleValueRootIndex);
__ b(ne, &l_resume);
- EmitReturnIteratorResult(false);
+ __ pop(r0); // result
+ EmitReturnSequence();
__ bind(&l_resume); // received in r0
__ PopTryHandler();
@@ -2214,13 +2218,20 @@
}
-void FullCodeGenerator::EmitReturnIteratorResult(bool done) {
+void FullCodeGenerator::EmitCreateIteratorResult(bool done) {
Label gc_required;
Label allocated;
Handle<Map> map(isolate()->native_context()->generator_result_map());
__ Allocate(map->instance_size(), r0, r2, r3, &gc_required, TAG_OBJECT);
+ __ jmp(&allocated);
+
+ __ bind(&gc_required);
+ __ Push(Smi::FromInt(map->instance_size()));
+ __ CallRuntime(Runtime::kAllocateInNewSpace, 1);
+ __ ldr(context_register(),
+ MemOperand(fp, StandardFrameConstants::kContextOffset));
__ bind(&allocated);
__ mov(r1, Operand(map));
@@ -2240,26 +2251,6 @@
// root set.
__ RecordWriteField(r0, JSGeneratorObject::kResultValuePropertyOffset,
r2, r3, kLRHasBeenSaved, kDontSaveFPRegs);
-
- if (done) {
- // Exit all nested statements.
- NestedStatement* current = nesting_stack_;
- int stack_depth = 0;
- int context_length = 0;
- while (current != NULL) {
- current = current->Exit(&stack_depth, &context_length);
- }
- __ Drop(stack_depth);
- }
-
- EmitReturnSequence();
-
- __ bind(&gc_required);
- __ Push(Smi::FromInt(map->instance_size()));
- __ CallRuntime(Runtime::kAllocateInNewSpace, 1);
- __ ldr(context_register(),
- MemOperand(fp, StandardFrameConstants::kContextOffset));
- __ jmp(&allocated);
}
=======================================
--- /branches/bleeding_edge/src/full-codegen.cc Mon Jun 10 02:26:18 2013
+++ /branches/bleeding_edge/src/full-codegen.cc Wed Jun 12 04:02:51 2013
@@ -1230,13 +1230,7 @@
}
-void FullCodeGenerator::VisitReturnStatement(ReturnStatement* stmt) {
- Comment cmnt(masm_, "[ ReturnStatement");
- SetStatementPosition(stmt);
- Expression* expr = stmt->expression();
- VisitForAccumulatorValue(expr);
-
- // Exit all nested statements.
+void FullCodeGenerator::EmitUnwindBeforeReturn() {
NestedStatement* current = nesting_stack_;
int stack_depth = 0;
int context_length = 0;
@@ -1244,7 +1238,15 @@
current = current->Exit(&stack_depth, &context_length);
}
__ Drop(stack_depth);
+}
+
+void FullCodeGenerator::VisitReturnStatement(ReturnStatement* stmt) {
+ Comment cmnt(masm_, "[ ReturnStatement");
+ SetStatementPosition(stmt);
+ Expression* expr = stmt->expression();
+ VisitForAccumulatorValue(expr);
+ EmitUnwindBeforeReturn();
EmitReturnSequence();
}
=======================================
--- /branches/bleeding_edge/src/full-codegen.h Thu May 23 02:51:06 2013
+++ /branches/bleeding_edge/src/full-codegen.h Wed Jun 12 04:02:51 2013
@@ -410,10 +410,10 @@
// this has to be a separate pass _before_ populating or executing any
module.
void AllocateModules(ZoneList<Declaration*>* declarations);
- // Generator code to return a fresh iterator result object. The "value"
- // property is set to a value popped from the stack, and "done" is set
- // according to the argument.
- void EmitReturnIteratorResult(bool done);
+ // Generate code to create an iterator result object. The "value"
property is
+ // set to a value popped from the stack, and "done" is set according to
the
+ // argument. The result object is left in the result register.
+ void EmitCreateIteratorResult(bool done);
// Try to perform a comparison as a fast inlined literal compare if
// the operands allow it. Returns true if the compare operations
@@ -472,6 +472,11 @@
void EmitProfilingCounterDecrement(int delta);
void EmitProfilingCounterReset();
+ // Emit code to pop values from the stack associated with nested
statements
+ // like try/catch, try/finally, etc, running the finallies and unwinding
the
+ // handlers as needed.
+ void EmitUnwindBeforeReturn();
+
// Platform-specific return sequence
void EmitReturnSequence();
=======================================
--- /branches/bleeding_edge/src/ia32/full-codegen-ia32.cc Mon Jun 10
08:47:23 2013
+++ /branches/bleeding_edge/src/ia32/full-codegen-ia32.cc Wed Jun 12
04:02:51 2013
@@ -1950,8 +1950,12 @@
VisitForStackValue(expr->expression());
switch (expr->yield_kind()) {
- case Yield::INITIAL:
- case Yield::SUSPEND: {
+ case Yield::SUSPEND:
+ // Pop value from top-of-stack slot; box result into result register.
+ EmitCreateIteratorResult(false);
+ __ push(result_register());
+ // Fall through.
+ case Yield::INITIAL: {
VisitForStackValue(expr->generator_object());
__ CallRuntime(Runtime::kSuspendJSGeneratorObject, 1);
__ mov(context_register(),
@@ -1960,12 +1964,8 @@
Label resume;
__ CompareRoot(result_register(), Heap::kTheHoleValueRootIndex);
__ j(not_equal, &resume);
- if (expr->yield_kind() == Yield::SUSPEND) {
- EmitReturnIteratorResult(false);
- } else {
- __ pop(result_register());
- EmitReturnSequence();
- }
+ __ pop(result_register());
+ EmitReturnSequence();
__ bind(&resume);
context()->Plug(result_register());
@@ -1977,7 +1977,10 @@
__ mov(FieldOperand(result_register(),
JSGeneratorObject::kContinuationOffset),
Immediate(Smi::FromInt(JSGeneratorObject::kGeneratorClosed)));
- EmitReturnIteratorResult(true);
+ // Pop value from top-of-stack slot, box result into result register.
+ EmitCreateIteratorResult(true);
+ EmitUnwindBeforeReturn();
+ EmitReturnSequence();
break;
}
@@ -2006,17 +2009,18 @@
// try { received = yield result.value }
__ bind(&l_try);
- __ pop(eax); // result.value
+ EmitCreateIteratorResult(false); // pop and box
to eax
__ PushTryHandler(StackHandler::CATCH, expr->index());
const int handler_size = StackHandlerConstants::kSize;
- __ push(eax); // result.value
+ __ push(eax); // result
__ push(Operand(esp, (0 + 1) * kPointerSize + handler_size)); // g
__ CallRuntime(Runtime::kSuspendJSGeneratorObject, 1);
__ mov(context_register(),
Operand(ebp, StandardFrameConstants::kContextOffset));
__ CompareRoot(eax, Heap::kTheHoleValueRootIndex);
__ j(not_equal, &l_resume);
- EmitReturnIteratorResult(false);
+ __ pop(eax); // result
+ EmitReturnSequence();
__ bind(&l_resume); // received in eax
__ PopTryHandler();
@@ -2169,13 +2173,20 @@
}
-void FullCodeGenerator::EmitReturnIteratorResult(bool done) {
+void FullCodeGenerator::EmitCreateIteratorResult(bool done) {
Label gc_required;
Label allocated;
Handle<Map> map(isolate()->native_context()->generator_result_map());
__ Allocate(map->instance_size(), eax, ecx, edx, &gc_required,
TAG_OBJECT);
+ __ jmp(&allocated);
+
+ __ bind(&gc_required);
+ __ Push(Smi::FromInt(map->instance_size()));
+ __ CallRuntime(Runtime::kAllocateInNewSpace, 1);
+ __ mov(context_register(),
+ Operand(ebp, StandardFrameConstants::kContextOffset));
__ bind(&allocated);
__ mov(ebx, map);
@@ -2194,26 +2205,6 @@
// root set.
__ RecordWriteField(eax, JSGeneratorObject::kResultValuePropertyOffset,
ecx, edx, kDontSaveFPRegs);
-
- if (done) {
- // Exit all nested statements.
- NestedStatement* current = nesting_stack_;
- int stack_depth = 0;
- int context_length = 0;
- while (current != NULL) {
- current = current->Exit(&stack_depth, &context_length);
- }
- __ Drop(stack_depth);
- }
-
- EmitReturnSequence();
-
- __ bind(&gc_required);
- __ Push(Smi::FromInt(map->instance_size()));
- __ CallRuntime(Runtime::kAllocateInNewSpace, 1);
- __ mov(context_register(),
- Operand(ebp, StandardFrameConstants::kContextOffset));
- __ jmp(&allocated);
}
=======================================
--- /branches/bleeding_edge/src/x64/full-codegen-x64.cc Mon Jun 10 08:47:23
2013
+++ /branches/bleeding_edge/src/x64/full-codegen-x64.cc Wed Jun 12 04:02:51
2013
@@ -1974,8 +1974,12 @@
VisitForStackValue(expr->expression());
switch (expr->yield_kind()) {
- case Yield::INITIAL:
- case Yield::SUSPEND: {
+ case Yield::SUSPEND:
+ // Pop value from top-of-stack slot; box result into result register.
+ EmitCreateIteratorResult(false);
+ __ push(result_register());
+ // Fall through.
+ case Yield::INITIAL: {
VisitForStackValue(expr->generator_object());
__ CallRuntime(Runtime::kSuspendJSGeneratorObject, 1);
__ movq(context_register(),
@@ -1984,12 +1988,8 @@
Label resume;
__ CompareRoot(result_register(), Heap::kTheHoleValueRootIndex);
__ j(not_equal, &resume);
- if (expr->yield_kind() == Yield::SUSPEND) {
- EmitReturnIteratorResult(false);
- } else {
- __ pop(result_register());
- EmitReturnSequence();
- }
+ __ pop(result_register());
+ EmitReturnSequence();
__ bind(&resume);
context()->Plug(result_register());
@@ -2001,7 +2001,10 @@
__ Move(FieldOperand(result_register(),
JSGeneratorObject::kContinuationOffset),
Smi::FromInt(JSGeneratorObject::kGeneratorClosed));
- EmitReturnIteratorResult(true);
+ // Pop value from top-of-stack slot, box result into result register.
+ EmitCreateIteratorResult(true);
+ EmitUnwindBeforeReturn();
+ EmitReturnSequence();
break;
}
@@ -2031,17 +2034,18 @@
// try { received = yield result.value }
__ bind(&l_try);
- __ pop(rax); // result.value
+ EmitCreateIteratorResult(false); // pop and box
to rax
__ PushTryHandler(StackHandler::CATCH, expr->index());
const int handler_size = StackHandlerConstants::kSize;
- __ push(rax); // result.value
+ __ push(rax); // result
__ push(Operand(rsp, (0 + 1) * kPointerSize + handler_size)); // g
__ CallRuntime(Runtime::kSuspendJSGeneratorObject, 1);
__ movq(context_register(),
Operand(rbp, StandardFrameConstants::kContextOffset));
__ CompareRoot(rax, Heap::kTheHoleValueRootIndex);
__ j(not_equal, &l_resume);
- EmitReturnIteratorResult(false);
+ __ pop(rax); // result
+ EmitReturnSequence();
__ bind(&l_resume); // received in rax
__ PopTryHandler();
@@ -2195,13 +2199,20 @@
}
-void FullCodeGenerator::EmitReturnIteratorResult(bool done) {
+void FullCodeGenerator::EmitCreateIteratorResult(bool done) {
Label gc_required;
Label allocated;
Handle<Map> map(isolate()->native_context()->generator_result_map());
__ Allocate(map->instance_size(), rax, rcx, rdx, &gc_required,
TAG_OBJECT);
+ __ jmp(&allocated);
+
+ __ bind(&gc_required);
+ __ Push(Smi::FromInt(map->instance_size()));
+ __ CallRuntime(Runtime::kAllocateInNewSpace, 1);
+ __ movq(context_register(),
+ Operand(rbp, StandardFrameConstants::kContextOffset));
__ bind(&allocated);
__ Move(rbx, map);
@@ -2222,26 +2233,6 @@
// root set.
__ RecordWriteField(rax, JSGeneratorObject::kResultValuePropertyOffset,
rcx, rdx, kDontSaveFPRegs);
-
- if (done) {
- // Exit all nested statements.
- NestedStatement* current = nesting_stack_;
- int stack_depth = 0;
- int context_length = 0;
- while (current != NULL) {
- current = current->Exit(&stack_depth, &context_length);
- }
- __ Drop(stack_depth);
- }
-
- EmitReturnSequence();
-
- __ bind(&gc_required);
- __ Push(Smi::FromInt(map->instance_size()));
- __ CallRuntime(Runtime::kAllocateInNewSpace, 1);
- __ movq(context_register(),
- Operand(rbp, StandardFrameConstants::kContextOffset));
- __ jmp(&allocated);
}
=======================================
--- /branches/bleeding_edge/test/mjsunit/mjsunit.status Tue Jun 11 07:45:17
2013
+++ /branches/bleeding_edge/test/mjsunit/mjsunit.status Wed Jun 12 04:02:51
2013
@@ -46,10 +46,6 @@
# Deferred stack trace formatting is temporarily disabled.
stack-traces-gc: PASS || FAIL
-##############################################################################
-# TODO(wingo): Re-enable when GC bug from r15060 is solved.
-harmony/generators-iteration: SKIP
-
##############################################################################
# Too slow in debug mode with --stress-opt mode.
compiler/regress-stacktrace-methods: PASS, SKIP if $mode == debug
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
