Changeset 245313 in webkit

Timestamp:

May 14, 2019 3:44:26 PM (5 years ago)

Author:

keith_miller@apple.com

Message:

Fix issue with byteOffset on ARM64E
​https://bugs.webkit.org/show_bug.cgi?id=197884

Reviewed by Saam Barati.

JSTests:

We didn't have any tests that run with non-byte/non-zero offset
typed arrays.

• stress/ftl-gettypedarrayoffset-wasteful.js:

Source/JavaScriptCore:

We forgot to remove the tag from the ArrayBuffer's data
pointer. This corrupted data when computing the offset. We didn't
catch this because we didn't run any with a non-zero byteOffset in
the JITs.

• dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::compileGetTypedArrayByteOffset):

• ftl/FTLLowerDFGToB3.cpp:

(JSC::FTL::DFG::LowerDFGToB3::compileGetTypedArrayByteOffset):
(JSC::FTL::DFG::LowerDFGToB3::untagArrayPtr):
(JSC::FTL::DFG::LowerDFGToB3::removeArrayPtrTag):
(JSC::FTL::DFG::LowerDFGToB3::speculateTypedArrayIsNotNeutered):

• jit/IntrinsicEmitter.cpp:

(JSC::IntrinsicGetterAccessCase::emitIntrinsicGetter):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/ftl-gettypedarrayoffset-wasteful.js (modified) (1 diff)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp (modified) (2 diffs)
• Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp (modified) (4 diffs)
• Source/JavaScriptCore/jit/IntrinsicEmitter.cpp (modified) (1 diff)

trunk/JSTests/ChangeLog

@@ +1 @@
+2019-05-14  Keith Miller  <keith_miller@apple.com>
+
+        Fix issue with byteOffset on ARM64E
+        https://bugs.webkit.org/show_bug.cgi?id=197884
+
+        Reviewed by Saam Barati.
+
+        We didn't have any tests that run with non-byte/non-zero offset
+        typed arrays.
+
+        * stress/ftl-gettypedarrayoffset-wasteful.js:
+
 2019-05-14  Yusuke Suzuki  <ysuzuki@apple.com>
 

trunk/JSTests/stress/ftl-gettypedarrayoffset-wasteful.js

@@ -8 +8 @@
     var b = new Uint8Array(new ArrayBuffer(42), 0);
     if (foo(b) != 0) 
-        throw "error"
+        throw new Error();
+    b = new Uint8Array(new ArrayBuffer(42), 5);
+    if (foo(b) !== 5)
+        throw new Error();
+    b = new Int32Array(new ArrayBuffer(100000 * 4), i * 4);
+    if (foo(b) !== i * 4)
+        throw new Error();
 }
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-05-14  Keith Miller  <keith_miller@apple.com>
+
+        Fix issue with byteOffset on ARM64E
+        https://bugs.webkit.org/show_bug.cgi?id=197884
+
+        Reviewed by Saam Barati.
+
+        We forgot to remove the tag from the ArrayBuffer's data
+        pointer. This corrupted data when computing the offset.  We didn't
+        catch this because we didn't run any with a non-zero byteOffset in
+        the JITs.
+
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileGetTypedArrayByteOffset):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileGetTypedArrayByteOffset):
+        (JSC::FTL::DFG::LowerDFGToB3::untagArrayPtr):
+        (JSC::FTL::DFG::LowerDFGToB3::removeArrayPtrTag):
+        (JSC::FTL::DFG::LowerDFGToB3::speculateTypedArrayIsNotNeutered):
+        * jit/IntrinsicEmitter.cpp:
+        (JSC::IntrinsicGetterAccessCase::emitIntrinsicGetter):
+
 2019-05-14  Tadeu Zagallo  <tzagallo@apple.com>
 

trunk/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp

@@ -6795 +6795 @@
 
     m_jit.loadPtr(MacroAssembler::Address(baseGPR, JSArrayBufferView::offsetOfVector()), vectorGPR);
+
+    // FIXME: This should mask the PAC bits
+    // https://bugs.webkit.org/show_bug.cgi?id=197701
     JITCompiler::Jump nullVector = m_jit.branchTestPtr(JITCompiler::Zero, vectorGPR);
 
@@ -6806 +6809 @@
     // https://bugs.webkit.org/show_bug.cgi?id=175515
     m_jit.loadPtr(MacroAssembler::Address(arrayBufferGPR, ArrayBuffer::offsetOfData()), dataGPR);
+#if CPU(ARM64E)
+    m_jit.removeArrayPtrTag(dataGPR);
+#endif
+
     m_jit.subPtr(dataGPR, vectorGPR);
     

trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

@@ -3916 +3916 @@
         // https://bugs.webkit.org/show_bug.cgi?id=175515
         LValue dataPtr = m_out.loadPtr(arrayBufferPtr, m_heaps.ArrayBuffer_data);
+        dataPtr = removeArrayPtrTag(dataPtr);
 
         ValueFromBlock wastefulOut = m_out.anchor(m_out.sub(vectorPtr, dataPtr));
@@ -14104 +14105 @@
     LValue untagArrayPtr(LValue ptr, LValue size)
     {
-
-#if !GIGACAGE_ENABLED && CPU(ARM64E)
+#if CPU(ARM64E)
         PatchpointValue* authenticate = m_out.patchpoint(pointerType());
         authenticate->appendSomeRegister(ptr);
@@ -14118 +14118 @@
         return ptr;
 #endif
+    }
+
+    LValue removeArrayPtrTag(LValue ptr)
+    {
+#if CPU(ARM64E)
+        PatchpointValue* authenticate = m_out.patchpoint(pointerType());
+        authenticate->appendSomeRegister(ptr);
+        authenticate->setGenerator([=] (CCallHelpers& jit, const StackmapGenerationParams& params) {
+            jit.move(params[1].gpr(), params[0].gpr());
+            jit.removeArrayPtrTag(params[0].gpr());
+        });
+        return authenticate;
+#endif
+        return ptr;
     }
 
@@ -16575 +16589 @@
         LBasicBlock lastNext = m_out.appendTo(isWasteful, continuation);
         LValue vector = m_out.loadPtr(base, m_heaps.JSArrayBufferView_vector);
-#if !GIGACAGE_ENABLED && CPU(ARM64E)
-        // FIXME: We could probably make this a mask. https://bugs.webkit.org/show_bug.cgi?id=197701
-        PatchpointValue* authenticate = m_out.patchpoint(pointerType());
-        authenticate->appendSomeRegister(vector);
-        authenticate->setGenerator([=] (CCallHelpers& jit, const StackmapGenerationParams& params) {
-            jit.move(params[1].gpr(), params[0].gpr());
-            jit.removeArrayPtrTag(params[0].gpr());
-        });
-        vector = authenticate;
-#endif
+        // FIXME: We could probably make this a mask.
+        // https://bugs.webkit.org/show_bug.cgi?id=197701
+        vector = removeArrayPtrTag(vector);
         speculate(Uncountable, jsValueValue(vector), m_node, m_out.isZero64(vector));
         m_out.jump(continuation);

trunk/Source/JavaScriptCore/jit/IntrinsicEmitter.cpp

@@ -115 +115 @@
         jit.loadPtr(MacroAssembler::Address(baseGPR, JSObject::butterflyOffset()), scratchGPR);
         jit.loadPtr(MacroAssembler::Address(baseGPR, JSArrayBufferView::offsetOfVector()), valueGPR);
-#if !GIGACAGE_ENABLED && CPU(ARM64E)
+#if CPU(ARM64E)
         jit.removeArrayPtrTag(valueGPR);
 #endif
         jit.loadPtr(MacroAssembler::Address(scratchGPR, Butterfly::offsetOfArrayBuffer()), scratchGPR);
         jit.loadPtr(MacroAssembler::Address(scratchGPR, ArrayBuffer::offsetOfData()), scratchGPR);
+#if CPU(ARM64E)
+        jit.removeArrayPtrTag(scratchGPR);
+#endif
         jit.subPtr(scratchGPR, valueGPR);