Changeset 208481 in webkit

Timestamp:

Nov 9, 2016 1:33:23 PM (7 years ago)

Author:

Yusuke Suzuki

Message:

[DOMJIT] Implement Node::ownerDocument
​https://bugs.webkit.org/show_bug.cgi?id=164004

Reviewed by Darin Adler.

Source/WebCore:

Test: js/dom/domjit-accessor-owner-document.html

Still I cannot reproduce this crash in x64 environment, according to the crash log, it accesses 0x8 address.
This can happen if document() accidentally returns nullptr. In the C++ ownerDocument implementation,
if document() returns nullptr, it just returns nullptr. But in the DOMJIT implementation, we assume that
document() won't return nullptr and access the member of it.

This patch aligns the DOMJIT implementation strictly to the C++ one.

• dom/Node.idl:
• domjit/JSNodeDOMJIT.cpp:

(WebCore::NodeOwnerDocumentDOMJIT::checkDOM):
(WebCore::NodeOwnerDocumentDOMJIT::callDOMGetter):

LayoutTests:

• js/dom/domjit-accessor-owner-document-expected.txt: Added.
• js/dom/domjit-accessor-owner-document.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/js/dom/domjit-accessor-owner-document-expected.txt (added)
• LayoutTests/js/dom/domjit-accessor-owner-document.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/Node.idl (modified) (1 diff)
• Source/WebCore/domjit/JSNodeDOMJIT.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-11-09  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [DOMJIT] Implement Node::ownerDocument
+        https://bugs.webkit.org/show_bug.cgi?id=164004
+
+        Reviewed by Darin Adler.
+
+        * js/dom/domjit-accessor-owner-document-expected.txt: Added.
+        * js/dom/domjit-accessor-owner-document.html: Added.
+
 2016-11-09  Dave Hyatt  <hyatt@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-11-09  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [DOMJIT] Implement Node::ownerDocument
+        https://bugs.webkit.org/show_bug.cgi?id=164004
+
+        Reviewed by Darin Adler.
+
+        Test: js/dom/domjit-accessor-owner-document.html
+
+        Still I cannot reproduce this crash in x64 environment, according to the crash log, it accesses 0x8 address.
+        This can happen if document() accidentally returns nullptr. In the C++ ownerDocument implementation,
+        if document() returns nullptr, it just returns nullptr. But in the DOMJIT implementation, we assume that
+        document() won't return nullptr and access the member of it.
+
+        This patch aligns the DOMJIT implementation strictly to the C++ one.
+
+        * dom/Node.idl:
+        * domjit/JSNodeDOMJIT.cpp:
+        (WebCore::NodeOwnerDocumentDOMJIT::checkDOM):
+        (WebCore::NodeOwnerDocumentDOMJIT::callDOMGetter):
+
 2016-11-09  Sam Weinig  <sam@webkit.org>
 

trunk/Source/WebCore/dom/Node.idl

@@ -54 +54 @@
     [DOMJIT] readonly attribute Node? previousSibling;
     [DOMJIT] readonly attribute Node? nextSibling;
-    readonly attribute Document? ownerDocument;
+    [DOMJIT] readonly attribute Document? ownerDocument;
 
     [CEReactions, Custom, MayThrowLegacyException] Node insertBefore(Node newChild, Node? refChild);

trunk/Source/WebCore/domjit/JSNodeDOMJIT.cpp

@@ -158 +158 @@
 }
 
+Ref<JSC::DOMJIT::Patchpoint> NodeOwnerDocumentDOMJIT::checkDOM()
+{
+    return DOMJIT::checkDOM<Node>();
+}
+
+Ref<JSC::DOMJIT::CallDOMGetterPatchpoint> NodeOwnerDocumentDOMJIT::callDOMGetter()
+{
+    Ref<JSC::DOMJIT::CallDOMGetterPatchpoint> patchpoint = JSC::DOMJIT::CallDOMGetterPatchpoint::create();
+    patchpoint->numGPScratchRegisters = 2;
+    patchpoint->setGenerator([=](CCallHelpers& jit, JSC::DOMJIT::PatchpointParams& params) {
+        JSValueRegs result = params[0].jsValueRegs();
+        GPRReg node = params[1].gpr();
+        GPRReg globalObject = params[2].gpr();
+        JSValue globalObjectValue = params[2].value();
+        GPRReg wrapped = params.gpScratch(0);
+        GPRReg document = params.gpScratch(1);
+
+        jit.loadPtr(CCallHelpers::Address(node, JSNode::offsetOfWrapped()), wrapped);
+        DOMJIT::loadDocument(jit, wrapped, document);
+        RELEASE_ASSERT(!CAST_OFFSET(EventTarget*, Node*));
+        RELEASE_ASSERT(!CAST_OFFSET(Node*, Document*));
+
+        CCallHelpers::JumpList nullCases;
+        // If the |this| is the document itself, ownerDocument will return null.
+        nullCases.append(jit.branchPtr(CCallHelpers::Equal, wrapped, document));
+        DOMJIT::toWrapper<Document>(jit, params, document, globalObject, result, DOMJIT::toWrapperSlow<Document>, globalObjectValue);
+        auto done = jit.jump();
+
+        nullCases.link(&jit);
+        jit.moveValue(jsNull(), result);
+        done.link(&jit);
+        return CCallHelpers::JumpList();
+    });
+    patchpoint->effect = JSC::DOMJIT::Effect::forDef(DOMJIT::AbstractHeapRepository::Node_ownerDocument);
+    return patchpoint;
+}
+
 }