Auto merge of #11803 - jdm:catch-unwind, r=nox

Avoid unwinding into C stack frames

Fix the biggest cause of #6462 by wrapping lots of JS->Rust transitions in catch_panic, and calling resume_panic after all Rust->JS transitions return.

Known issue:
* Finalizers can be called in response to any JS engine allocation that triggers a GC, so it's possible for a Rust object's Drop implementation that panics to leave an interrupted panic in TLS. This is why 30d8009 is part of this PR; the underlying problem is that there's no clear place to resume the panic after it is interrupted.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #6462 (github issue number if applicable).
- [X] There are tests for these changes OR

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11803)
<!-- Reviewable:end -->

components/script/dom/bindings/codegen/CodegenRust.py

@@ -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 @@ class CGAbstractMethod(CGThing):
         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 @@ class CGAbstractMethod(CGThing):
         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 @@ return true;""" % (getIndexedOrExpando, getNamed)
 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 @@ class CGBindingRoot(CGThing):
             '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 @@ class CallbackMethod(CallbackMember):
             "        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)

components/script/dom/node.rs

@@ -192,7 +192,9 @@ impl Node {
         debug_assert!(thread_state::get().is_script());
         let win = window_from_node(self);
         self.style_and_layout_data.set(None);
-        win.layout_chan().send(Msg::ReapStyleAndLayoutData(data)).unwrap();
+        if win.layout_chan().send(Msg::ReapStyleAndLayoutData(data)).is_err() {
+            warn!("layout thread unreachable - leaking layout data");
+        }
     }
 
     /// Adds a new child to the end of this node's list of children.

components/script/dom/testbinding.rs

@@ -580,6 +580,8 @@ impl TestBindingMethods for TestBinding {
             ptr::write_volatile(p, 0xbaadc0de);
         }
     }
+
+    fn Panic(&self) { panic!("explicit panic from script") }
 }
 
 impl TestBinding {

components/script/dom/webidls/TestBinding.webidl

@@ -455,6 +455,8 @@ interface TestBinding {
   static void funcControlledStaticMethodEnabled();
   [Func="TestBinding::condition_satisfied"]
   const unsigned short funcControlledConstEnabled = 0;
+
+  void panic();
 };
 
 partial interface TestBinding {

components/script/dom/window.rs

@@ -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);
+                }
             }
         )
     }

components/script/dom/workerglobalscope.rs

@@ -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");

components/script/script_runtime.rs

@@ -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));
 

tests/html/panic.html

@@ -0,0 +1,16 @@
+<!-- To exercise these tests, set the `dom.testbinding.enabled` to true.
+     It is expected that the browser will not abort due to failed JS engine assertions. -->
+
+<!-- Straightforward test - invoking a panic from a toplevel script execution -->
+<script>
+(new TestBinding()).panic();
+</script>
+
+<!-- invoking a panic from an event handler which is invoked by native code -->
+<!--<iframe src="data:,hi there" onload="(new TestBinding()).panic()"></iframe>-->
+
+<!-- invoking a panic from an event handler which is invoked by script -->
+<!--<div onclick="(new TestBinding()).panic()"></div>
+<script>
+document.querySelector('div').click();
+</script>-->