Title: [275392] trunk/Source/_javascript_Core
Revision
275392
Author
[email protected]
Date
2021-04-01 16:06:51 -0700 (Thu, 01 Apr 2021)

Log Message

REGRESSION(r274724): JITCage trampoline needs to be adjusted
https://bugs.webkit.org/show_bug.cgi?id=224065

Reviewed by Saam Barati.

r274724 introduced a new parameter to custom setters, but it didn't change the parameter recognization of JITCage trampolines for custom accessors.
As a result, we are jumping with the wrong pointer, and crash when custom setter is called with JITCage.

This patch fixes the above bug.

1. Now, custom getter and custom setter have different number of parameters. We should have two different trampolines to invoke it. We remove vmEntryCustomAccessor, and
   add vmEntryCustomGetter/vmEntryCustomSetter.
2. vmEntryCustomSetter should use a4 parameter as a executable address for trampoline.

* bytecode/AccessCase.cpp:
(JSC::AccessCase::generateImpl):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileCallDOMGetter):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileCallDOMGetter):
* llint/LLIntThunks.cpp:
* llint/LLIntThunks.h:
* llint/LowLevelInterpreter.asm:
* offlineasm/arm64.rb:
* offlineasm/registers.rb:
* runtime/PropertySlot.h:

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (275391 => 275392)


--- trunk/Source/_javascript_Core/ChangeLog	2021-04-01 22:50:07 UTC (rev 275391)
+++ trunk/Source/_javascript_Core/ChangeLog	2021-04-01 23:06:51 UTC (rev 275392)
@@ -1,3 +1,32 @@
+2021-04-01  Yusuke Suzuki  <[email protected]>
+
+        REGRESSION(r274724): JITCage trampoline needs to be adjusted
+        https://bugs.webkit.org/show_bug.cgi?id=224065
+
+        Reviewed by Saam Barati.
+
+        r274724 introduced a new parameter to custom setters, but it didn't change the parameter recognization of JITCage trampolines for custom accessors.
+        As a result, we are jumping with the wrong pointer, and crash when custom setter is called with JITCage.
+
+        This patch fixes the above bug.
+
+        1. Now, custom getter and custom setter have different number of parameters. We should have two different trampolines to invoke it. We remove vmEntryCustomAccessor, and
+           add vmEntryCustomGetter/vmEntryCustomSetter.
+        2. vmEntryCustomSetter should use a4 parameter as a executable address for trampoline.
+
+        * bytecode/AccessCase.cpp:
+        (JSC::AccessCase::generateImpl):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileCallDOMGetter):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileCallDOMGetter):
+        * llint/LLIntThunks.cpp:
+        * llint/LLIntThunks.h:
+        * llint/LowLevelInterpreter.asm:
+        * offlineasm/arm64.rb:
+        * offlineasm/registers.rb:
+        * runtime/PropertySlot.h:
+
 2021-04-01  Ross Kirsling  <[email protected]>
 
         [JSC] Use ucal_getTimeZoneOffsetFromLocal if ICU 69 is present

Modified: trunk/Source/_javascript_Core/bytecode/AccessCase.cpp (275391 => 275392)


--- trunk/Source/_javascript_Core/bytecode/AccessCase.cpp	2021-04-01 22:50:07 UTC (rev 275391)
+++ trunk/Source/_javascript_Core/bytecode/AccessCase.cpp	2021-04-01 23:06:51 UTC (rev 275392)
@@ -1800,9 +1800,12 @@
             else
                 operationCall = jit.call(CustomAccessorPtrTag);
             jit.addLinkTask([=] (LinkBuffer& linkBuffer) {
-                if (Options::useJITCage())
-                    linkBuffer.link(operationCall, FunctionPtr<OperationPtrTag>(vmEntryCustomAccessor));
-                else
+                if (Options::useJITCage()) {
+                    if (m_type == CustomValueGetter || m_type == CustomAccessorGetter)
+                        linkBuffer.link(operationCall, FunctionPtr<OperationPtrTag>(vmEntryCustomGetter));
+                    else
+                        linkBuffer.link(operationCall, FunctionPtr<OperationPtrTag>(vmEntryCustomSetter));
+                } else
                     linkBuffer.link(operationCall, this->as<GetterSetterAccessCase>().m_customAccessor);
             });
 

Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp (275391 => 275392)


--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2021-04-01 22:50:07 UTC (rev 275391)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT.cpp	2021-04-01 23:06:51 UTC (rev 275392)
@@ -10212,7 +10212,7 @@
         m_jit.storePtr(GPRInfo::callFrameRegister, &vm().topCallFrame);
         m_jit.emitStoreCodeOrigin(m_currentNode->origin.semantic);
         if (Options::useJITCage())
-            m_jit.appendCall(vmEntryCustomAccessor);
+            m_jit.appendCall(vmEntryCustomGetter);
         else {
             FunctionPtr<OperationPtrTag> bypassedFunction = FunctionPtr<OperationPtrTag>(MacroAssemblerCodePtr<OperationPtrTag>(WTF::tagNativeCodePtrImpl<OperationPtrTag>(WTF::untagNativeCodePtrImpl<CustomAccessorPtrTag>(getter.executableAddress()))));
             m_jit.appendOperationCall(bypassedFunction);

Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (275391 => 275392)


--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2021-04-01 22:50:07 UTC (rev 275391)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp	2021-04-01 23:06:51 UTC (rev 275392)
@@ -14635,7 +14635,7 @@
             m_out.storePtr(m_callFrame, m_out.absolute(&vm().topCallFrame));
             if (Options::useJITCage()) {
                 setJSValue(
-                    vmCall(Int64, vmEntryCustomAccessor, weakPointer(globalObject), lowCell(m_node->child1()), m_out.constIntPtr(m_graph.identifiers()[m_node->callDOMGetterData()->identifierNumber]), m_out.constIntPtr(m_node->callDOMGetterData()->customAccessorGetter.executableAddress())));
+                    vmCall(Int64, vmEntryCustomGetter, weakPointer(globalObject), lowCell(m_node->child1()), m_out.constIntPtr(m_graph.identifiers()[m_node->callDOMGetterData()->identifierNumber]), m_out.constIntPtr(m_node->callDOMGetterData()->customAccessorGetter.executableAddress())));
             } else {
                 FunctionPtr<CustomAccessorPtrTag> getter = m_node->callDOMGetterData()->customAccessorGetter;
                 FunctionPtr<OperationPtrTag> bypassedFunction = FunctionPtr<OperationPtrTag>(MacroAssemblerCodePtr<OperationPtrTag>(WTF::tagNativeCodePtrImpl<OperationPtrTag>(WTF::untagNativeCodePtrImpl<CustomAccessorPtrTag>(getter.executableAddress()))));

Modified: trunk/Source/_javascript_Core/llint/LLIntThunks.cpp (275391 => 275392)


--- trunk/Source/_javascript_Core/llint/LLIntThunks.cpp	2021-04-01 22:50:07 UTC (rev 275391)
+++ trunk/Source/_javascript_Core/llint/LLIntThunks.cpp	2021-04-01 23:06:51 UTC (rev 275392)
@@ -57,9 +57,13 @@
 JSC_ANNOTATE_LLINT_JIT_OPERATION(llint_function_for_call_arity_checkTagGateAfter);
 JSC_ANNOTATE_LLINT_JIT_OPERATION(llint_function_for_construct_arity_checkUntagGateAfter);
 JSC_ANNOTATE_LLINT_JIT_OPERATION(llint_function_for_construct_arity_checkTagGateAfter);
-JSC_ANNOTATE_LLINT_JIT_OPERATION(vmEntryCustomAccessor);
+JSC_ANNOTATE_LLINT_JIT_OPERATION(vmEntryCustomGetter);
+JSC_ANNOTATE_LLINT_JIT_OPERATION(vmEntryCustomSetter);
 JSC_ANNOTATE_LLINT_JIT_OPERATION(vmEntryHostFunction);
 
+static_assert(FunctionTraits<decltype(vmEntryCustomGetter)>::arity == FunctionTraits<GetValueFuncWithPtr>::arity, "When changing GetValueFuncWithPtr, need to change vmEntryCustomGetter implementation too.");
+static_assert(FunctionTraits<decltype(vmEntryCustomSetter)>::arity == FunctionTraits<PutValueFuncWithPtr>::arity, "When changing PutValueFuncWithPtr, need to change vmEntryCustomSetter implementation too.");
+
 #endif
 
 namespace LLInt {

Modified: trunk/Source/_javascript_Core/llint/LLIntThunks.h (275391 => 275392)


--- trunk/Source/_javascript_Core/llint/LLIntThunks.h	2021-04-01 22:50:07 UTC (rev 275391)
+++ trunk/Source/_javascript_Core/llint/LLIntThunks.h	2021-04-01 23:06:51 UTC (rev 275392)
@@ -38,7 +38,8 @@
 extern "C" {
     EncodedJSValue vmEntryToJavaScript(void*, VM*, ProtoCallFrame*);
     EncodedJSValue vmEntryToNative(void*, VM*, ProtoCallFrame*);
-    EncodedJSValue vmEntryCustomAccessor(CPURegister, CPURegister, CPURegister, CPURegister);
+    EncodedJSValue vmEntryCustomGetter(CPURegister, CPURegister, CPURegister, CPURegister);
+    EncodedJSValue vmEntryCustomSetter(CPURegister, CPURegister, CPURegister, CPURegister, CPURegister);
     EncodedJSValue vmEntryHostFunction(JSGlobalObject*, CallFrame*, void*);
 }
 

Modified: trunk/Source/_javascript_Core/llint/LowLevelInterpreter.asm (275391 => 275392)


--- trunk/Source/_javascript_Core/llint/LowLevelInterpreter.asm	2021-04-01 22:50:07 UTC (rev 275391)
+++ trunk/Source/_javascript_Core/llint/LowLevelInterpreter.asm	2021-04-01 23:06:51 UTC (rev 275392)
@@ -1624,10 +1624,25 @@
     ret
 
 # a0, a1, a2 are used. a3 contains function address.
-global _vmEntryCustomAccessor
-_vmEntryCustomAccessor:
+# EncodedJSValue(JIT_OPERATION_ATTRIBUTES*)(JSGlobalObject*, EncodedJSValue, PropertyName, void*);
+global _vmEntryCustomGetter
+_vmEntryCustomGetter:
+if ARM64E
     jmp a3, CustomAccessorPtrTag
+else
+    crash()
+end
 
+# a0, a1, a2, a3 are used. a4 contains function address.
+# bool (JIT_OPERATION_ATTRIBUTES*)(JSGlobalObject*, EncodedJSValue, EncodedJSValue, PropertyName, void*);
+global _vmEntryCustomSetter
+_vmEntryCustomSetter:
+if ARM64E
+    jmp a4, CustomAccessorPtrTag
+else
+    crash()
+end
+
 # a0 and a1 are used. a2 contains function address.
 global _vmEntryHostFunction
 _vmEntryHostFunction:

Modified: trunk/Source/_javascript_Core/offlineasm/arm64.rb (275391 => 275392)


--- trunk/Source/_javascript_Core/offlineasm/arm64.rb	2021-04-01 22:50:07 UTC (rev 275391)
+++ trunk/Source/_javascript_Core/offlineasm/arm64.rb	2021-04-01 23:06:51 UTC (rev 275392)
@@ -135,13 +135,13 @@
             arm64GPRName('x2', kind)
         when 't3', 'a3', 'wa3'
             arm64GPRName('x3', kind)
-        when 't4', 'wa4'
+        when 't4', 'a4', 'wa4'
             arm64GPRName('x4', kind)
-        when 't5', 'wa5'
+        when 't5', 'a5', 'wa5'
           arm64GPRName('x5', kind)
-        when 't6', 'wa6'
+        when 't6', 'a6', 'wa6'
           arm64GPRName('x6', kind)
-        when 't7', 'wa7'
+        when 't7', 'a7', 'wa7'
           arm64GPRName('x7', kind)
         when 'ws0'
           arm64GPRName('x9', kind)

Modified: trunk/Source/_javascript_Core/offlineasm/registers.rb (275391 => 275392)


--- trunk/Source/_javascript_Core/offlineasm/registers.rb	2021-04-01 22:50:07 UTC (rev 275391)
+++ trunk/Source/_javascript_Core/offlineasm/registers.rb	2021-04-01 23:06:51 UTC (rev 275392)
@@ -38,6 +38,10 @@
      "a1",
      "a2",
      "a3",
+     "a4",
+     "a5",
+     "a6",
+     "a7",
      "r0",
      "r1",
      "sp",

Modified: trunk/Source/_javascript_Core/runtime/PropertySlot.h (275391 => 275392)


--- trunk/Source/_javascript_Core/runtime/PropertySlot.h	2021-04-01 22:50:07 UTC (rev 275391)
+++ trunk/Source/_javascript_Core/runtime/PropertySlot.h	2021-04-01 23:06:51 UTC (rev 275392)
@@ -87,8 +87,8 @@
 using GetValueFunc = EncodedJSValue(JIT_OPERATION_ATTRIBUTES*)(JSGlobalObject*, EncodedJSValue thisValue, PropertyName);
 using GetValueFuncWithPtr = EncodedJSValue(JIT_OPERATION_ATTRIBUTES*)(JSGlobalObject*, EncodedJSValue thisValue, PropertyName, void*);
 
-using PutValueFunc = bool (*)(JSGlobalObject*, EncodedJSValue baseObject, EncodedJSValue value, PropertyName);
-using PutValueFuncWithPtr = bool (*)(JSGlobalObject*, EncodedJSValue baseObject, EncodedJSValue value, PropertyName, void*);
+using PutValueFunc = bool (JIT_OPERATION_ATTRIBUTES*)(JSGlobalObject*, EncodedJSValue baseObject, EncodedJSValue value, PropertyName);
+using PutValueFuncWithPtr = bool (JIT_OPERATION_ATTRIBUTES*)(JSGlobalObject*, EncodedJSValue baseObject, EncodedJSValue value, PropertyName, void*);
 
 class PropertySlot {
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to