Changeset 160056 in webkit

Timestamp:

Dec 3, 2013 3:56:31 PM (10 years ago)

Author:

msaboff@apple.com

Message:

ARM64: Crash in JIT code due to improper reuse of cached memory temp register
​https://bugs.webkit.org/show_bug.cgi?id=125181

Reviewed by Geoffrey Garen.

Changed load8() and load() to invalidate the memory temp CachedTempRegister when the
destination of an absolute load is the memory temp register since the source address
is also the memory temp register. Change branch{8,32,64} of an AbsoluteAddress with
a register to use the dataTempRegister as the destinate of the absolute load to
reduce the chance that we need to invalidate the memory temp register cache.
In the process, found and fixed an outright bug in branch8() where we'd load into
the data temp register and then compare and branch on the memory temp register.

• assembler/MacroAssemblerARM64.h:

(JSC::MacroAssemblerARM64::load8):
(JSC::MacroAssemblerARM64::branch32):
(JSC::MacroAssemblerARM64::branch64):
(JSC::MacroAssemblerARM64::branch8):
(JSC::MacroAssemblerARM64::load):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• assembler/MacroAssemblerARM64.h (modified) (6 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2013-12-03  Michael Saboff  <msaboff@apple.com>
+
+        ARM64: Crash in JIT code due to improper reuse of cached memory temp register
+        https://bugs.webkit.org/show_bug.cgi?id=125181
+
+        Reviewed by Geoffrey Garen.
+
+        Changed load8() and load() to invalidate the memory temp CachedTempRegister when the
+        destination of an absolute load is the memory temp register since the source address
+        is also the memory temp register.  Change branch{8,32,64} of an AbsoluteAddress with
+        a register to use the dataTempRegister as the destinate of the absolute load to
+        reduce the chance that we need to invalidate the memory temp register cache.
+        In the process, found and fixed an outright bug in branch8() where we'd load into
+        the data temp register and then compare and branch on the memory temp register.
+
+        * assembler/MacroAssemblerARM64.h:
+        (JSC::MacroAssemblerARM64::load8):
+        (JSC::MacroAssemblerARM64::branch32):
+        (JSC::MacroAssemblerARM64::branch64):
+        (JSC::MacroAssemblerARM64::branch8):
+        (JSC::MacroAssemblerARM64::load):
+
 2013-12-03  Michael Saboff  <msaboff@apple.com>
 

trunk/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h

@@ -899 +899 @@
         moveToCachedReg(TrustedImmPtr(address), m_cachedMemoryTempRegister);
         m_assembler.ldrb(dest, memoryTempRegister, ARM64Registers::zr);
+        if (dest == memoryTempRegister)
+            m_cachedMemoryTempRegister.invalidate();
     }
 
@@ -1571 +1573 @@
     Jump branch32(RelationalCondition cond, AbsoluteAddress left, RegisterID right)
     {
-        load32(left.m_ptr, getCachedMemoryTempRegisterIDAndInvalidate());
-        return branch32(cond, memoryTempRegister, right);
+        load32(left.m_ptr, getCachedDataTempRegisterIDAndInvalidate());
+        return branch32(cond, dataTempRegister, right);
     }
 
@@ -1609 +1611 @@
     Jump branch64(RelationalCondition cond, AbsoluteAddress left, RegisterID right)
     {
-        load64(left.m_ptr, getCachedMemoryTempRegisterIDAndInvalidate());
-        return branch64(cond, memoryTempRegister, right);
+        load64(left.m_ptr, getCachedDataTempRegisterIDAndInvalidate());
+        return branch64(cond, dataTempRegister, right);
     }
 
@@ -1642 +1644 @@
     {
         ASSERT(!(0xffffff00 & right.m_value));
-        load8(left.m_ptr, getCachedDataTempRegisterIDAndInvalidate());
+        load8(left.m_ptr, getCachedMemoryTempRegisterIDAndInvalidate());
         return branch32(cond, memoryTempRegister, right);
     }
@@ -2494 +2496 @@
             intptr_t addressDelta = addressAsInt - currentRegisterContents;
 
+            if (dest == memoryTempRegister)
+                m_cachedMemoryTempRegister.invalidate();
+
             if (isInIntRange(addressDelta)) {
                 if (ARM64Assembler::canEncodeSImmOffset(addressDelta)) {
@@ -2515 +2520 @@
 
         move(TrustedImmPtr(address), memoryTempRegister);
-        m_cachedMemoryTempRegister.setValue(reinterpret_cast<intptr_t>(address));
+        if (dest == memoryTempRegister)
+            m_cachedMemoryTempRegister.invalidate();
+        else
+            m_cachedMemoryTempRegister.setValue(reinterpret_cast<intptr_t>(address));
         m_assembler.ldr<datasize>(dest, memoryTempRegister, ARM64Registers::zr);
     }