Title: [249458] trunk
Revision
249458
Author
mark....@apple.com
Date
2019-09-03 23:13:46 -0700 (Tue, 03 Sep 2019)

Log 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/_javascript_Core:

* 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):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (249457 => 249458)


--- trunk/JSTests/ChangeLog	2019-09-04 06:13:40 UTC (rev 249457)
+++ trunk/JSTests/ChangeLog	2019-09-04 06:13:46 UTC (rev 249458)
@@ -1,3 +1,13 @@
+2019-09-03  Mark Lam  <mark....@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  <ysuz...@apple.com>
 
         [JSC] Generate new.target register only when it is used

Added: trunk/JSTests/stress/JSArrayBufferView-byteOffset-is-racy-from-compiler-thread.js (0 => 249458)


--- trunk/JSTests/stress/JSArrayBufferView-byteOffset-is-racy-from-compiler-thread.js	                        (rev 0)
+++ trunk/JSTests/stress/JSArrayBufferView-byteOffset-is-racy-from-compiler-thread.js	2019-09-04 06:13:46 UTC (rev 249458)
@@ -0,0 +1,22 @@
+//@ skip if ["arm", "mips"].include?($architecture)
+//@ slow!
+//@ runDefault("--jitPolicyScale=0")
+
+// This test should not crash.
+
+script = `
+    let a = new Int32Array(1);
+    for (let i = 0; i < 1000; ++i)
+        ~a.byteOffset;
+
+    transferArrayBuffer(a.buffer);
+
+    eval(a.byteOffset);
+    let description = describe(a.byteOffset);
+    if (description !== 'Int32: 0')
+        print(description);
+`;
+
+const iterations = 1000;
+for (let i = 0; i < iterations; i++)
+    runString(script);

Modified: trunk/Source/_javascript_Core/ChangeLog (249457 => 249458)


--- trunk/Source/_javascript_Core/ChangeLog	2019-09-04 06:13:40 UTC (rev 249457)
+++ trunk/Source/_javascript_Core/ChangeLog	2019-09-04 06:13:46 UTC (rev 249458)
@@ -1,3 +1,21 @@
+2019-09-03  Mark Lam  <mark....@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  <drou...@apple.com>
 
         Web Inspector: implement blackboxing of script resources

Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (249457 => 249458)


--- trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h	2019-09-04 06:13:40 UTC (rev 249457)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h	2019-09-04 06:13:46 UTC (rev 249458)
@@ -3257,8 +3257,11 @@
     case GetTypedArrayByteOffset: {
         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);
         break;

Modified: trunk/Source/_javascript_Core/runtime/JSArrayBufferView.h (249457 => 249458)


--- trunk/Source/_javascript_Core/runtime/JSArrayBufferView.h	2019-09-04 06:13:40 UTC (rev 249457)
+++ trunk/Source/_javascript_Core/runtime/JSArrayBufferView.h	2019-09-04 06:13:46 UTC (rev 249458)
@@ -162,7 +162,7 @@
     
     bool isShared();
     JS_EXPORT_PRIVATE ArrayBuffer* unsharedBuffer();
-    ArrayBuffer* possiblySharedBuffer();
+    inline ArrayBuffer* possiblySharedBuffer();
     JSArrayBuffer* unsharedJSBuffer(ExecState* exec);
     JSArrayBuffer* possiblySharedJSBuffer(ExecState* exec);
     RefPtr<ArrayBufferView> unsharedImpl();
@@ -173,7 +173,9 @@
     bool hasVector() const { return !!m_vector; }
     void* vector() const { return m_vector.getMayBeNull(length()); }
     
-    unsigned byteOffset();
+    inline unsigned byteOffset();
+    inline Optional<unsigned> byteOffsetConcurrently();
+
     unsigned length() const { return m_length; }
 
     DECLARE_EXPORT_INFO;
@@ -185,6 +187,10 @@
     static RefPtr<ArrayBufferView> toWrapped(VM&, JSValue);
 
 private:
+    enum Requester { Mutator, ConcurrentThread };
+    template<Requester, typename ResultType> ResultType byteOffsetImpl();
+    template<Requester> ArrayBuffer* possiblySharedBufferImpl();
+
     JS_EXPORT_PRIVATE ArrayBuffer* slowDownAndWasteMemory();
     static void finalize(JSCell*);
 

Modified: trunk/Source/_javascript_Core/runtime/JSArrayBufferViewInlines.h (249457 => 249458)


--- trunk/Source/_javascript_Core/runtime/JSArrayBufferViewInlines.h	2019-09-04 06:13:40 UTC (rev 249457)
+++ trunk/Source/_javascript_Core/runtime/JSArrayBufferViewInlines.h	2019-09-04 06:13:46 UTC (rev 249458)
@@ -43,8 +43,12 @@
     }
 }
 
-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:
         return existingBufferInButterfly();
@@ -58,6 +62,11 @@
     return nullptr;
 }
 
+inline ArrayBuffer* JSArrayBufferView::possiblySharedBuffer()
+{
+    return possiblySharedBufferImpl<Mutator>();
+}
+
 inline ArrayBuffer* JSArrayBufferView::existingBufferInButterfly()
 {
     ASSERT(m_mode == WastefulTypedArray);
@@ -71,22 +80,45 @@
     return result;
 }
 
-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>>();
+}
+
 inline RefPtr<ArrayBufferView> JSArrayBufferView::toWrapped(VM& vm, JSValue value)
 {
     if (JSArrayBufferView* view = jsDynamicCast<JSArrayBufferView*>(vm, value)) {
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to