Changeset 249458 in webkit

Timestamp:

Sep 3, 2019, 11:13:46 PM (6 years ago)

Author:

mark.lam@apple.com

Message:

Assertions in JSArrayBufferView::byteOffset() are only valid for the mutator thread.
​https://bugs.webkit.org/show_bug.cgi?id=201309
<rdar://problem/54832121>

Reviewed by Yusuke Suzuki.

JSTests:

• stress/JSArrayBufferView-byteOffset-is-racy-from-compiler-thread.js: Added.

Source/JavaScriptCore:

• dfg/DFGAbstractInterpreterInlines.h:

(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):

• runtime/JSArrayBufferView.h:
• runtime/JSArrayBufferViewInlines.h:

(JSC::JSArrayBufferView::possiblySharedBufferImpl):
(JSC::JSArrayBufferView::possiblySharedBuffer):
(JSC::JSArrayBufferView::byteOffsetImpl):
(JSC::JSArrayBufferView::byteOffset):
(JSC::JSArrayBufferView::byteOffsetConcurrently):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/JSArrayBufferView-byteOffset-is-racy-from-compiler-thread.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h (modified) (1 diff)
• Source/JavaScriptCore/runtime/JSArrayBufferView.h (modified) (3 diffs)
• Source/JavaScriptCore/runtime/JSArrayBufferViewInlines.h (modified) (3 diffs)

trunk/JSTests/ChangeLog

@@ +1 @@
+2019-09-03  Mark Lam  <mark.lam@apple.com>
+
+        Assertions in JSArrayBufferView::byteOffset() are only valid for the mutator thread.
+        https://bugs.webkit.org/show_bug.cgi?id=201309
+        <rdar://problem/54832121>
+
+        Reviewed by Yusuke Suzuki.
+
+        * stress/JSArrayBufferView-byteOffset-is-racy-from-compiler-thread.js: Added.
+
 2019-08-30  Yusuke Suzuki  <ysuzuki@apple.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-09-03  Mark Lam  <mark.lam@apple.com>
+
+        Assertions in JSArrayBufferView::byteOffset() are only valid for the mutator thread.
+        https://bugs.webkit.org/show_bug.cgi?id=201309
+        <rdar://problem/54832121>
+
+        Reviewed by Yusuke Suzuki.
+
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+        * runtime/JSArrayBufferView.h:
+        * runtime/JSArrayBufferViewInlines.h:
+        (JSC::JSArrayBufferView::possiblySharedBufferImpl):
+        (JSC::JSArrayBufferView::possiblySharedBuffer):
+        (JSC::JSArrayBufferView::byteOffsetImpl):
+        (JSC::JSArrayBufferView::byteOffset):
+        (JSC::JSArrayBufferView::byteOffsetConcurrently):
+
 2019-09-03  Devin Rousso  <drousso@apple.com>
 

trunk/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h

@@ -3258 +3258 @@
         JSArrayBufferView* view = m_graph.tryGetFoldableView(forNode(node->child1()).m_value);
         if (view) {
-            setConstant(node, jsNumber(view->byteOffset()));
-            break;
+            Optional<unsigned> byteOffset = view->byteOffsetConcurrently();
+            if (byteOffset) {
+                setConstant(node, jsNumber(*byteOffset));
+                break;
+            }
         }
         setNonCellTypeForNode(node, SpecInt32Only);

trunk/Source/JavaScriptCore/runtime/JSArrayBufferView.h

@@ -163 +163 @@
     bool isShared();
     JS_EXPORT_PRIVATE ArrayBuffer* unsharedBuffer();
-    ArrayBuffer* possiblySharedBuffer();
+    inline ArrayBuffer* possiblySharedBuffer();
     JSArrayBuffer* unsharedJSBuffer(ExecState* exec);
     JSArrayBuffer* possiblySharedJSBuffer(ExecState* exec);
@@ -174 +174 @@
     void* vector() const { return m_vector.getMayBeNull(length()); }
     
-    unsigned byteOffset();
+    inline unsigned byteOffset();
+    inline Optional<unsigned> byteOffsetConcurrently();
+
     unsigned length() const { return m_length; }
 
@@ -186 +188 @@
 
 private:
+    enum Requester { Mutator, ConcurrentThread };
+    template<Requester, typename ResultType> ResultType byteOffsetImpl();
+    template<Requester> ArrayBuffer* possiblySharedBufferImpl();
+
     JS_EXPORT_PRIVATE ArrayBuffer* slowDownAndWasteMemory();
     static void finalize(JSCell*);

trunk/Source/JavaScriptCore/runtime/JSArrayBufferViewInlines.h

@@ -44 +44 @@
 }
 
-inline ArrayBuffer* JSArrayBufferView::possiblySharedBuffer()
+template<JSArrayBufferView::Requester requester>
+inline ArrayBuffer* JSArrayBufferView::possiblySharedBufferImpl()
 {
+    if (requester == ConcurrentThread)
+        ASSERT(m_mode != FastTypedArray && m_mode != OversizeTypedArray);
+
     switch (m_mode) {
     case WastefulTypedArray:
@@ -57 +61 @@
     ASSERT_NOT_REACHED();
     return nullptr;
+}
+
+inline ArrayBuffer* JSArrayBufferView::possiblySharedBuffer()
+{
+    return possiblySharedBufferImpl<Mutator>();
 }
 
@@ -72 +81 @@
 }
 
-inline unsigned JSArrayBufferView::byteOffset()
+template<JSArrayBufferView::Requester requester, typename ResultType>
+inline ResultType JSArrayBufferView::byteOffsetImpl()
 {
     if (!hasArrayBuffer())
         return 0;
-    
-    ArrayBuffer* buffer = possiblySharedBuffer();
-    ASSERT(!vector() == !buffer->data());
-    
+
+    if (requester == ConcurrentThread)
+        WTF::loadLoadFence();
+
+    ArrayBuffer* buffer = possiblySharedBufferImpl<requester>();
+    if (requester == Mutator) {
+        ASSERT(!isCompilationThread());
+        ASSERT(!vector() == !buffer->data());
+    }
+
     ptrdiff_t delta =
         bitwise_cast<uint8_t*>(vector()) - static_cast<uint8_t*>(buffer->data());
-    
+
     unsigned result = static_cast<unsigned>(delta);
-    ASSERT(static_cast<ptrdiff_t>(result) == delta);
+    if (requester == Mutator)
+        ASSERT(static_cast<ptrdiff_t>(result) == delta);
+    else {
+        if (static_cast<ptrdiff_t>(result) != delta)
+            return { };
+    }
+
     return result;
+}
+
+inline unsigned JSArrayBufferView::byteOffset()
+{
+    return byteOffsetImpl<Mutator, unsigned>();
+}
+
+inline Optional<unsigned> JSArrayBufferView::byteOffsetConcurrently()
+{
+    return byteOffsetImpl<ConcurrentThread, Optional<unsigned>>();
 }