From d5020b69bd84b937de89b2aef5787bfdb4e37dac Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Mon, 16 Oct 2023 01:10:26 -0500 Subject: Issue #1442 Follow-up: Stop pretending proxies have a JSNative call/construct hook. https://bugzilla.mozilla.org/show_bug.cgi?id=1471924 Part 1 Also remove an erroneous debug assert and guard against future issues. This fixes crashes on vk.com but still does not behave correctly. --- dom/workers/WorkerPrivate.cpp | 2 -- js/src/builtin/Stream.cpp | 2 +- js/src/jit/BaselineIC.cpp | 3 --- js/src/jscntxtinlines.h | 17 ++--------------- js/src/jsobj.cpp | 32 ++++++++++---------------------- js/src/proxy/Proxy.cpp | 18 ------------------ js/src/vm/Interpreter.cpp | 21 +++++++++++++++++---- 7 files changed, 30 insertions(+), 65 deletions(-) diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp index 3b3de7e3b5..51ab67f708 100644 --- a/dom/workers/WorkerPrivate.cpp +++ b/dom/workers/WorkerPrivate.cpp @@ -6226,8 +6226,6 @@ WorkerPrivate::RunExpiredTimeouts(JSContext* aCx) // Guard against recursion. mRunningExpiredTimeouts = true; - MOZ_DIAGNOSTIC_ASSERT(data->mCurrentTimerNestingLevel == 0); - // Run expired timeouts. for (uint32_t index = 0; index < expiredTimeouts.Length(); index++) { TimeoutInfo*& info = expiredTimeouts[index]; diff --git a/js/src/builtin/Stream.cpp b/js/src/builtin/Stream.cpp index c8d8e3e324..26457709d8 100644 --- a/js/src/builtin/Stream.cpp +++ b/js/src/builtin/Stream.cpp @@ -2790,7 +2790,7 @@ ReadableStreamControllerCallPullIfNeeded(JSContext* cx, HandleNativeObject contr } else { pullPromise = PromiseInvokeOrNoop(cx, underlyingSource, cx->names().pull, controllerVal); } - if (!pullPromise) + if (!pullPromise || !pullPromise->is()) return false; RootedObject onPullFulfilled(cx, NewHandler(cx, ControllerPullHandler, controller)); diff --git a/js/src/jit/BaselineIC.cpp b/js/src/jit/BaselineIC.cpp index b657359d3e..a6adc122c7 100644 --- a/js/src/jit/BaselineIC.cpp +++ b/js/src/jit/BaselineIC.cpp @@ -5593,9 +5593,6 @@ TryAttachCallStub(JSContext* cx, ICCall_Fallback* stub, HandleScript script, jsb RootedObject obj(cx, &callee.toObject()); if (!obj->is()) { // Try to attach a stub for a call/construct hook on the object. - // Ignore proxies, which are special cased by callHook/constructHook. - if (obj->is()) - return true; if (JSNative hook = constructing ? obj->constructHook() : obj->callHook()) { if (op != JSOP_FUNAPPLY && !isSpread && !createSingleton) { RootedObject templateObject(cx); diff --git a/js/src/jscntxtinlines.h b/js/src/jscntxtinlines.h index c89ef86ec2..d5bd0dc46e 100644 --- a/js/src/jscntxtinlines.h +++ b/js/src/jscntxtinlines.h @@ -278,22 +278,9 @@ CallJSNativeConstructor(JSContext* cx, Native native, const CallArgs& args) * constructor to return the callee, the assertion can be removed or * (another) conjunct can be added to the antecedent. * - * Exceptions: - * - * - Proxies are exceptions to both rules: they can return primitives and - * they allow content to return the callee. - * - * - CallOrConstructBoundFunction is an exception as well because we might - * have used bind on a proxy function. - * - * - new Iterator(x) is user-hookable; it returns x.__iterator__() which - * could be any object. - * - * - (new Object(Object)) returns the callee. + * Exception: (new Object(Object)) returns the callee. */ - MOZ_ASSERT_IF(native != js::proxy_Construct && - native != js::IteratorConstructor && - (!callee->is() || callee->as().native() != obj_construct), + MOZ_ASSERT_IF((!callee->is() || callee->as().native() != obj_construct), args.rval().isObject() && callee != &args.rval().toObject()); return true; diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp index 730805e038..46f89e547e 100644 --- a/js/src/jsobj.cpp +++ b/js/src/jsobj.cpp @@ -2066,6 +2066,10 @@ JSObject::isCallable() const { if (is()) return true; + if (is()) { + const js::ProxyObject& p = as(); + return p.handler()->isCallable(const_cast(this)); + } return callHook() != nullptr; } @@ -2076,39 +2080,23 @@ JSObject::isConstructor() const const JSFunction& fun = as(); return fun.isConstructor(); } + if (is()) { + const js::ProxyObject& p = as(); + return p.handler()->isConstructor(const_cast(this)); + } return constructHook() != nullptr; } JSNative JSObject::callHook() const { - const js::Class* clasp = getClass(); - - if (JSNative call = clasp->getCall()) - return call; - - if (is()) { - const js::ProxyObject& p = as(); - if (p.handler()->isCallable(const_cast(this))) - return js::proxy_Call; - } - return nullptr; + return getClass()->getCall(); } JSNative JSObject::constructHook() const { - const js::Class* clasp = getClass(); - - if (JSNative construct = clasp->getConstruct()) - return construct; - - if (is()) { - const js::ProxyObject& p = as(); - if (p.handler()->isConstructor(const_cast(this))) - return js::proxy_Construct; - } - return nullptr; + return getClass()->getConstruct(); } bool diff --git a/js/src/proxy/Proxy.cpp b/js/src/proxy/Proxy.cpp index 8f6a262106..984e1f411d 100644 --- a/js/src/proxy/Proxy.cpp +++ b/js/src/proxy/Proxy.cpp @@ -665,24 +665,6 @@ js::proxy_HasInstance(JSContext* cx, HandleObject proxy, MutableHandleValue v, b return Proxy::hasInstance(cx, proxy, v, bp); } -bool -js::proxy_Call(JSContext* cx, unsigned argc, Value* vp) -{ - CallArgs args = CallArgsFromVp(argc, vp); - RootedObject proxy(cx, &args.callee()); - MOZ_ASSERT(proxy->is()); - return Proxy::call(cx, proxy, args); -} - -bool -js::proxy_Construct(JSContext* cx, unsigned argc, Value* vp) -{ - CallArgs args = CallArgsFromVp(argc, vp); - RootedObject proxy(cx, &args.callee()); - MOZ_ASSERT(proxy->is()); - return Proxy::construct(cx, proxy, args); -} - bool js::proxy_GetElements(JSContext* cx, HandleObject proxy, uint32_t begin, uint32_t end, ElementAdder* adder) diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp index f97ba99272..ad52234a31 100644 --- a/js/src/vm/Interpreter.cpp +++ b/js/src/vm/Interpreter.cpp @@ -457,10 +457,18 @@ js::InternalCallOrConstruct(JSContext* cx, const CallArgs& args, MaybeConstruct /* Invoke non-functions. */ if (MOZ_UNLIKELY(!args.callee().is())) { - MOZ_ASSERT_IF(construct, !args.callee().constructHook()); - JSNative call = args.callee().callHook(); - if (!call) + MOZ_ASSERT_IF(construct, !args.callee().isConstructor()); + + if (!args.callee().isCallable()) return ReportIsNotFunction(cx, args.calleev(), skipForCallee, construct); + + if (args.callee().is()) { + RootedObject proxy(cx, &args.callee()); + return Proxy::call(cx, proxy, args); + } + + JSNative call = args.callee().callHook(); + MOZ_ASSERT(call, "isCallable without a callHook?"); return CallJSNative(cx, call, args); } @@ -579,6 +587,11 @@ InternalConstruct(JSContext* cx, const AnyConstructArgs& args) return true; } + if (callee.is()) { + RootedObject proxy(cx, &callee); + return Proxy::construct(cx, proxy, args); + } + JSNative construct = callee.constructHook(); MOZ_ASSERT(construct != nullptr, "IsConstructor without a construct hook?"); @@ -4846,7 +4859,7 @@ js::SpreadCallOperation(JSContext* cx, HandleScript script, jsbytecode* pc, Hand constructing ? CONSTRUCT : NO_CONSTRUCT); } - if (MOZ_UNLIKELY(!callee.toObject().is()) && !callee.toObject().callHook()) { + if (!callee.toObject().isCallable()) { return ReportIsNotFunction(cx, callee, 2 + constructing, constructing ? CONSTRUCT : NO_CONSTRUCT); } -- cgit v1.2.3