Fix issue with byteOffset on ARM64E
authorkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 May 2019 22:44:26 +0000 (22:44 +0000)
committerkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 May 2019 22:44:26 +0000 (22:44 +0000)
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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@245313 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/stress/ftl-gettypedarrayoffset-wasteful.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
Source/JavaScriptCore/jit/IntrinsicEmitter.cpp

index e5d837e..5b19a0e 100644 (file)
@@ -1,3 +1,15 @@
+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>
 
         [JSC] Shrink sizeof(UnlinkedFunctionExecutable) more
index 0694e4c..d551e0f 100644 (file)
@@ -7,6 +7,12 @@ noInline(foo);
 for (var i = 0; i < 100000; ++i) {
     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();
 }
 
index 59d886f..3128631 100644 (file)
@@ -1,3 +1,25 @@
+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>
 
         REGRESSION (r245249): ASSERTION FAILED: !m_needExceptionCheck seen with stress/proxy-delete.js and stress/proxy-property-descriptor.js
index df0f1c8..debd03e 100644 (file)
@@ -6794,6 +6794,9 @@ void SpeculativeJIT::compileGetTypedArrayByteOffset(Node* node)
         TrustedImm32(WastefulTypedArray));
 
     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);
 
     m_jit.loadPtr(MacroAssembler::Address(baseGPR, JSObject::butterflyOffset()), dataGPR);
@@ -6805,6 +6808,10 @@ void SpeculativeJIT::compileGetTypedArrayByteOffset(Node* node)
     // FIXME: This needs caging.
     // 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);
     
     JITCompiler::Jump done = m_jit.jump();
index 6c65748..5e28a1a 100644 (file)
@@ -3915,6 +3915,7 @@ private:
         // FIXME: This needs caging.
         // 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));
 
@@ -14103,8 +14104,7 @@ private:
 
     LValue untagArrayPtr(LValue ptr, LValue size)
     {
-
-#if !GIGACAGE_ENABLED && CPU(ARM64E)
+#if CPU(ARM64E)
         PatchpointValue* authenticate = m_out.patchpoint(pointerType());
         authenticate->appendSomeRegister(ptr);
         authenticate->append(size, B3::ValueRep(B3::ValueRep::SomeLateRegister));
@@ -14119,6 +14119,20 @@ private:
 #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;
+    }
+
     LValue caged(Gigacage::Kind kind, LValue ptr, LValue base)
     {
 #if GIGACAGE_ENABLED
@@ -16574,16 +16588,9 @@ private:
 
         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);
 
index 06c6127..1a6b432 100644 (file)
@@ -114,11 +114,14 @@ void IntrinsicGetterAccessCase::emitIntrinsicGetter(AccessGenerationState& state
 
         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);
 
         CCallHelpers::Jump done = jit.jump();