Author: olehougaard
Date: Thu Dec 18 03:28:13 2008
New Revision: 999
Added:
branches/bleeding_edge/test/mjsunit/throw-and-catch-function.js
Modified:
branches/bleeding_edge/src/ast.h
branches/bleeding_edge/src/codegen-arm.cc
branches/bleeding_edge/src/codegen-ia32.cc
branches/bleeding_edge/src/contexts.h
branches/bleeding_edge/src/factory.cc
branches/bleeding_edge/src/factory.h
branches/bleeding_edge/src/heap.cc
branches/bleeding_edge/src/heap.h
branches/bleeding_edge/src/objects-inl.h
branches/bleeding_edge/src/objects.h
branches/bleeding_edge/src/parser.cc
branches/bleeding_edge/src/runtime.cc
branches/bleeding_edge/src/runtime.h
Log:
Fixing a subtle bug in receiver resolution when a thrown and caught
function is called from a catch-block.
Second attempt - now with better memory efficiency.
Review URL: http://codereview.chromium.org/14834
Modified: branches/bleeding_edge/src/ast.h
==============================================================================
--- branches/bleeding_edge/src/ast.h (original)
+++ branches/bleeding_edge/src/ast.h Thu Dec 18 03:28:13 2008
@@ -409,15 +409,18 @@
class WithEnterStatement: public Statement {
public:
- explicit WithEnterStatement(Expression* expression)
- : expression_(expression) { }
+ explicit WithEnterStatement(Expression* expression, bool is_catch_block)
+ : expression_(expression), is_catch_block_(is_catch_block) { }
virtual void Accept(AstVisitor* v);
Expression* expression() const { return expression_; }
+ bool is_catch_block() const { return is_catch_block_; }
+
private:
Expression* expression_;
+ bool is_catch_block_;
};
Modified: branches/bleeding_edge/src/codegen-arm.cc
==============================================================================
--- branches/bleeding_edge/src/codegen-arm.cc (original)
+++ branches/bleeding_edge/src/codegen-arm.cc Thu Dec 18 03:28:13 2008
@@ -1276,7 +1276,11 @@
Comment cmnt(masm_, "[ WithEnterStatement");
CodeForStatement(node);
Load(node->expression());
- __ CallRuntime(Runtime::kPushContext, 1);
+ if (node->is_catch_block()) {
+ __ CallRuntime(Runtime::kPushCatchContext, 1);
+ } else {
+ __ CallRuntime(Runtime::kPushContext, 1);
+ }
if (kDebug) {
Label verified_true;
__ cmp(r0, Operand(cp));
Modified: branches/bleeding_edge/src/codegen-ia32.cc
==============================================================================
--- branches/bleeding_edge/src/codegen-ia32.cc (original)
+++ branches/bleeding_edge/src/codegen-ia32.cc Thu Dec 18 03:28:13 2008
@@ -1586,7 +1586,11 @@
Comment cmnt(masm_, "[ WithEnterStatement");
CodeForStatement(node);
Load(node->expression());
- __ CallRuntime(Runtime::kPushContext, 1);
+ if (node->is_catch_block()) {
+ __ CallRuntime(Runtime::kPushCatchContext, 1);
+ } else {
+ __ CallRuntime(Runtime::kPushContext, 1);
+ }
if (kDebug) {
Label verified_true;
Modified: branches/bleeding_edge/src/contexts.h
==============================================================================
--- branches/bleeding_edge/src/contexts.h (original)
+++ branches/bleeding_edge/src/contexts.h Thu Dec 18 03:28:13 2008
@@ -264,6 +264,12 @@
global_context()->set_out_of_memory(Heap::true_value());
}
+ // The exception holder is the object used as a with object in
+ // the implementation of a catch block.
+ bool is_exception_holder(Object* object) {
+ return IsCatchContext() && extension() == object;
+ }
+
#define GLOBAL_CONTEXT_FIELD_ACCESSORS(index, type, name) \
void set_##name(type* value) { \
ASSERT(IsGlobalContext()); \
Modified: branches/bleeding_edge/src/factory.cc
==============================================================================
--- branches/bleeding_edge/src/factory.cc (original)
+++ branches/bleeding_edge/src/factory.cc Thu Dec 18 03:28:13 2008
@@ -130,8 +130,12 @@
Handle<Context> Factory::NewWithContext(Handle<Context> previous,
- Handle<JSObject> extension) {
- CALL_HEAP_FUNCTION(Heap::AllocateWithContext(*previous, *extension),
Context);
+ Handle<JSObject> extension,
+ bool is_catch_context) {
+ CALL_HEAP_FUNCTION(Heap::AllocateWithContext(*previous,
+ *extension,
+ is_catch_context),
+ Context);
}
Modified: branches/bleeding_edge/src/factory.h
==============================================================================
--- branches/bleeding_edge/src/factory.h (original)
+++ branches/bleeding_edge/src/factory.h Thu Dec 18 03:28:13 2008
@@ -126,7 +126,8 @@
// Create a 'with' context.
static Handle<Context> NewWithContext(Handle<Context> previous,
- Handle<JSObject> extension);
+ Handle<JSObject> extension,
+ bool is_catch_context);
// Return the Symbol maching the passed in string.
static Handle<String> SymbolFromString(Handle<String> value);
Modified: branches/bleeding_edge/src/heap.cc
==============================================================================
--- branches/bleeding_edge/src/heap.cc (original)
+++ branches/bleeding_edge/src/heap.cc Thu Dec 18 03:28:13 2008
@@ -1017,6 +1017,10 @@
obj = AllocateMap(FIXED_ARRAY_TYPE, HeapObject::kHeaderSize);
if (obj->IsFailure()) return false;
+ catch_context_map_ = Map::cast(obj);
+
+ obj = AllocateMap(FIXED_ARRAY_TYPE, HeapObject::kHeaderSize);
+ if (obj->IsFailure()) return false;
global_context_map_ = Map::cast(obj);
obj = AllocateMap(JS_FUNCTION_TYPE, JSFunction::kSize);
@@ -2387,11 +2391,13 @@
}
-Object* Heap::AllocateWithContext(Context* previous, JSObject* extension) {
+Object* Heap::AllocateWithContext(Context* previous,
+ JSObject* extension,
+ bool is_catch_context) {
Object* result = Heap::AllocateFixedArray(Context::MIN_CONTEXT_SLOTS);
if (result->IsFailure()) return result;
Context* context = reinterpret_cast<Context*>(result);
- context->set_map(context_map());
+ context->set_map(is_catch_context ? catch_context_map() : context_map());
context->set_closure(previous->closure());
context->set_fcontext(previous->fcontext());
context->set_previous(previous);
Modified: branches/bleeding_edge/src/heap.h
==============================================================================
--- branches/bleeding_edge/src/heap.h (original)
+++ branches/bleeding_edge/src/heap.h Thu Dec 18 03:28:13 2008
@@ -92,6 +92,7 @@
V(Map, fixed_array_map) \
V(Map, hash_table_map) \
V(Map, context_map) \
+ V(Map, catch_context_map) \
V(Map, global_context_map) \
V(Map, code_map) \
V(Map, oddball_map) \
@@ -430,7 +431,9 @@
static Object* AllocateFunctionContext(int length, JSFunction* closure);
// Allocate a 'with' context.
- static Object* AllocateWithContext(Context* previous, JSObject*
extension);
+ static Object* AllocateWithContext(Context* previous,
+ JSObject* extension,
+ bool is_catch_context);
// Allocates a new utility object in the old generation.
static Object* AllocateStruct(InstanceType type);
Modified: branches/bleeding_edge/src/objects-inl.h
==============================================================================
--- branches/bleeding_edge/src/objects-inl.h (original)
+++ branches/bleeding_edge/src/objects-inl.h Thu Dec 18 03:28:13 2008
@@ -348,7 +348,14 @@
bool Object::IsContext() {
return Object::IsHeapObject()
&& (HeapObject::cast(this)->map() == Heap::context_map() ||
+ HeapObject::cast(this)->map() == Heap::catch_context_map() ||
HeapObject::cast(this)->map() == Heap::global_context_map());
+}
+
+
+bool Object::IsCatchContext() {
+ return Object::IsHeapObject()
+ && HeapObject::cast(this)->map() == Heap::catch_context_map();
}
Modified: branches/bleeding_edge/src/objects.h
==============================================================================
--- branches/bleeding_edge/src/objects.h (original)
+++ branches/bleeding_edge/src/objects.h Thu Dec 18 03:28:13 2008
@@ -616,6 +616,7 @@
inline bool IsFixedArray();
inline bool IsDescriptorArray();
inline bool IsContext();
+ inline bool IsCatchContext();
inline bool IsGlobalContext();
inline bool IsJSFunction();
inline bool IsCode();
Modified: branches/bleeding_edge/src/parser.cc
==============================================================================
--- branches/bleeding_edge/src/parser.cc (original)
+++ branches/bleeding_edge/src/parser.cc Thu Dec 18 03:28:13 2008
@@ -120,7 +120,10 @@
Statement* ParseContinueStatement(bool* ok);
Statement* ParseBreakStatement(ZoneStringList* labels, bool* ok);
Statement* ParseReturnStatement(bool* ok);
- Block* WithHelper(Expression* obj, ZoneStringList* labels, bool* ok);
+ Block* WithHelper(Expression* obj,
+ ZoneStringList* labels,
+ bool is_catch_block,
+ bool* ok);
Statement* ParseWithStatement(ZoneStringList* labels, bool* ok);
CaseClause* ParseCaseClause(bool* default_seen_ptr, bool* ok);
SwitchStatement* ParseSwitchStatement(ZoneStringList* labels, bool* ok);
@@ -1923,7 +1926,10 @@
}
-Block* Parser::WithHelper(Expression* obj, ZoneStringList* labels, bool*
ok) {
+Block* Parser::WithHelper(Expression* obj,
+ ZoneStringList* labels,
+ bool is_catch_block,
+ bool* ok) {
// Parse the statement and collect escaping labels.
ZoneList<Label*>* label_list = NEW(ZoneList<Label*>(0));
LabelCollector collector(label_list);
@@ -1940,7 +1946,7 @@
Block* result = NEW(Block(NULL, 2, false));
if (result) {
- result->AddStatement(NEW(WithEnterStatement(obj)));
+ result->AddStatement(NEW(WithEnterStatement(obj, is_catch_block)));
// Create body block.
Block* body = NEW(Block(NULL, 1, false));
@@ -1976,7 +1982,7 @@
Expression* expr = ParseExpression(true, CHECK_OK);
Expect(Token::RPAREN, CHECK_OK);
- return WithHelper(expr, labels, CHECK_OK);
+ return WithHelper(expr, labels, false, CHECK_OK);
}
@@ -2137,7 +2143,7 @@
catch_var = top_scope_->NewTemporary(Factory::catch_var_symbol());
Expression* obj = MakeCatchContext(name, catch_var);
{ Target target(this, &catch_collector);
- catch_block = WithHelper(obj, NULL, CHECK_OK);
+ catch_block = WithHelper(obj, NULL, true, CHECK_OK);
}
} else {
Expect(Token::LBRACE, CHECK_OK);
Modified: branches/bleeding_edge/src/runtime.cc
==============================================================================
--- branches/bleeding_edge/src/runtime.cc (original)
+++ branches/bleeding_edge/src/runtime.cc Thu Dec 18 03:28:13 2008
@@ -3428,19 +3428,15 @@
return result; // non-failure
}
-
-static Object* Runtime_PushContext(Arguments args) {
- NoHandleAllocation ha;
- ASSERT(args.length() == 1);
-
+static Object* PushContextHelper(Object* object, bool is_catch_context) {
// Convert the object to a proper JavaScript object.
- Object* object = args[0];
- if (!object->IsJSObject()) {
- object = object->ToObject();
- if (object->IsFailure()) {
- if (!Failure::cast(object)->IsInternalError()) return object;
+ Object* js_object = object;
+ if (!js_object->IsJSObject()) {
+ js_object = js_object->ToObject();
+ if (js_object->IsFailure()) {
+ if (!Failure::cast(js_object)->IsInternalError()) return js_object;
HandleScope scope;
- Handle<Object> handle(args[0]);
+ Handle<Object> handle(object);
Handle<Object> result =
Factory::NewTypeError("with_expression", HandleVector(&handle,
1));
return Top::Throw(*result);
@@ -3448,15 +3444,32 @@
}
Object* result =
- Heap::AllocateWithContext(Top::context(), JSObject::cast(object));
+ Heap::AllocateWithContext(Top::context(),
+ JSObject::cast(js_object),
+ is_catch_context);
if (result->IsFailure()) return result;
- Top::set_context(Context::cast(result));
+ Context* context = Context::cast(result);
+ Top::set_context(context);
return result;
}
+static Object* Runtime_PushContext(Arguments args) {
+ NoHandleAllocation ha;
+ ASSERT(args.length() == 1);
+ return PushContextHelper(args[0], false);
+}
+
+
+static Object* Runtime_PushCatchContext(Arguments args) {
+ NoHandleAllocation ha;
+ ASSERT(args.length() == 1);
+ return PushContextHelper(args[0], true);
+}
+
+
static Object* Runtime_LookupContext(Arguments args) {
HandleScope scope;
ASSERT(args.length() == 2);
@@ -3554,9 +3567,14 @@
if (!holder.is_null() && holder->IsJSObject()) {
ASSERT(Handle<JSObject>::cast(holder)->HasProperty(*name));
JSObject* object = JSObject::cast(*holder);
- JSObject* receiver = (object->IsGlobalObject())
- ? GlobalObject::cast(object)->global_receiver()
- : ComputeReceiverForNonGlobal(object);
+ JSObject* receiver;
+ if (object->IsGlobalObject()) {
+ receiver = GlobalObject::cast(object)->global_receiver();
+ } else if (context->is_exception_holder(*holder)) {
+ receiver = Top::context()->global()->global_receiver();
+ } else {
+ receiver = ComputeReceiverForNonGlobal(object);
+ }
// No need to unhole the value here. This is taken care of by the
// GetProperty function.
Object* value = object->GetProperty(*name);
@@ -5220,7 +5238,9 @@
Handle<Context> previous(context_chain->previous());
Handle<JSObject> extension(JSObject::cast(context_chain->extension()));
return Factory::NewWithContext(
- CopyWithContextChain(function_context, previous), extension);
+ CopyWithContextChain(function_context, previous),
+ extension,
+ context_chain->IsCatchContext());
}
Modified: branches/bleeding_edge/src/runtime.h
==============================================================================
--- branches/bleeding_edge/src/runtime.h (original)
+++ branches/bleeding_edge/src/runtime.h Thu Dec 18 03:28:13 2008
@@ -264,6 +264,7 @@
/* Contexts */ \
F(NewContext, 1) \
F(PushContext, 1) \
+ F(PushCatchContext, 1) \
F(LookupContext, 2) \
F(LoadContextSlot, 2) \
F(LoadContextSlotNoReferenceError, 2) \
Added: branches/bleeding_edge/test/mjsunit/throw-and-catch-function.js
==============================================================================
--- (empty file)
+++ branches/bleeding_edge/test/mjsunit/throw-and-catch-function.js Thu Dec
18 03:28:13 2008
@@ -0,0 +1,50 @@
+// Copyright 2008 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+// * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+// * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following
+// disclaimer in the documentation and/or other materials provided
+// with the distribution.
+// * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived
+// from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+var g = this;
+var x = new Object();
+x.e = function() { return this; };
+try {
+ throw x.e;
+} catch (e) {
+ assertTrue(e() === g);
+}
+try {
+ throw x.e;
+} catch (e) {
+ with(x) { assertTrue(e() === x); }
+}
+with(x) {
+ try { throw e; } catch (e) { assertTrue(e() === g); }
+}
+var e = 0;
+try {
+ throw x.e;
+} catch (e) {
+ var e = 7;
+}
+assertEquals(0, e);
--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---
