Changeset 245251 in webkit

Timestamp:

May 13, 2019, 2:34:43 PM (7 years ago)

Author:

sbarati@apple.com

Message:

macro assembler code-pointer tagging has its arguments backwards
​https://bugs.webkit.org/show_bug.cgi?id=197677

Reviewed by Michael Saboff.

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):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• assembler/AbstractMacroAssembler.h (modified) (1 diff)
• assembler/MacroAssemblerARM64E.h (modified) (4 diffs)
• dfg/DFGOSRExitCompilerCommon.cpp (modified) (2 diffs)
• ftl/FTLThunks.cpp (modified) (1 diff)
• jit/CCallHelpers.h (modified) (1 diff)
• jit/CallFrameShuffler.cpp (modified) (1 diff)
• jit/ThunkGenerators.cpp (modified) (3 diffs)
• wasm/js/WebAssemblyFunction.cpp (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+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 Michael Saboff.
+
+        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  Tadeu Zagallo  <tzagallo@apple.com>
 

trunk/Source/JavaScriptCore/assembler/AbstractMacroAssembler.h

@@ -978 +978 @@
     ALWAYS_INLINE void untagReturnAddress() { }
 
-    ALWAYS_INLINE void tagPtr(RegisterID, PtrTag) { }
+    ALWAYS_INLINE void tagPtr(PtrTag, RegisterID) { }
     ALWAYS_INLINE void tagPtr(RegisterID, RegisterID) { }
-    ALWAYS_INLINE void untagPtr(RegisterID, PtrTag) { }
+    ALWAYS_INLINE void untagPtr(PtrTag, RegisterID) { }
     ALWAYS_INLINE void untagPtr(RegisterID, RegisterID) { }
     ALWAYS_INLINE void removePtrTag(RegisterID) { }

trunk/Source/JavaScriptCore/assembler/MacroAssemblerARM64E.h

@@ -42 +42 @@
     ALWAYS_INLINE void tagReturnAddress()
     {
-        tagPtr(ARM64Registers::lr, ARM64Registers::sp);
+        tagPtr(ARM64Registers::sp, ARM64Registers::lr);
     }
 
     ALWAYS_INLINE void untagReturnAddress()
     {
-        untagPtr(ARM64Registers::lr, ARM64Registers::sp);
-    }
-
-    ALWAYS_INLINE void tagPtr(RegisterID target, PtrTag tag)
+        untagPtr(ARM64Registers::sp, ARM64Registers::lr);
+    }
+
+    ALWAYS_INLINE void tagPtr(PtrTag tag, RegisterID target)
     {
         auto tagGPR = getCachedDataTempRegisterIDAndInvalidate();
@@ -57 +57 @@
     }
 
-    ALWAYS_INLINE void tagPtr(RegisterID target, RegisterID tag)
+    ALWAYS_INLINE void tagPtr(RegisterID tag, RegisterID target)
     {
         if (target == ARM64Registers::lr && tag == ARM64Registers::sp) {
@@ -66 +66 @@
     }
 
-    ALWAYS_INLINE void untagPtr(RegisterID target, PtrTag tag)
+    ALWAYS_INLINE void untagPtr(PtrTag tag, RegisterID target)
     {
         auto tagGPR = getCachedDataTempRegisterIDAndInvalidate();
@@ -73 +73 @@
     }
 
-    ALWAYS_INLINE void untagPtr(RegisterID target, RegisterID tag)
+    ALWAYS_INLINE void untagPtr(RegisterID tag, RegisterID target)
     {
         m_assembler.autib(target, tag);

trunk/Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp

@@ -158 +158 @@
 #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()));
@@ -210 +210 @@
             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

trunk/Source/JavaScriptCore/ftl/FTLThunks.cpp

@@ -117 +117 @@
 
 #if CPU(ARM64E)
-    jit.untagPtr(AssemblyHelpers::linkRegister, resultTag);
+    jit.untagPtr(resultTag, AssemblyHelpers::linkRegister);
     jit.tagReturnAddress();
 #else

trunk/Source/JavaScriptCore/jit/CCallHelpers.h

@@ -808 +808 @@
 #if CPU(ARM64E)
         addPtr(TrustedImm32(sizeof(CallerFrameAndPC)), MacroAssembler::framePointerRegister, tempGPR);
-        untagPtr(linkRegister, tempGPR);
+        untagPtr(tempGPR, linkRegister);
 #endif
 #elif CPU(MIPS)

trunk/Source/JavaScriptCore/jit/CallFrameShuffler.cpp

@@ -457 +457 @@
 #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

trunk/Source/JavaScriptCore/jit/ThunkGenerators.cpp

@@ -54 +54 @@
     isNonZero.link(&jit);
     jit.pushToSave(pointerGPR);
-    jit.untagPtr(pointerGPR, tag);
+    jit.untagPtr(tag, pointerGPR);
     jit.load8(pointerGPR, pointerGPR);
     jit.popToRestore(pointerGPR);
@@ -460 +460 @@
     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
@@ -516 +516 @@
     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

trunk/Source/JavaScriptCore/wasm/js/WebAssemblyFunction.cpp

@@ -481 +481 @@
     jit.emitFunctionEpilogue();
 #if CPU(ARM64E)
-    jit.untagPtr(MacroAssembler::linkRegister, MacroAssembler::stackPointerRegister);
+    jit.untagReturnAddress();
 #endif
     auto jumpToHostCallThunk = jit.jump();