Bug 1492977: Rework RAII exception guards r=tcampbell
authorIain Ireland <iireland@mozilla.com>
Thu, 11 Oct 2018 02:07:02 +0000
changeset 440669 838b2692a934fba342f5b288326b224405305f7f
parent 440668 336da65f35ea1237d94c4a8e6d67b8f2bd794496
child 440670 462ef74f8ce9e6ad91390158254966715fcd26da
push id34829
push useraiakab@mozilla.com
push dateThu, 11 Oct 2018 09:59:57 +0000
treeherdermozilla-central@ddcd7cc2f3cd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcampbell
bugs1492977
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1492977: Rework RAII exception guards r=tcampbell Depends on D7942 Differential Revision: https://phabricator.services.mozilla.com/D7943
js/src/jsexn.h
js/src/jsmath.cpp
js/src/vm/JSContext.cpp
js/src/vm/JSContext.h
js/src/vm/JSObject.cpp
js/src/vm/NativeObject.cpp
--- a/js/src/jsexn.h
+++ b/js/src/jsexn.h
@@ -112,30 +112,16 @@ class AutoClearPendingException
       : cx(cxArg)
     { }
 
     ~AutoClearPendingException() {
         JS_ClearPendingException(cx);
     }
 };
 
-class AutoAssertNoPendingException
-{
-    mozilla::DebugOnly<JSContext*> cx;
-
-  public:
-    explicit AutoAssertNoPendingException(JSContext* cxArg)
-      : cx(cxArg)
-    { }
-
-    ~AutoAssertNoPendingException() {
-        MOZ_ASSERT(!JS_IsExceptionPending(cx));
-    }
-};
-
 extern const char*
 ValueToSourceForError(JSContext* cx, HandleValue val, JS::UniqueChars& bytes);
 
 bool
 GetInternalError(JSContext* cx, unsigned errorNumber, MutableHandleValue error);
 bool
 GetTypeError(JSContext* cx, unsigned errorNumber, MutableHandleValue error);
 
--- a/js/src/jsmath.cpp
+++ b/js/src/jsmath.cpp
@@ -230,17 +230,17 @@ bool
 js::math_atan(JSContext* cx, unsigned argc, Value* vp)
 {
     return math_function<math_atan_impl>(cx, argc, vp);
 }
 
 double
 js::ecmaAtan2(double y, double x)
 {
-    AutoUnsafeCallWithABI unsafe;
+    AutoUnsafeCallWithABI unsafe(UnsafeABIStrictness::AllowPendingExceptions);
     return fdlibm::atan2(y, x);
 }
 
 bool
 js::math_atan2_handle(JSContext* cx, HandleValue y, HandleValue x, MutableHandleValue res)
 {
     double dy;
     if (!ToNumber(cx, y, &dy)) {
@@ -263,17 +263,17 @@ js::math_atan2(JSContext* cx, unsigned a
     CallArgs args = CallArgsFromVp(argc, vp);
 
     return math_atan2_handle(cx, args.get(0), args.get(1), args.rval());
 }
 
 double
 js::math_ceil_impl(double x)
 {
-    AutoUnsafeCallWithABI unsafe;
+    AutoUnsafeCallWithABI unsafe(UnsafeABIStrictness::AllowPendingExceptions);
     return fdlibm::ceil(x);
 }
 
 bool
 js::math_ceil_handle(JSContext* cx, HandleValue v, MutableHandleValue res)
 {
     double d;
     if(!ToNumber(cx, v, &d))
@@ -345,17 +345,17 @@ bool
 js::math_exp(JSContext* cx, unsigned argc, Value* vp)
 {
     return math_function<math_exp_impl>(cx, argc, vp);
 }
 
 double
 js::math_floor_impl(double x)
 {
-    AutoUnsafeCallWithABI unsafe;
+    AutoUnsafeCallWithABI unsafe(UnsafeABIStrictness::AllowPendingExceptions);
     return fdlibm::floor(x);
 }
 
 bool
 js::math_floor_handle(JSContext* cx, HandleValue v, MutableHandleValue r)
 {
     double d;
     if (!ToNumber(cx, v, &d)) {
@@ -438,17 +438,17 @@ js::math_fround(JSContext* cx, unsigned 
 
     return RoundFloat32(cx, args[0], args.rval());
 }
 
 
 double
 js::math_log_impl(double x)
 {
-    AutoUnsafeCallWithABI unsafe;
+    AutoUnsafeCallWithABI unsafe(UnsafeABIStrictness::AllowPendingExceptions);
     return fdlibm::log(x);
 }
 
 bool
 js::math_log_handle(JSContext* cx, HandleValue val, MutableHandleValue res)
 {
     return math_function<math_log_impl>(cx, val, res);
 }
@@ -725,17 +725,17 @@ js::GetBiggestNumberLessThan(T x)
 }
 
 template double js::GetBiggestNumberLessThan<>(double x);
 template float js::GetBiggestNumberLessThan<>(float x);
 
 double
 js::math_round_impl(double x)
 {
-    AutoUnsafeCallWithABI unsafe;
+    AutoUnsafeCallWithABI unsafe(UnsafeABIStrictness::AllowPendingExceptions);
 
     int32_t ignored;
     if (NumberEqualsInt32(x, &ignored)) {
         return x;
     }
 
     /* Some numbers are so big that adding 0.5 would give the wrong number. */
     if (ExponentComponent(x) >= int_fast16_t(FloatingPoint<double>::kExponentShift)) {
@@ -776,17 +776,17 @@ js::math_round(JSContext* cx, unsigned a
     }
 
     return math_round_handle(cx, args[0], args.rval());
 }
 
 double
 js::math_sin_impl(double x)
 {
-    AutoUnsafeCallWithABI unsafe;
+    AutoUnsafeCallWithABI unsafe(UnsafeABIStrictness::AllowPendingExceptions);
     return sin(x);
 }
 
 bool
 js::math_sin_handle(JSContext* cx, HandleValue val, MutableHandleValue res)
 {
     return math_function<math_sin_impl>(cx, val, res);
 }
@@ -809,17 +809,17 @@ js::math_sincos_impl(double x, double *s
     *sin = js::math_sin_impl(x);
     *cos = js::math_cos_impl(x);
 #endif
 }
 
 double
 js::math_sqrt_impl(double x)
 {
-    AutoUnsafeCallWithABI unsafe;
+    AutoUnsafeCallWithABI unsafe(UnsafeABIStrictness::AllowPendingExceptions);
     return sqrt(x);
 }
 
 bool
 js::math_sqrt_handle(JSContext* cx, HandleValue number, MutableHandleValue result)
 {
     return math_function<math_sqrt_impl>(cx, number, result);
 }
@@ -1080,17 +1080,17 @@ js::math_hypot_handle(JSContext* cx, Han
                     scale * sqrt(sumsq);
     res.setDouble(result);
     return true;
 }
 
 double
 js::math_trunc_impl(double x)
 {
-    AutoUnsafeCallWithABI unsafe;
+    AutoUnsafeCallWithABI unsafe(UnsafeABIStrictness::AllowPendingExceptions);
     return fdlibm::trunc(x);
 }
 
 float
 js::math_truncf_impl(float x)
 {
     AutoUnsafeCallWithABI unsafe;
     return fdlibm::truncf(x);
--- a/js/src/vm/JSContext.cpp
+++ b/js/src/vm/JSContext.cpp
@@ -1672,25 +1672,39 @@ AutoEnterOOMUnsafeRegion::crash(size_t s
         if (annotateOOMSizeCallback) {
             annotateOOMSizeCallback(size);
         }
     }
     crash(reason);
 }
 
 #ifdef DEBUG
-AutoUnsafeCallWithABI::AutoUnsafeCallWithABI()
+AutoUnsafeCallWithABI::AutoUnsafeCallWithABI(UnsafeABIStrictness strictness)
   : cx_(TlsContext.get()),
     nested_(cx_->hasAutoUnsafeCallWithABI),
     nogc(cx_)
 {
+    switch(strictness) {
+      case UnsafeABIStrictness::NoExceptions:
+        MOZ_ASSERT(!JS_IsExceptionPending(cx_));
+        checkForPendingException_ = true;
+        break;
+      case UnsafeABIStrictness::AllowPendingExceptions:
+        checkForPendingException_ = !JS_IsExceptionPending(cx_);
+        break;
+      case UnsafeABIStrictness::AllowThrownExceptions:
+        checkForPendingException_ = false;
+        break;
+    }
+
     cx_->hasAutoUnsafeCallWithABI = true;
 }
 
 AutoUnsafeCallWithABI::~AutoUnsafeCallWithABI()
 {
     MOZ_ASSERT(cx_->hasAutoUnsafeCallWithABI);
     if (!nested_) {
         cx_->hasAutoUnsafeCallWithABI = false;
         cx_->inUnsafeCallWithABI = false;
     }
+    MOZ_ASSERT_IF(checkForPendingException_, !JS_IsExceptionPending(cx_));
 }
 #endif
--- a/js/src/vm/JSContext.h
+++ b/js/src/vm/JSContext.h
@@ -1145,36 +1145,37 @@ class MOZ_RAII AutoArrayRooter : private
     friend void JS::AutoGCRooter::trace(JSTracer* trc);
 
   private:
     Value* array_;
     size_t length_;
     MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 };
 
-class AutoAssertNoException
+
+class AutoAssertNoPendingException
 {
 #ifdef DEBUG
-    JSContext* cx;
-    bool hadException;
-#endif
+    JSContext* cx_;
 
   public:
-    explicit AutoAssertNoException(JSContext* cx)
-#ifdef DEBUG
-      : cx(cx),
-        hadException(cx->isExceptionPending())
-#endif
+    explicit AutoAssertNoPendingException(JSContext* cxArg)
+      : cx_(cxArg)
     {
+        MOZ_ASSERT(!JS_IsExceptionPending(cx_));
     }
 
-    ~AutoAssertNoException()
-    {
-        MOZ_ASSERT_IF(!hadException, !cx->isExceptionPending());
+    ~AutoAssertNoPendingException() {
+        MOZ_ASSERT(!JS_IsExceptionPending(cx_));
     }
+#else
+  public:
+    explicit AutoAssertNoPendingException(JSContext* cxArg)
+    {}
+#endif
 };
 
 class MOZ_RAII AutoLockScriptData
 {
     JSRuntime* runtime;
 
   public:
     explicit AutoLockScriptData(JSRuntime* rt MOZ_GUARD_OBJECT_NOTIFIER_PARAM) {
@@ -1256,30 +1257,52 @@ class MOZ_RAII AutoEnterIonCompilation
         cx->ionCompiling = false;
         cx->ionCompilingSafeForMinorGC = false;
 #endif
     }
 
     MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 };
 
+enum UnsafeABIStrictness {
+    NoExceptions,
+    AllowPendingExceptions,
+    AllowThrownExceptions
+};
+
 // Should be used in functions called directly from JIT code (with
 // masm.callWithABI) to assert invariants in debug builds.
+// In debug mode, masm.callWithABI inserts code to verify that the
+// callee function uses AutoUnsafeCallWithABI.
+// While this object is live:
+// 1. cx->hasAutoUnsafeCallWithABI must be true.
+// 2. We can't GC.
+// 3. Exceptions should not be pending/thrown.
+//
+// Note that #3 is a precaution, not a requirement. By default, we
+// assert that the function is not called with a pending exception,
+// and that it does not throw an exception itself.
 class MOZ_RAII AutoUnsafeCallWithABI
 {
 #ifdef DEBUG
     JSContext* cx_;
     bool nested_;
+    bool checkForPendingException_;
 #endif
     JS::AutoCheckCannotGC nogc;
 
   public:
 #ifdef DEBUG
-    AutoUnsafeCallWithABI();
+    explicit AutoUnsafeCallWithABI(UnsafeABIStrictness strictness =
+                                   UnsafeABIStrictness::NoExceptions);
     ~AutoUnsafeCallWithABI();
+#else
+    explicit AutoUnsafeCallWithABI(UnsafeABIStrictness unused_ =
+                                   UnsafeABIStrictness::NoExceptions)
+    {}
 #endif
 };
 
 namespace gc {
 
 // In debug builds, set/unset the performing GC flag for the current thread.
 struct MOZ_RAII AutoSetThreadIsPerformingGC
 {
--- a/js/src/vm/JSObject.cpp
+++ b/js/src/vm/JSObject.cpp
@@ -2462,17 +2462,17 @@ js::LookupName(JSContext* cx, HandleProp
     propp.setNotFound();
     return true;
 }
 
 bool
 js::LookupNameNoGC(JSContext* cx, PropertyName* name, JSObject* envChain,
                    JSObject** objp, JSObject** pobjp, PropertyResult* propp)
 {
-    AutoAssertNoException nogc(cx);
+    AutoAssertNoPendingException nogc(cx);
 
     MOZ_ASSERT(!*objp && !*pobjp && !*propp);
 
     for (JSObject* env = envChain; env; env = env->enclosingEnvironment()) {
         if (env->getOpsLookupProperty()) {
             return false;
         }
         if (!LookupPropertyInline<NoGC>(cx, &env->as<NativeObject>(), NameToId(name), pobjp, propp)) {
--- a/js/src/vm/NativeObject.cpp
+++ b/js/src/vm/NativeObject.cpp
@@ -2560,17 +2560,17 @@ js::NativeGetProperty(JSContext* cx, Han
                       MutableHandleValue vp)
 {
     return NativeGetPropertyInline<CanGC>(cx, obj, receiver, id, NotNameLookup, vp);
 }
 
 bool
 js::NativeGetPropertyNoGC(JSContext* cx, NativeObject* obj, const Value& receiver, jsid id, Value* vp)
 {
-    AutoAssertNoException noexc(cx);
+    AutoAssertNoPendingException noexc(cx);
     return NativeGetPropertyInline<NoGC>(cx, obj, receiver, id, NotNameLookup, vp);
 }
 
 bool
 js::NativeGetElement(JSContext* cx, HandleNativeObject obj, HandleValue receiver,
                      int32_t index, MutableHandleValue vp)
 {
     RootedId id(cx);