[webkit-changes] [93189] trunk/Source/JavaScriptCore

Tue, 16 Aug 2011 18:34:37 -0700

Title: [93189] trunk/Source/_javascript_Core
Revision
93189
Author
msab...@apple.com
Date
2011-08-16 18:34:29 -0700 (Tue, 16 Aug 2011)

Log Message

Crash in Structure::visitChildren running iAd.js regression test suite under memory pressure
https://bugs.webkit.org/show_bug.cgi?id=66351

JIT::privateCompilePutByIdTransition expects that regT0 and regT1
have the basePayload and baseTag respectively.  In some cases,
we may get to this generated code with one or both of these
registers trash.  One know case is that regT0 on ARM may be
trashed as regT0 (r0) is also arg0 and can be overrun with sp due
to calls to JIT::restoreReturnAddress().  This patch uses the
values on the stack.  A longer term solution is to work out all
cases so that the register entry assumptions can assured.

While fixing this, also determined that the additional stack offset
of sizeof(void*) is not needed for ARM.

Reviewed by Gavin Barraclough.

* jit/JITPropertyAccess32_64.cpp:
(JSC::JIT::privateCompilePutByIdTransition):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (93188 => 93189)


--- trunk/Source/_javascript_Core/ChangeLog	2011-08-17 01:32:20 UTC (rev 93188)
+++ trunk/Source/_javascript_Core/ChangeLog	2011-08-17 01:34:29 UTC (rev 93189)
@@ -1,3 +1,25 @@
+2011-08-16  Michael Saboff  <msab...@apple.com>
+
+        Crash in Structure::visitChildren running iAd.js regression test suite under memory pressure
+        https://bugs.webkit.org/show_bug.cgi?id=66351
+
+        JIT::privateCompilePutByIdTransition expects that regT0 and regT1
+        have the basePayload and baseTag respectively.  In some cases,
+        we may get to this generated code with one or both of these
+        registers trash.  One know case is that regT0 on ARM may be
+        trashed as regT0 (r0) is also arg0 and can be overrun with sp due
+        to calls to JIT::restoreReturnAddress().  This patch uses the
+        values on the stack.  A longer term solution is to work out all
+        cases so that the register entry assumptions can assured.
+
+        While fixing this, also determined that the additional stack offset
+        of sizeof(void*) is not needed for ARM.
+
+        Reviewed by Gavin Barraclough.
+
+        * jit/JITPropertyAccess32_64.cpp:
+        (JSC::JIT::privateCompilePutByIdTransition):
+
 2011-08-15  Gavin Barraclough  <barraclo...@apple.com>
 
         https://bugs.webkit.org/show_bug.cgi?id=66263

Modified: trunk/Source/_javascript_Core/jit/JITPropertyAccess32_64.cpp (93188 => 93189)


--- trunk/Source/_javascript_Core/jit/JITPropertyAccess32_64.cpp	2011-08-17 01:32:20 UTC (rev 93188)
+++ trunk/Source/_javascript_Core/jit/JITPropertyAccess32_64.cpp	2011-08-17 01:34:29 UTC (rev 93189)
@@ -470,8 +470,17 @@
 
 void JIT::privateCompilePutByIdTransition(StructureStubInfo* stubInfo, Structure* oldStructure, Structure* newStructure, size_t cachedOffset, StructureChain* chain, ReturnAddressPtr returnAddress, bool direct)
 {
-    // It is assumed that regT0 contains the basePayload and regT1 contains the baseTag.  The value can be found on the stack.
-    
+    // The code below assumes that regT0 contains the basePayload and regT1 contains the baseTag. Restore them from the stack.
+#if CPU(MIPS) || CPU(SH4) || CPU(ARM)
+    // For MIPS, we don't add sizeof(void*) to the stack offset.
+    load32(Address(stackPointerRegister, OBJECT_OFFSETOF(JITStackFrame, args[0]) + OBJECT_OFFSETOF(JSValue, u.asBits.payload)), regT0);
+    // For MIPS, we don't add sizeof(void*) to the stack offset.
+    load32(Address(stackPointerRegister, OBJECT_OFFSETOF(JITStackFrame, args[0]) + OBJECT_OFFSETOF(JSValue, u.asBits.tag)), regT1);
+#else
+    load32(Address(stackPointerRegister, OBJECT_OFFSETOF(JITStackFrame, args[0]) + sizeof(void*) + OBJECT_OFFSETOF(JSValue, u.asBits.payload)), regT0);
+    load32(Address(stackPointerRegister, OBJECT_OFFSETOF(JITStackFrame, args[0]) + sizeof(void*) + OBJECT_OFFSETOF(JSValue, u.asBits.tag)), regT1);
+#endif
+
     JumpList failureCases;
     failureCases.append(branch32(NotEqual, regT1, TrustedImm32(JSValue::CellTag)));
     failureCases.append(branchPtr(NotEqual, Address(regT0, JSCell::structureOffset()), TrustedImmPtr(oldStructure)));
@@ -498,14 +507,24 @@
         stubCall.addArgument(TrustedImm32(oldStructure->propertyStorageCapacity()));
         stubCall.addArgument(TrustedImm32(newStructure->propertyStorageCapacity()));
         stubCall.call(regT0);
-        
+
         restoreReturnAddressBeforeReturn(regT3);
+
+#if CPU(MIPS) || CPU(SH4) || CPU(ARM)
+        // For MIPS, we don't add sizeof(void*) to the stack offset.
+        load32(Address(stackPointerRegister, OBJECT_OFFSETOF(JITStackFrame, args[0]) + OBJECT_OFFSETOF(JSValue, u.asBits.payload)), regT0);
+        // For MIPS, we don't add sizeof(void*) to the stack offset.
+        load32(Address(stackPointerRegister, OBJECT_OFFSETOF(JITStackFrame, args[0]) + OBJECT_OFFSETOF(JSValue, u.asBits.tag)), regT1);
+#else
+        load32(Address(stackPointerRegister, OBJECT_OFFSETOF(JITStackFrame, args[0]) + sizeof(void*) + OBJECT_OFFSETOF(JSValue, u.asBits.payload)), regT0);
+        load32(Address(stackPointerRegister, OBJECT_OFFSETOF(JITStackFrame, args[0]) + sizeof(void*) + OBJECT_OFFSETOF(JSValue, u.asBits.tag)), regT1);
+#endif
     }
 
     emitWriteBarrier(regT0, regT1);
 
     storePtr(TrustedImmPtr(newStructure), Address(regT0, JSCell::structureOffset()));
-#if CPU(MIPS) || CPU(SH4)
+#if CPU(MIPS) || CPU(SH4) || CPU(ARM)
     // For MIPS, we don't add sizeof(void*) to the stack offset.
     load32(Address(stackPointerRegister, OBJECT_OFFSETOF(JITStackFrame, args[2]) + OBJECT_OFFSETOF(JSValue, u.asBits.payload)), regT3);
     load32(Address(stackPointerRegister, OBJECT_OFFSETOF(JITStackFrame, args[2]) + OBJECT_OFFSETOF(JSValue, u.asBits.tag)), regT2);

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to