Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid unwinding into C stack frames #11803

Merged
merged 3 commits into from Jun 22, 2016
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Prev

Wrap executions of Rust code called from JS in catch_unwind. Propagat…

…e the interrupted panic to the origin of the JS execution via TLS before resuming. Fix #6462.
  • Loading branch information
jdm committed Jun 22, 2016
commit b8853554dbe595dedf3d68532af66bbf0c329b42
@@ -2145,7 +2145,7 @@ class CGAbstractMethod(CGThing):
"""
def __init__(self, descriptor, name, returnType, args, inline=False,
alwaysInline=False, extern=False, pub=False, templateArgs=None,
unsafe=False, docs=None):
unsafe=False, docs=None, doesNotPanic=False):
CGThing.__init__(self)
self.descriptor = descriptor
self.name = name
@@ -2157,6 +2157,7 @@ def __init__(self, descriptor, name, returnType, args, inline=False,
self.pub = pub
self.unsafe = unsafe
self.docs = docs
self.catchPanic = self.extern and not doesNotPanic

def _argstring(self):
return ', '.join([a.declare() for a in self.args])
@@ -2199,6 +2200,19 @@ def define(self):
if self.unsafe and not self.extern:
body = CGWrapper(CGIndenter(body), pre="unsafe {\n", post="\n}")

if self.catchPanic:
body = CGWrapper(CGIndenter(body),
pre="let result = panic::catch_unwind(AssertUnwindSafe(|| {\n",
post=("""}));
match result {
Ok(result) => result,
Err(error) => {
store_panic_result(error);
return%s;
}
}
""" % ("" if self.returnType == "void" else " false")))

return CGWrapper(CGIndenter(body),
pre=self.definition_prologue(),
post=self.definition_epilogue()).define()
@@ -2447,9 +2461,9 @@ class CGAbstractExternMethod(CGAbstractMethod):
Abstract base class for codegen of implementation-only (no
declaration) static methods.
"""
def __init__(self, descriptor, name, returnType, args):
def __init__(self, descriptor, name, returnType, args, doesNotPanic=False):
CGAbstractMethod.__init__(self, descriptor, name, returnType, args,
inline=False, extern=True)
inline=False, extern=True, doesNotPanic=doesNotPanic)


class PropertyArrays():
@@ -4830,7 +4844,7 @@ def definition_body(self):
class CGDOMJSProxyHandler_className(CGAbstractExternMethod):
def __init__(self, descriptor):
args = [Argument('*mut JSContext', 'cx'), Argument('HandleObject', '_proxy')]
CGAbstractExternMethod.__init__(self, descriptor, "className", "*const i8", args)
CGAbstractExternMethod.__init__(self, descriptor, "className", "*const i8", args, doesNotPanic=True)
self.descriptor = descriptor

def getBody(self):
@@ -4845,7 +4859,7 @@ class CGAbstractClassHook(CGAbstractExternMethod):
Meant for implementing JSClass hooks, like Finalize or Trace. Does very raw
'this' unwrapping as it assumes that the unwrapped type is always known.
"""
def __init__(self, descriptor, name, returnType, args):
def __init__(self, descriptor, name, returnType, args, doesNotPanic=False):
CGAbstractExternMethod.__init__(self, descriptor, name, returnType,
args)

@@ -4905,7 +4919,7 @@ class CGClassTraceHook(CGAbstractClassHook):
def __init__(self, descriptor):
args = [Argument('*mut JSTracer', 'trc'), Argument('*mut JSObject', 'obj')]
CGAbstractClassHook.__init__(self, descriptor, TRACE_HOOK_NAME, 'void',
args)
args, doesNotPanic=True)
self.traceGlobal = descriptor.isGlobal()

def generate_code(self):
@@ -5597,11 +5611,13 @@ def __init__(self, config, prefix, webIDLFile):
'mem::heap_size_of_raw_self_and_children',
'libc',
'util::prefs',
'script_runtime::{store_panic_result, maybe_take_panic_result}',
'std::borrow::ToOwned',
'std::cmp',
'std::mem',
'std::num',
'std::os',
'std::panic::{self, AssertUnwindSafe}',
'std::ptr',
'std::str',
'std::rc',
@@ -6088,6 +6104,9 @@ def getCall(self):
" length_: ${argc} as ::libc::size_t,\n"
" elements_: ${argv}\n"
" }, rval.handle_mut());\n"
"if let Some(error) = maybe_take_panic_result() {\n"
" panic::resume_unwind(error);\n"
"}\n"
"if !ok {\n"
" return Err(JSFailed);\n"
"}\n").substitute(replacements)
@@ -61,7 +61,7 @@ use script_layout_interface::message::{Msg, Reflow, ReflowQueryType, ScriptReflo
use script_layout_interface::reporter::CSSErrorReporter;
use script_layout_interface::rpc::{ContentBoxResponse, ContentBoxesResponse, LayoutRPC};
use script_layout_interface::rpc::{MarginStyleResponse, ResolvedStyleResponse};
use script_runtime::{ScriptChan, ScriptPort};
use script_runtime::{ScriptChan, ScriptPort, maybe_take_panic_result};
use script_thread::SendableMainThreadScriptChan;
use script_thread::{MainThreadScriptChan, MainThreadScriptMsg, RunnableWrapper};
use script_traits::{ConstellationControlMsg, UntrustedNodeAddress};
@@ -74,6 +74,7 @@ use std::collections::{HashMap, HashSet};
use std::default::Default;
use std::ffi::CString;
use std::io::{Write, stderr, stdout};
use std::panic;
use std::rc::Rc;
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::mpsc::TryRecvError::{Disconnected, Empty};
@@ -914,6 +915,10 @@ impl<'a, T: Reflectable> ScriptHelpers for &'a T {
report_pending_exception(cx, globalhandle.get());
}
}

if let Some(error) = maybe_take_panic_result() {
panic::resume_unwind(error);
}
}
)
}
@@ -28,11 +28,12 @@ use msg::constellation_msg::{PipelineId, ReferrerPolicy, PanicMsg};
use net_traits::{LoadContext, ResourceThreads, load_whole_resource};
use net_traits::{RequestSource, LoadOrigin, CustomResponseSender, IpcSend};
use profile_traits::{mem, time};
use script_runtime::{CommonScriptMsg, ScriptChan, ScriptPort};
use script_runtime::{CommonScriptMsg, ScriptChan, ScriptPort, maybe_take_panic_result};
use script_traits::ScriptMsg as ConstellationMsg;
use script_traits::{MsDuration, TimerEvent, TimerEventId, TimerEventRequest, TimerSource};
use std::cell::Cell;
use std::default::Default;
use std::panic;
use std::rc::Rc;
use std::sync::Arc;
use std::sync::atomic::{AtomicBool, Ordering};
@@ -320,8 +321,14 @@ impl WorkerGlobalScopeMethods for WorkerGlobalScope {
}
};

match self.runtime.evaluate_script(
self.reflector().get_jsobject(), &source, url.as_str(), 1, rval.handle_mut()) {
let result = self.runtime.evaluate_script(
self.reflector().get_jsobject(), &source, url.as_str(), 1, rval.handle_mut());

if let Some(error) = maybe_take_panic_result() {
panic::resume_unwind(error);
}

match result {
Ok(_) => (),
Err(_) => {
println!("evaluate_script failed");
@@ -19,7 +19,8 @@ use js::jsapi::{JSObject, RuntimeOptionsRef, SetPreserveWrapperCallback};
use js::rust::Runtime;
use profile_traits::mem::{Report, ReportKind, ReportsChan};
use script_thread::{Runnable, STACK_ROOTS, trace_thread};
use std::cell::Cell;
use std::any::Any;
use std::cell::{RefCell, Cell};
use std::io::{Write, stdout};
use std::marker::PhantomData;
use std::os;
@@ -321,6 +322,21 @@ pub fn get_reports(cx: *mut JSContext, path_seg: String) -> Vec<Report> {
reports
}

thread_local!(static PANIC_RESULT: RefCell<Option<Box<Any + Send>>> = RefCell::new(None));

pub fn store_panic_result(error: Box<Any + Send>) {
PANIC_RESULT.with(|result| {
assert!(result.borrow().is_none());
*result.borrow_mut() = Some(error);
});
}

pub fn maybe_take_panic_result() -> Option<Box<Any + Send>> {
PANIC_RESULT.with(|result| {
result.borrow_mut().take()
})
}

thread_local!(static GC_CYCLE_START: Cell<Option<Tm>> = Cell::new(None));
thread_local!(static GC_SLICE_START: Cell<Option<Tm>> = Cell::new(None));

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.