Author: [email protected]
Date: Tue Mar 31 08:23:58 2009
New Revision: 1657
Modified:
branches/bleeding_edge/src/top.cc
branches/bleeding_edge/src/top.h
branches/bleeding_edge/test/cctest/cctest.status
branches/bleeding_edge/test/cctest/test-api.cc
branches/bleeding_edge/tools/test.py
Log:
Reland exception propagation fix.
Fix exception propagation problem where undefined was returned instead
of an empty handle in case of an exception. This problem can break
C++ programs that are not interested in catching exceptions and just
want to propagate them out by testing for empty handles.
The issue is that exceptions are not rescheduled if they are
externally caught. Externally caught exceptions have to be
rescheduled if there is a JavaScript frame on the way to the C++ frame
that holds the external handler.
A couple of tests will fail on the ARM simulator because the simulator
has separate stacks for C++ and JavaScript. I have marked the tests
as failing only on the simulator.
Review URL: http://codereview.chromium.org/56105
Modified: branches/bleeding_edge/src/top.cc
==============================================================================
--- branches/bleeding_edge/src/top.cc (original)
+++ branches/bleeding_edge/src/top.cc Tue Mar 31 08:23:58 2009
@@ -285,6 +285,7 @@
void Top::UnregisterTryCatchHandler(v8::TryCatch* that) {
ASSERT(thread_local_.try_catch_handler_ == that);
thread_local_.try_catch_handler_ = that->next_;
+ thread_local_.catcher_ = NULL;
}
@@ -724,10 +725,9 @@
// Determine reporting and whether the exception is caught externally.
bool is_caught_externally = false;
bool is_out_of_memory = exception == Failure::OutOfMemoryException();
- bool should_return_exception =
ShouldReportException(&is_caught_externally);
+ bool should_return_exception =
ShouldReportException(&is_caught_externally);
bool report_exception = !is_out_of_memory && should_return_exception;
-
// Notify debugger of exception.
Debugger::OnException(exception_handle, report_exception);
@@ -823,16 +823,36 @@
bool Top::optional_reschedule_exception(bool is_bottom_call) {
- if (!is_out_of_memory() &&
- (thread_local_.external_caught_exception_ || is_bottom_call)) {
- thread_local_.external_caught_exception_ = false;
- clear_pending_exception();
- return false;
- } else {
- thread_local_.scheduled_exception_ = pending_exception();
- clear_pending_exception();
- return true;
+ // Allways reschedule out of memory exceptions.
+ if (!is_out_of_memory()) {
+ // Never reschedule the exception if this is the bottom call.
+ bool clear_exception = is_bottom_call;
+
+ // If the exception is externally caught, clear it if there are no
+ // JavaScript frames on the way to the C++ frame that has the
+ // external handler.
+ if (thread_local_.external_caught_exception_) {
+ ASSERT(thread_local_.try_catch_handler_ != NULL);
+ Address external_handler_address =
+ reinterpret_cast<Address>(thread_local_.try_catch_handler_);
+ JavaScriptFrameIterator it;
+ if (it.done() || (it.frame()->sp() > external_handler_address)) {
+ clear_exception = true;
+ }
+ }
+
+ // Clear the exception if needed.
+ if (clear_exception) {
+ thread_local_.external_caught_exception_ = false;
+ clear_pending_exception();
+ return false;
+ }
}
+
+ // Reschedule the exception.
+ thread_local_.scheduled_exception_ = pending_exception();
+ clear_pending_exception();
+ return true;
}
Modified: branches/bleeding_edge/src/top.h
==============================================================================
--- branches/bleeding_edge/src/top.h (original)
+++ branches/bleeding_edge/src/top.h Tue Mar 31 08:23:58 2009
@@ -127,7 +127,6 @@
return !thread_local_.pending_exception_->IsTheHole();
}
static void clear_pending_message() {
- thread_local_.catcher_ = NULL;
thread_local_.has_pending_message_ = false;
thread_local_.pending_message_ = NULL;
thread_local_.pending_message_obj_ = Heap::the_hole_value();
@@ -159,9 +158,9 @@
static void setup_external_caught() {
thread_local_.external_caught_exception_ =
- (!thread_local_.pending_exception_->IsTheHole()) &&
+ has_pending_exception() &&
(thread_local_.catcher_ != NULL) &&
- (Top::thread_local_.try_catch_handler_ ==
Top::thread_local_.catcher_);
+ (thread_local_.try_catch_handler_ == thread_local_.catcher_);
}
// Tells whether the current context has experienced an out of memory
Modified: branches/bleeding_edge/test/cctest/cctest.status
==============================================================================
--- branches/bleeding_edge/test/cctest/cctest.status (original)
+++ branches/bleeding_edge/test/cctest/cctest.status Tue Mar 31 08:23:58
2009
@@ -42,3 +42,11 @@
# BUG(240): Test seems flaky on ARM.
test-api/RegExpInterruption: SKIP
+
+[ $simulator == arm ]
+
+# BUG(271): During exception propagation, we compare pointers into the
+# stack. These tests fail on the ARM simulator because the C++ and
+# the JavaScript stacks are separate.
+test-api/ExceptionOrder: FAIL
+test-api/TryCatchInTryFinally: FAIL
Modified: branches/bleeding_edge/test/cctest/test-api.cc
==============================================================================
--- branches/bleeding_edge/test/cctest/test-api.cc (original)
+++ branches/bleeding_edge/test/cctest/test-api.cc Tue Mar 31 08:23:58 2009
@@ -1812,7 +1812,8 @@
if (args.Length() < 1) return v8::Boolean::New(false);
v8::HandleScope scope;
v8::TryCatch try_catch;
- v8::Script::Compile(args[0]->ToString())->Run();
+ Local<Value> result = v8::Script::Compile(args[0]->ToString())->Run();
+ CHECK(!try_catch.HasCaught() || result.IsEmpty());
return v8::Boolean::New(try_catch.HasCaught());
}
@@ -1849,7 +1850,12 @@
// Test that a try-finally block doesn't shadow a try-catch block
// when setting up an external handler.
-THREADED_TEST(TryCatchInTryFinally) {
+//
+// BUG(271): Some of the exception propagation does not work on the
+// ARM simulator because the simulator separates the C++ stack and the
+// JS stack. This test therefore fails on the simulator. The test is
+// not threaded to allow the threading tests to run on the simulator.
+TEST(TryCatchInTryFinally) {
v8::HandleScope scope;
Local<ObjectTemplate> templ = ObjectTemplate::New();
templ->Set(v8_str("CCatcher"),
@@ -1868,6 +1874,7 @@
static void receive_message(v8::Handle<v8::Message> message,
v8::Handle<v8::Value> data) {
+ message->Get();
message_received = true;
}
@@ -1896,8 +1903,9 @@
LocalContext context(0, templ);
v8::TryCatch try_catch;
try_catch.SetVerbose(true);
- CompileRun("ThrowFromC();");
+ Local<Value> result = CompileRun("ThrowFromC();");
CHECK(try_catch.HasCaught());
+ CHECK(result.IsEmpty());
CHECK(message_received);
v8::V8::RemoveMessageListeners(check_message);
}
@@ -1943,6 +1951,7 @@
int expected = args[3]->Int32Value();
if (try_catch.HasCaught()) {
CHECK_EQ(expected, count);
+ CHECK(result.IsEmpty());
CHECK(!i::Top::has_scheduled_exception());
} else {
CHECK_NE(expected, count);
@@ -2000,7 +2009,12 @@
// Each entry is an activation, either JS or C. The index is the count at
that
// level. Stars identify activations with exception handlers, the @
identifies
// the exception handler that should catch the exception.
-THREADED_TEST(ExceptionOrder) {
+//
+// BUG(271): Some of the exception propagation does not work on the
+// ARM simulator because the simulator separates the C++ stack and the
+// JS stack. This test therefore fails on the simulator. The test is
+// not threaded to allow the threading tests to run on the simulator.
+TEST(ExceptionOrder) {
v8::HandleScope scope;
Local<ObjectTemplate> templ = ObjectTemplate::New();
templ->Set(v8_str("check"), v8::FunctionTemplate::New(JSCheck));
Modified: branches/bleeding_edge/tools/test.py
==============================================================================
--- branches/bleeding_edge/tools/test.py (original)
+++ branches/bleeding_edge/tools/test.py Tue Mar 31 08:23:58 2009
@@ -1261,7 +1261,8 @@
env = {
'mode': mode,
'system': utils.GuessOS(),
- 'arch': options.arch
+ 'arch': options.arch,
+ 'simulator': options.simulator
}
test_list = root.ListTests([], path, context, mode)
unclassified_tests += test_list
--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---
