Do not swallow compilation errors when setting breakpoint (issue 1127653003 by hausner@google.com)

[hau...@google.com]

Reviewers: Cutch,

Description:
Do not swallow compilation errors when setting breakpoint

Fixes 23352

Please review this at https://codereview.chromium.org/1127653003/

Base URL: http://dart.googlecode.com/svn/branches/bleeding_edge/dart/

Affected files (+20, -7 lines):
M runtime/vm/compiler.cc
M runtime/vm/debugger.cc


Index: runtime/vm/compiler.cc
===================================================================
--- runtime/vm/compiler.cc (revision 45505)
+++ runtime/vm/compiler.cc (working copy)
@@ -169,8 +169,8 @@
DEFINE_RUNTIME_ENTRY(CompileFunction, 1) {
const Function& function = Function::CheckedHandle(arguments.ArgAt(0));
ASSERT(!function.HasCode());
- const Error& error = Error::Handle(Compiler::CompileFunction(thread,
- function));
+ const Error& error =
+ Error::Handle(Compiler::CompileFunction(thread, function));
if (!error.IsNull()) {
Exceptions::PropagateError(error);
}
Index: runtime/vm/debugger.cc
===================================================================
--- runtime/vm/debugger.cc (revision 45505)
+++ runtime/vm/debugger.cc (working copy)
@@ -1638,6 +1638,7 @@
GrowableObjectArray& closures = GrowableObjectArray::Handle(isolate_);
Function& function = Function::Handle(isolate_);
Function& best_fit = Function::Handle(isolate_);
+ Error& error = Error::Handle(isolate_);

const ClassTable& class_table = *isolate_->class_table();
const intptr_t num_classes = class_table.NumCids();
@@ -1654,7 +1655,14 @@
continue;
}
// Parse class definition if not done yet.
- cls.EnsureIsFinalized(isolate_);
+ error = cls.EnsureIsFinalized(isolate_);
+ if (!error.IsNull()) {
+ // Ignore functions in this class.
+ // TODO(hausner): Should we propagate this error? How?
+ // EnsureIsFinalized only returns an error object if there
+ // is no longjump base on the stack.
+ continue;
+ }
functions = cls.functions();
if (!functions.IsNull()) {
const intptr_t num_functions = functions.Length();
@@ -1781,11 +1789,16 @@


RawError* Debugger::OneTimeBreakAtEntry(const Function& target_function) {
- SourceBreakpoint* bpt = SetBreakpointAtEntry(target_function);
- if (bpt != NULL) {
- bpt->SetIsOneShot();
+ LongJumpScope jump;
+ if (setjmp(*jump.Set()) == 0) {
+ SourceBreakpoint* bpt = SetBreakpointAtEntry(target_function);
+ if (bpt != NULL) {
+ bpt->SetIsOneShot();
+ }
+ return Error::null();
+ } else {
+ return isolate_->object_store()->sticky_error();
}
- return Error::null();
}

[johnmc...@google.com]

lgtm



https://codereview.chromium.org/1127653003/

[re...@chromiumcodereview-hr.appspotmail.com]

Committed patchset #1 (id:1) manually as r45510 (presubmit successful).

https://codereview.chromium.org/1127653003/