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

Thu, 16 Sep 2021 03:40:00 -0700

Title: [282540] trunk/Source/_javascript_Core
Revision
282540
Author
[email protected]
Date
2021-09-16 03:39:11 -0700 (Thu, 16 Sep 2021)

Log Message

Fix crash in 32 bits due to not enough scratch registers available
https://bugs.webkit.org/show_bug.cgi?id=230241

Patch by Mikhail R. Gadelha <[email protected]> on 2021-09-16
Reviewed by Filip Pizlo.

Since patch 229229 (Polymorphic PutByVal) landed, jsc is now reaching
the case Transition in `AccessCase::generateImpl` which needs three
scratch registers when reallocating, but in ARMv7/MIPS, there are only
two registers available.

So in this patch, `AccessCase::createTransition` is changed to actually
check if there are enough registers available before creating the
AccessCase object.

* bytecode/AccessCase.cpp:
(JSC::AccessCase::generateImpl):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (282539 => 282540)


--- trunk/Source/_javascript_Core/ChangeLog	2021-09-16 10:37:03 UTC (rev 282539)
+++ trunk/Source/_javascript_Core/ChangeLog	2021-09-16 10:39:11 UTC (rev 282540)
@@ -1,3 +1,22 @@
+2021-09-16  Mikhail R. Gadelha  <[email protected]>
+
+        Fix crash in 32 bits due to not enough scratch registers available
+        https://bugs.webkit.org/show_bug.cgi?id=230241
+
+        Reviewed by Filip Pizlo.
+
+        Since patch 229229 (Polymorphic PutByVal) landed, jsc is now reaching
+        the case Transition in `AccessCase::generateImpl` which needs three
+        scratch registers when reallocating, but in ARMv7/MIPS, there are only
+        two registers available.
+
+        So in this patch, `AccessCase::createTransition` is changed to actually
+        check if there are enough registers available before creating the
+        AccessCase object.
+
+        * bytecode/AccessCase.cpp:
+        (JSC::AccessCase::generateImpl):
+
 2021-09-15  Saam Barati  <[email protected]>
 
         Move some profiling to UnlinkedCodeBlock

Modified: trunk/Source/_javascript_Core/bytecode/AccessCase.cpp (282539 => 282540)


--- trunk/Source/_javascript_Core/bytecode/AccessCase.cpp	2021-09-16 10:37:03 UTC (rev 282539)
+++ trunk/Source/_javascript_Core/bytecode/AccessCase.cpp	2021-09-16 10:39:11 UTC (rev 282540)
@@ -123,16 +123,50 @@
 
 RefPtr<AccessCase> AccessCase::createTransition(
     VM& vm, JSCell* owner, CacheableIdentifier identifier, PropertyOffset offset, Structure* oldStructure, Structure* newStructure,
-    const ObjectPropertyConditionSet& conditionSet, RefPtr<PolyProtoAccessChain>&& prototypeAccessChain)
+    const ObjectPropertyConditionSet& conditionSet, RefPtr<PolyProtoAccessChain>&& prototypeAccessChain, const StructureStubInfo& stubInfo)
 {
     RELEASE_ASSERT(oldStructure == newStructure->previousID());
 
     // Skip optimizing the case where we need a realloc, if we don't have
     // enough registers to make it happen.
-    if (GPRInfo::numberOfRegisters < 6
-        && oldStructure->outOfLineCapacity() != newStructure->outOfLineCapacity()
-        && oldStructure->outOfLineCapacity()) {
-        return nullptr;
+    if (oldStructure->outOfLineCapacity() != newStructure->outOfLineCapacity()) {
+        // In 64 bits jsc uses 1 register for value, and it uses 2 registers in 32 bits
+        size_t requiredRegisters = 1; // stubInfo.valueRegs()
+#if USE(JSVALUE32_64)
+        ++requiredRegisters;
+#endif
+
+        // 1 register for the property in 64 bits
+        ++requiredRegisters;
+#if USE(JSVALUE32_64)
+        // In 32 bits, jsc uses may use one extra register, if it is not a Cell
+        if (stubInfo.propertyRegs().tagGPR() != InvalidGPRReg)
+            ++requiredRegisters;
+#endif
+
+        // 1 register for the base in 64 bits
+        ++requiredRegisters;
+#if USE(JSVALUE32_64)
+        // In 32 bits, jsc uses may use one extra register, if it is not a Cell
+        if (stubInfo.baseRegs().tagGPR() != InvalidGPRReg)
+            ++requiredRegisters;
+#endif
+
+        if (stubInfo.m_stubInfoGPR != InvalidGPRReg)
+            ++requiredRegisters;
+        if (stubInfo.m_arrayProfileGPR != InvalidGPRReg)
+            ++requiredRegisters;
+
+        // One extra register for scratchGPR
+        ++requiredRegisters;
+
+        // Check if we have enough registers when reallocating
+        if (oldStructure->outOfLineCapacity() && GPRInfo::numberOfRegisters < requiredRegisters)
+            return nullptr;
+
+        // If we are (re)allocating inline, jsc needs two extra scratchGPRs
+        if (!oldStructure->couldHaveIndexingHeader() && GPRInfo::numberOfRegisters < (requiredRegisters + 2))
+            return nullptr;
     }
 
     return adoptRef(*new AccessCase(vm, owner, Transition, identifier, offset, newStructure, conditionSet, WTFMove(prototypeAccessChain)));
@@ -2249,7 +2283,7 @@
 
     case Transition: {
         ASSERT(!viaProxy());
-        // AccessCase::transition() should have returned null if this wasn't true.
+        // AccessCase::createTransition() should have returned null if this wasn't true.
         RELEASE_ASSERT(GPRInfo::numberOfRegisters >= 6 || !structure()->outOfLineCapacity() || structure()->outOfLineCapacity() == newStructure()->outOfLineCapacity());
 
         // NOTE: This logic is duplicated in AccessCase::doesCalls(). It's important that doesCalls() knows

Modified: trunk/Source/_javascript_Core/bytecode/AccessCase.h (282539 => 282540)


--- trunk/Source/_javascript_Core/bytecode/AccessCase.h	2021-09-16 10:37:03 UTC (rev 282539)
+++ trunk/Source/_javascript_Core/bytecode/AccessCase.h	2021-09-16 10:39:11 UTC (rev 282540)
@@ -167,7 +167,7 @@
         Structure* = nullptr, const ObjectPropertyConditionSet& = ObjectPropertyConditionSet(), RefPtr<PolyProtoAccessChain>&& = nullptr);
 
     static RefPtr<AccessCase> createTransition(VM&, JSCell* owner, CacheableIdentifier, PropertyOffset, Structure* oldStructure,
-        Structure* newStructure, const ObjectPropertyConditionSet&, RefPtr<PolyProtoAccessChain>&&);
+        Structure* newStructure, const ObjectPropertyConditionSet&, RefPtr<PolyProtoAccessChain>&&, const StructureStubInfo&);
 
     static Ref<AccessCase> createDelete(VM&, JSCell* owner, CacheableIdentifier, PropertyOffset, Structure* oldStructure,
         Structure* newStructure);

Modified: trunk/Source/_javascript_Core/jit/Repatch.cpp (282539 => 282540)


--- trunk/Source/_javascript_Core/jit/Repatch.cpp	2021-09-16 10:37:03 UTC (rev 282539)
+++ trunk/Source/_javascript_Core/jit/Repatch.cpp	2021-09-16 10:39:11 UTC (rev 282540)
@@ -801,7 +801,7 @@
                         return GiveUpOnCache;
                 }
 
-                newCase = AccessCase::createTransition(vm, codeBlock, propertyName, offset, oldStructure, newStructure, conditionSet, WTFMove(prototypeAccessChain));
+                newCase = AccessCase::createTransition(vm, codeBlock, propertyName, offset, oldStructure, newStructure, conditionSet, WTFMove(prototypeAccessChain), stubInfo);
             }
         } else if (slot.isCacheableCustom() || slot.isCacheableSetter()) {
             if (slot.isCacheableCustom()) {

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to