Fixed problem where popping a handle scope after calling an accessor

would clobber the register holding the result.

Review URL: http://codereview.chromium.org/377004


git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@3237 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

Author: [email protected]

src/ia32/macro-assembler-ia32.cc

@@ -973,14 +973,18 @@ void MacroAssembler::PushHandleScope(Register scratch) {
 }
 
 
-void MacroAssembler::PopHandleScope(Register scratch) {
+void MacroAssembler::PopHandleScope(Register saved, Register scratch) {
   ExternalReference extensions_address =
         ExternalReference::handle_scope_extensions_address();
   Label write_back;
   mov(scratch, Operand::StaticVariable(extensions_address));
   cmp(Operand(scratch), Immediate(0));
   j(equal, &write_back);
+  // Calling a runtime function messes with registers so we save and
+  // restore any one we're asked not to change
+  if (saved.is_valid()) push(saved);
   CallRuntime(Runtime::kDeleteHandleScopeExtensions, 0);
+  if (saved.is_valid()) pop(saved);
 
   bind(&write_back);
   ExternalReference limit_address =

src/ia32/macro-assembler-ia32.h

@@ -272,7 +272,10 @@ class MacroAssembler: public Assembler {
                        int result_size);
 
   void PushHandleScope(Register scratch);
-  void PopHandleScope(Register scratch);
+
+  // Pops a handle scope using the specified scratch register and
+  // ensuring that saved register, it is not no_reg, is left unchanged.
+  void PopHandleScope(Register saved, Register scratch);
 
   // Jump to a runtime routine.
   void JumpToRuntime(const ExternalReference& ext);

src/ia32/stub-cache-ia32.cc

@@ -804,8 +804,9 @@ void StubCompiler::GenerateLoadCallback(JSObject* object,
   ApiGetterEntryStub stub(callback_handle, &fun);
   __ CallStub(&stub);
 
+  // We need to avoid using eax since that now holds the result.
   Register tmp = other.is(eax) ? reg : other;
-  __ PopHandleScope(tmp);
+  __ PopHandleScope(eax, tmp);
   __ LeaveInternalFrame();
 
   __ ret(0);

test/cctest/test-accessors.cc

@@ -422,3 +422,29 @@ THREADED_TEST(StackIteration) {
       "  foo();"
       "}"))->Run();
 }
+
+
+static v8::Handle<Value> AllocateHandles(Local<String> name,
+                                         const AccessorInfo& info) {
+  for (int i = 0; i < i::kHandleBlockSize + 1; i++) {
+    v8::Local<v8::Value>::New(name);
+  }
+  return v8::Integer::New(100);
+}
+
+
+THREADED_TEST(HandleScopeSegment) {
+  // Check that we can return values past popping of handle scope
+  // segments.
+  v8::HandleScope scope;
+  v8::Handle<v8::ObjectTemplate> obj = ObjectTemplate::New();
+  obj->SetAccessor(v8_str("xxx"), AllocateHandles);
+  LocalContext env;
+  env->Global()->Set(v8_str("obj"), obj->NewInstance());
+  v8::Handle<v8::Value> result = Script::Compile(String::New(
+      "var result;"
+      "for (var i = 0; i < 4; i++)"
+      "  result = obj.xxx;"
+      "result;"))->Run();
+  CHECK_EQ(100, result->Int32Value());
+}