Attachment 369863 Details for Bug 197884 – Patch

[patch] Patch

bug-197884-20190514095216.patch (text/plain), 7.61 KB, created by Keith Miller on 2019-05-14 09:52:19 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Keith Miller

Created: 2019-05-14 09:52:19 PDT

Size: 7.61 KB

patch

obsolete

>Subversion Revision: 245237
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index eeb5b5c5aba6c329c7d8147131d091051250eac1..7bef75a7b3f88c748cd6388d825b995b15ef2e52 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -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 NOBODY (OOPS!).
>+
>+        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-12  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         [JSC] Compress Watchpoint size by using enum type and Packed<> data structure
>diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>index f50a8ad7b9e64fb02c25c42ecb266587eb6c6449..4aaf6fed54c14927eb0b7b50eafb3169e0337325 100644
>--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>@@ -6798,6 +6798,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);
>@@ -6809,6 +6812,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();
>diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>index 473a038570b6b376440a5e94040603d69d07bd64..36421187d34051f4fafc4e8ecd9c14e1e00bc4f8 100644
>--- a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>+++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>@@ -3920,6 +3920,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));
> 
>@@ -14108,8 +14109,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));
>@@ -14124,6 +14124,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
>@@ -16579,16 +16593,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);
> 
>diff --git a/Source/JavaScriptCore/jit/IntrinsicEmitter.cpp b/Source/JavaScriptCore/jit/IntrinsicEmitter.cpp
>index 06c6127641bf4b688e315e279145278a32384b4d..1a6b432fdbdbff763a136003afd386c564f214ab 100644
>--- a/Source/JavaScriptCore/jit/IntrinsicEmitter.cpp
>+++ b/Source/JavaScriptCore/jit/IntrinsicEmitter.cpp
>@@ -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();
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index 2c83cfcdff0bba9790ae006615a2f673a717ba1f..236e78bb7780f4aae433f710b62c0aecf66de1a6 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -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 NOBODY (OOPS!).
>+
>+        We didn't have any tests that run with non-byte/non-zero offset
>+        typed arrays.
>+
>+        * stress/ftl-gettypedarrayoffset-wasteful.js:
>+
> 2019-05-10  Saam barati  <sbarati@apple.com>
> 
>         Call to JSToWasmICCallee::createStructure passes in wrong prototype value
>diff --git a/JSTests/stress/ftl-gettypedarrayoffset-wasteful.js b/JSTests/stress/ftl-gettypedarrayoffset-wasteful.js
>index 0694e4ccc231965bfbeecac6aac9049be5b0c79e..d551e0f6c50e2b137c6c23b5bd31bd9a2b0483d6 100644
>--- a/JSTests/stress/ftl-gettypedarrayoffset-wasteful.js
>+++ b/JSTests/stress/ftl-gettypedarrayoffset-wasteful.js
>@@ -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();
> }
>

Actions: View | Formatted Diff | Diff

Attachments on bug 197884: 369863