Attachment 369765 Details for Bug 197677 – patch

[patch] patch

a-backup.diff (text/plain), 9.81 KB, created by Saam Barati on 2019-05-13 12:53:07 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Saam Barati

Created: 2019-05-13 12:53:07 PDT

Size: 9.81 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 245245)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,33 @@
>+2019-05-13  Saam Barati  <sbarati@apple.com>
>+
>+        macro assembler code-pointer tagging has its arguments backwards
>+        https://bugs.webkit.org/show_bug.cgi?id=197677
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        We had the destination as the leftmost instead of the rightmost argument,
>+        which goes against the convention of how we order arguments in macro assembler
>+        methods.
>+
>+        * assembler/MacroAssemblerARM64E.h:
>+        (JSC::MacroAssemblerARM64E::tagReturnAddress):
>+        (JSC::MacroAssemblerARM64E::untagReturnAddress):
>+        (JSC::MacroAssemblerARM64E::tagPtr):
>+        (JSC::MacroAssemblerARM64E::untagPtr):
>+        * dfg/DFGOSRExitCompilerCommon.cpp:
>+        (JSC::DFG::reifyInlinedCallFrames):
>+        * ftl/FTLThunks.cpp:
>+        (JSC::FTL::genericGenerationThunkGenerator):
>+        * jit/CCallHelpers.h:
>+        (JSC::CCallHelpers::prepareForTailCallSlow):
>+        * jit/CallFrameShuffler.cpp:
>+        (JSC::CallFrameShuffler::prepareForTailCall):
>+        * jit/ThunkGenerators.cpp:
>+        (JSC::emitPointerValidation):
>+        (JSC::arityFixupGenerator):
>+        * wasm/js/WebAssemblyFunction.cpp:
>+        (JSC::WebAssemblyFunction::jsCallEntrypointSlow):
>+
> 2019-05-13  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         [JSC] Compress miscelaneous JIT related data structures with Packed<>
>Index: Source/JavaScriptCore/assembler/MacroAssemblerARM64E.h
>===================================================================
>--- Source/JavaScriptCore/assembler/MacroAssemblerARM64E.h	(revision 245243)
>+++ Source/JavaScriptCore/assembler/MacroAssemblerARM64E.h	(working copy)
>@@ -41,22 +41,22 @@ class MacroAssemblerARM64E : public Macr
> public:
>     ALWAYS_INLINE void tagReturnAddress()
>     {
>-        tagPtr(ARM64Registers::lr, ARM64Registers::sp);
>+        tagPtr(ARM64Registers::sp, ARM64Registers::lr);
>     }
> 
>     ALWAYS_INLINE void untagReturnAddress()
>     {
>-        untagPtr(ARM64Registers::lr, ARM64Registers::sp);
>+        untagPtr(ARM64Registers::sp, ARM64Registers::lr);
>     }
> 
>-    ALWAYS_INLINE void tagPtr(RegisterID target, PtrTag tag)
>+    ALWAYS_INLINE void tagPtr(PtrTag tag, RegisterID target)
>     {
>         auto tagGPR = getCachedDataTempRegisterIDAndInvalidate();
>         move(TrustedImm64(tag), tagGPR);
>         m_assembler.pacib(target, tagGPR);
>     }
> 
>-    ALWAYS_INLINE void tagPtr(RegisterID target, RegisterID tag)
>+    ALWAYS_INLINE void tagPtr(RegisterID tag, RegisterID target)
>     {
>         if (target == ARM64Registers::lr && tag == ARM64Registers::sp) {
>             m_assembler.pacibsp();
>@@ -65,14 +65,14 @@ public:
>         m_assembler.pacib(target, tag);
>     }
> 
>-    ALWAYS_INLINE void untagPtr(RegisterID target, PtrTag tag)
>+    ALWAYS_INLINE void untagPtr(PtrTag tag, RegisterID target)
>     {
>         auto tagGPR = getCachedDataTempRegisterIDAndInvalidate();
>         move(TrustedImm64(tag), tagGPR);
>         m_assembler.autib(target, tagGPR);
>     }
> 
>-    ALWAYS_INLINE void untagPtr(RegisterID target, RegisterID tag)
>+    ALWAYS_INLINE void untagPtr(RegisterID tag, RegisterID target)
>     {
>         m_assembler.autib(target, tag);
>     }
>Index: Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp	(revision 245243)
>+++ Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp	(working copy)
>@@ -157,9 +157,9 @@ void reifyInlinedCallFrames(CCallHelpers
>             jit.loadPtr(AssemblyHelpers::Address(GPRInfo::callFrameRegister, CallFrame::returnPCOffset()), GPRInfo::regT3);
> #if CPU(ARM64E)
>             jit.addPtr(AssemblyHelpers::TrustedImm32(sizeof(CallerFrameAndPC)), GPRInfo::callFrameRegister, GPRInfo::regT2);
>-            jit.untagPtr(GPRInfo::regT3, GPRInfo::regT2);
>+            jit.untagPtr(GPRInfo::regT2, GPRInfo::regT3);
>             jit.addPtr(AssemblyHelpers::TrustedImm32(inlineCallFrame->returnPCOffset() + sizeof(void*)), GPRInfo::callFrameRegister, GPRInfo::regT2);
>-            jit.tagPtr(GPRInfo::regT3, GPRInfo::regT2);
>+            jit.tagPtr(GPRInfo::regT2, GPRInfo::regT3);
> #endif
>             jit.storePtr(GPRInfo::regT3, AssemblyHelpers::addressForByteOffset(inlineCallFrame->returnPCOffset()));
>             jit.loadPtr(AssemblyHelpers::Address(GPRInfo::callFrameRegister, CallFrame::callerFrameOffset()), GPRInfo::regT3);
>@@ -209,7 +209,7 @@ void reifyInlinedCallFrames(CCallHelpers
> #if CPU(ARM64E)
>             jit.addPtr(AssemblyHelpers::TrustedImm32(inlineCallFrame->returnPCOffset() + sizeof(void*)), GPRInfo::callFrameRegister, GPRInfo::regT2);
>             jit.move(AssemblyHelpers::TrustedImmPtr(jumpTarget), GPRInfo::nonArgGPR0);
>-            jit.tagPtr(GPRInfo::nonArgGPR0, GPRInfo::regT2);
>+            jit.tagPtr(GPRInfo::regT2, GPRInfo::nonArgGPR0);
>             jit.storePtr(GPRInfo::nonArgGPR0, AssemblyHelpers::addressForByteOffset(inlineCallFrame->returnPCOffset()));
> #else
>             jit.storePtr(AssemblyHelpers::TrustedImmPtr(jumpTarget), AssemblyHelpers::addressForByteOffset(inlineCallFrame->returnPCOffset()));
>Index: Source/JavaScriptCore/ftl/FTLThunks.cpp
>===================================================================
>--- Source/JavaScriptCore/ftl/FTLThunks.cpp	(revision 245243)
>+++ Source/JavaScriptCore/ftl/FTLThunks.cpp	(working copy)
>@@ -116,7 +116,7 @@ static MacroAssemblerCodeRef<JITThunkPtr
>     restoreAllRegisters(jit, buffer);
> 
> #if CPU(ARM64E)
>-    jit.untagPtr(AssemblyHelpers::linkRegister, resultTag);
>+    jit.untagPtr(resultTag, AssemblyHelpers::linkRegister);
>     jit.tagReturnAddress();
> #else
>     UNUSED_PARAM(resultTag);
>Index: Source/JavaScriptCore/jit/CCallHelpers.h
>===================================================================
>--- Source/JavaScriptCore/jit/CCallHelpers.h	(revision 245243)
>+++ Source/JavaScriptCore/jit/CCallHelpers.h	(working copy)
>@@ -807,7 +807,7 @@ public:
>         subPtr(TrustedImm32(2 * sizeof(void*)), newFrameSizeGPR);
> #if CPU(ARM64E)
>         addPtr(TrustedImm32(sizeof(CallerFrameAndPC)), MacroAssembler::framePointerRegister, tempGPR);
>-        untagPtr(linkRegister, tempGPR);
>+        untagPtr(tempGPR, linkRegister);
> #endif
> #elif CPU(MIPS)
>         loadPtr(Address(framePointerRegister, sizeof(void*)), returnAddressRegister);
>Index: Source/JavaScriptCore/jit/CallFrameShuffler.cpp
>===================================================================
>--- Source/JavaScriptCore/jit/CallFrameShuffler.cpp	(revision 245243)
>+++ Source/JavaScriptCore/jit/CallFrameShuffler.cpp	(working copy)
>@@ -456,7 +456,7 @@ void CallFrameShuffler::prepareForTailCa
>         MacroAssembler::linkRegister);
> #if CPU(ARM64E)
>     m_jit.addPtr(MacroAssembler::TrustedImm32(sizeof(CallerFrameAndPC)), MacroAssembler::framePointerRegister);
>-    m_jit.untagPtr(MacroAssembler::linkRegister, MacroAssembler::framePointerRegister);
>+    m_jit.untagPtr(MacroAssembler::framePointerRegister, MacroAssembler::linkRegister);
>     m_jit.subPtr(MacroAssembler::TrustedImm32(sizeof(CallerFrameAndPC)), MacroAssembler::framePointerRegister);
> #endif
> 
>Index: Source/JavaScriptCore/jit/ThunkGenerators.cpp
>===================================================================
>--- Source/JavaScriptCore/jit/ThunkGenerators.cpp	(revision 245243)
>+++ Source/JavaScriptCore/jit/ThunkGenerators.cpp	(working copy)
>@@ -53,7 +53,7 @@ inline void emitPointerValidation(CCallH
>     jit.abortWithReason(TGInvalidPointer);
>     isNonZero.link(&jit);
>     jit.pushToSave(pointerGPR);
>-    jit.untagPtr(pointerGPR, tag);
>+    jit.untagPtr(tag, pointerGPR);
>     jit.load8(pointerGPR, pointerGPR);
>     jit.popToRestore(pointerGPR);
> }
>@@ -459,10 +459,10 @@ MacroAssemblerCodeRef<JITThunkPtrTag> ar
> #if CPU(ARM64E)
>     jit.loadPtr(JSInterfaceJIT::Address(GPRInfo::callFrameRegister, CallFrame::returnPCOffset()), GPRInfo::regT3);
>     jit.addPtr(JSInterfaceJIT::TrustedImm32(sizeof(CallerFrameAndPC)), GPRInfo::callFrameRegister, extraTemp);
>-    jit.untagPtr(GPRInfo::regT3, extraTemp);
>+    jit.untagPtr(extraTemp, GPRInfo::regT3);
>     PtrTag tempReturnPCTag = static_cast<PtrTag>(random());
>     jit.move(JSInterfaceJIT::TrustedImmPtr(tempReturnPCTag), extraTemp);
>-    jit.tagPtr(GPRInfo::regT3, extraTemp);
>+    jit.tagPtr(extraTemp, GPRInfo::regT3);
>     jit.storePtr(GPRInfo::regT3, JSInterfaceJIT::Address(GPRInfo::callFrameRegister, CallFrame::returnPCOffset()));
> #endif
>     jit.move(JSInterfaceJIT::callFrameRegister, JSInterfaceJIT::regT3);
>@@ -515,9 +515,9 @@ MacroAssemblerCodeRef<JITThunkPtrTag> ar
> #if CPU(ARM64E)
>     jit.loadPtr(JSInterfaceJIT::Address(GPRInfo::callFrameRegister, CallFrame::returnPCOffset()), GPRInfo::regT3);
>     jit.move(JSInterfaceJIT::TrustedImmPtr(tempReturnPCTag), extraTemp);
>-    jit.untagPtr(GPRInfo::regT3, extraTemp);
>+    jit.untagPtr(extraTemp, GPRInfo::regT3);
>     jit.addPtr(JSInterfaceJIT::TrustedImm32(sizeof(CallerFrameAndPC)), GPRInfo::callFrameRegister, extraTemp);
>-    jit.tagPtr(GPRInfo::regT3, extraTemp);
>+    jit.tagPtr(extraTemp, GPRInfo::regT3);
>     jit.storePtr(GPRInfo::regT3, JSInterfaceJIT::Address(GPRInfo::callFrameRegister, CallFrame::returnPCOffset()));
> #endif
> 
>Index: Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp
>===================================================================
>--- Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp	(revision 245243)
>+++ Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp	(working copy)
>@@ -480,7 +480,7 @@ MacroAssemblerCodePtr<JSEntryPtrTag> Web
>     jit.move(CCallHelpers::TrustedImmPtr(this), GPRInfo::regT0);
>     jit.emitFunctionEpilogue();
> #if CPU(ARM64E)
>-    jit.untagPtr(MacroAssembler::linkRegister, MacroAssembler::stackPointerRegister);
>+    jit.untagReturnAddress();
> #endif
>     auto jumpToHostCallThunk = jit.jump();
>

Flags:

msaboff: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 197677: 369765 | 369769 | 369779