From cc2589c33379ace185b60b227bea9b7f971b3038 Mon Sep 17 00:00:00 2001 From: Jason Orendorff Date: Tue, 30 Oct 2018 21:58:21 +0000 Subject: [PATCH] Bug 1499813 - Part 2: JSObject::unwrapAs() and canUnwrapAs() methods. r=tcampbell Depends on D9834 Differential Revision: https://phabricator.services.mozilla.com/D9835 --HG-- extra : moz-landing-system : lando --- js/src/builtin/DataViewObject.cpp | 7 ---- js/src/builtin/Stream.cpp | 19 +++-------- js/src/jsapi.cpp | 15 ++------- js/src/jsfriendapi.h | 6 ---- js/src/vm/JSObject.h | 55 +++++++++++++++++++++++++++++++ js/src/vm/SavedFrame.h | 8 ----- js/src/vm/SavedStacks-inl.h | 2 +- js/src/vm/SavedStacks.cpp | 2 +- js/src/vm/StructuredClone.cpp | 8 ++--- js/src/vm/TypedArrayObject.cpp | 3 +- 10 files changed, 69 insertions(+), 56 deletions(-) diff --git a/js/src/builtin/DataViewObject.cpp b/js/src/builtin/DataViewObject.cpp index d5a7f62f0b28..5b594ecbd3fe 100644 --- a/js/src/builtin/DataViewObject.cpp +++ b/js/src/builtin/DataViewObject.cpp @@ -957,13 +957,6 @@ const JSPropertySpec DataViewObject::properties[] = { JS_PS_END }; -JS_FRIEND_API(bool) -JS_IsDataViewObject(JSObject* obj) -{ - obj = CheckedUnwrap(obj); - return obj ? obj->is() : false; -} - JS_FRIEND_API(uint32_t) JS_GetDataViewByteOffset(JSObject* obj) { diff --git a/js/src/builtin/Stream.cpp b/js/src/builtin/Stream.cpp index 8e5fd37aa434..a9fa52be3664 100644 --- a/js/src/builtin/Stream.cpp +++ b/js/src/builtin/Stream.cpp @@ -170,18 +170,7 @@ template bool IsMaybeWrapped(const HandleValue v) { - if (!v.isObject()) { - return false; - } - JSObject* obj = &v.toObject(); - if (obj->is()) { - return true; - } - obj = CheckedUnwrap(obj); - if (!obj) { - return false; - } - return obj->is(); + return v.isObject() && v.toObject().canUnwrapAs(); } static inline uint32_t @@ -2742,7 +2731,7 @@ static MOZ_MUST_USE bool ReadableStreamDefaultController_close_impl(JSContext* cx, const CallArgs& args) { Rooted controller(cx); - controller = &UncheckedUnwrap(&args.thisv().toObject())->as(); + controller = &args.thisv().toObject().unwrapAs(); // Steps 2-3. if (!VerifyControllerStateForClosing(cx, controller)) { @@ -2773,7 +2762,7 @@ static MOZ_MUST_USE bool ReadableStreamDefaultController_enqueue_impl(JSContext* cx, const CallArgs& args) { Rooted controller(cx); - controller = &UncheckedUnwrap(&args.thisv().toObject())->as(); + controller = &args.thisv().toObject().unwrapAs(); // Step 2: If this.[[closeRequested]] is true, throw a TypeError exception. if (ControllerFlags(controller) & ControllerFlag_CloseRequested) { @@ -2819,7 +2808,7 @@ static MOZ_MUST_USE bool ReadableStreamDefaultController_error_impl(JSContext* cx, const CallArgs& args) { Rooted controller(cx); - controller = &UncheckedUnwrap(&args.thisv().toObject())->as(); + controller = &args.thisv().toObject().unwrapAs(); // Step 2: Let stream be this.[[controlledReadableStream]]. // Step 3: If stream.[[state]] is not "readable", throw a TypeError exception. diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index 54476b904426..246f5393b8c6 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -4719,28 +4719,19 @@ JS::NewReadableExternalSourceStreamObject(JSContext* cx, void* underlyingSource, JS_PUBLIC_API(bool) JS::IsReadableStream(JSObject* obj) { - if (IsWrapper(obj)) { - obj = CheckedUnwrap(obj); - } - return obj && obj->is(); + return obj->canUnwrapAs(); } JS_PUBLIC_API(bool) JS::IsReadableStreamReader(JSObject* obj) { - if (IsWrapper(obj)) { - obj = CheckedUnwrap(obj); - } - return obj && obj->is(); + return obj->canUnwrapAs(); } JS_PUBLIC_API(bool) JS::IsReadableStreamDefaultReader(JSObject* obj) { - if (IsWrapper(obj)) { - obj = CheckedUnwrap(obj); - } - return obj && obj->is(); + return obj->canUnwrapAs(); } template diff --git a/js/src/jsfriendapi.h b/js/src/jsfriendapi.h index 4988696a0417..3ebebabcfc46 100644 --- a/js/src/jsfriendapi.h +++ b/js/src/jsfriendapi.h @@ -2068,12 +2068,6 @@ JS_DetachArrayBuffer(JSContext* cx, JS::HandleObject obj); extern JS_FRIEND_API(bool) JS_IsDetachedArrayBufferObject(JSObject* obj); -/** - * Check whether obj supports JS_GetDataView* APIs. - */ -JS_FRIEND_API(bool) -JS_IsDataViewObject(JSObject* obj); - /** * Create a new DataView using the given buffer for storage. The given buffer * must be an ArrayBuffer or SharedArrayBuffer (or a cross-compartment wrapper diff --git a/js/src/vm/JSObject.h b/js/src/vm/JSObject.h index 723f66f7cc61..e21354bea8df 100644 --- a/js/src/vm/JSObject.h +++ b/js/src/vm/JSObject.h @@ -13,6 +13,7 @@ #include "js/Conversions.h" #include "js/GCVector.h" #include "js/HeapAPI.h" +#include "js/Wrapper.h" #include "vm/Printer.h" #include "vm/Shape.h" #include "vm/StringType.h" @@ -535,6 +536,27 @@ class JSObject : public js::gc::Cell return *static_cast(this); } + /* + * True if either this or CheckedUnwrap(this) is an object of class T. + * (Only two objects are checked, regardless of how many wrappers there + * are.) + * + * /!\ Note: This can be true at one point, but false later for the same + * object, thanks to js::NukeCrossCompartmentWrapper and friends. + */ + template + bool canUnwrapAs(); + + /* + * Unwrap and downcast to class T. + * + * Precondition: `this->canUnwrapAs()`. Note that it's not enough to + * have checked this at some point in the past; if there's any doubt as to + * whether js::Nuke* could have been called in the meantime, check again. + */ + template + T& unwrapAs(); + #if defined(DEBUG) || defined(JS_JITSPEW) void dump(js::GenericPrinter& fp) const; void dump() const; @@ -590,6 +612,39 @@ js::HandleBase::as() const return Handle::fromMarkedLocation(reinterpret_cast(self.address())); } +template +bool +JSObject::canUnwrapAs() +{ + static_assert(!std::is_convertible::value, + "T can't be a Wrapper type; this function discards wrappers"); + + if (is()) { + return true; + } + JSObject* obj = js::CheckedUnwrap(this); + return obj && obj->is(); +} + +template +T& +JSObject::unwrapAs() +{ + static_assert(!std::is_convertible::value, + "T can't be a Wrapper type; this function discards wrappers"); + + if (is()) { + return as(); + } + + // Since the caller just called canUnwrapAs(), which does a + // CheckedUnwrap, this does not need to repeat the security check. + JSObject* unwrapped = js::UncheckedUnwrap(this); + MOZ_ASSERT(js::CheckedUnwrap(this) == unwrapped, + "check that the security check we skipped really is redundant"); + return unwrapped->as(); +} + /* * The only sensible way to compare JSObject with == is by identity. We use * const& instead of * as a syntactic way to assert non-null. This leads to an diff --git a/js/src/vm/SavedFrame.h b/js/src/vm/SavedFrame.h index 5e0bc5bbbc94..118046179ae4 100644 --- a/js/src/vm/SavedFrame.h +++ b/js/src/vm/SavedFrame.h @@ -97,14 +97,6 @@ class SavedFrame : public NativeObject { RootedIterator end() { return RootedIterator(); } }; - static bool isSavedFrameOrWrapper(JSObject& obj) { - auto unwrapped = CheckedUnwrap(&obj); - if (!unwrapped) { - return false; - } - return unwrapped->is(); - } - struct Lookup; struct HashPolicy; diff --git a/js/src/vm/SavedStacks-inl.h b/js/src/vm/SavedStacks-inl.h index 80fa313b6f60..7edec9dda6f5 100644 --- a/js/src/vm/SavedStacks-inl.h +++ b/js/src/vm/SavedStacks-inl.h @@ -23,7 +23,7 @@ inline void js::AssertObjectIsSavedFrameOrWrapper(JSContext* cx, HandleObject stack) { if (stack) { - MOZ_RELEASE_ASSERT(js::SavedFrame::isSavedFrameOrWrapper(*stack)); + MOZ_RELEASE_ASSERT(stack->canUnwrapAs()); } } diff --git a/js/src/vm/SavedStacks.cpp b/js/src/vm/SavedStacks.cpp index e4c9961ab400..998207f6601d 100644 --- a/js/src/vm/SavedStacks.cpp +++ b/js/src/vm/SavedStacks.cpp @@ -1132,7 +1132,7 @@ JS_PUBLIC_API(bool) IsMaybeWrappedSavedFrame(JSObject* obj) { MOZ_ASSERT(obj); - return js::SavedFrame::isSavedFrameOrWrapper(*obj); + return obj->canUnwrapAs(); } JS_PUBLIC_API(bool) diff --git a/js/src/vm/StructuredClone.cpp b/js/src/vm/StructuredClone.cpp index ab1cef1be546..779528a9c810 100644 --- a/js/src/vm/StructuredClone.cpp +++ b/js/src/vm/StructuredClone.cpp @@ -1827,16 +1827,16 @@ JSStructuredCloneWriter::startWrite(HandleValue v) break; case ESClass::Other: { - if (JS_IsTypedArrayObject(obj)) { + if (obj->canUnwrapAs()) { return writeTypedArray(obj); } - if (JS_IsDataViewObject(obj)) { + if (obj->canUnwrapAs()) { return writeDataView(obj); } if (wasm::IsSharedWasmMemoryObject(obj)) { return writeSharedWasmMemory(obj); } - if (SavedFrame::isSavedFrameOrWrapper(*obj)) { + if (obj->canUnwrapAs()) { return traverseSavedFrame(obj); } break; @@ -2072,7 +2072,7 @@ JSStructuredCloneWriter::write(HandleValue v) if (!startWrite(key) || !startWrite(val)) { return false; } - } else if (cls == ESClass::Set || SavedFrame::isSavedFrameOrWrapper(*obj)) { + } else if (cls == ESClass::Set || obj->canUnwrapAs()) { key = otherEntries.popCopy(); checkStack(); diff --git a/js/src/vm/TypedArrayObject.cpp b/js/src/vm/TypedArrayObject.cpp index 191eebdf23d7..ec003e46f7f9 100644 --- a/js/src/vm/TypedArrayObject.cpp +++ b/js/src/vm/TypedArrayObject.cpp @@ -2269,8 +2269,7 @@ js::DefineTypedArrayElement(JSContext* cx, HandleObject obj, uint64_t index, JS_FRIEND_API(bool) JS_IsTypedArrayObject(JSObject* obj) { - obj = CheckedUnwrap(obj); - return obj ? obj->is() : false; + return obj->canUnwrapAs(); } JS_FRIEND_API(uint32_t)