Changeset 209673 in webkit for trunk/Source/JavaScriptCore/jit/CallFrameShuffler.h

Timestamp:

Dec 10, 2016, 1:04:05 PM (9 years ago)

Author:

[email protected]

Message:

REGRESSION(r209653) Crash in CallFrameShuffler::snapshot()
​https://bugs.webkit.org/show_bug.cgi?id=165728

Reviewed by Filip Pizlo.

JSTests:

New regression test.

• stress/regress-165728.js: Added.

(sum1):
(sum2):
(tailCaller):
(test):

Source/JavaScriptCore:

It can be the case that a JSValueReg's CachedRecovery is the source for mutliple
GPRs. We only store the CachedRecovery in one slot of m_newRegisters to simplify
the recovery process. This is also done for the case where the recovery source
and destination are the same GPR.

In light of this change, snapshot needs to be taught that one CacheRecovery is
the source for multiple registers. This is done by using a two step process.
First find all the argument CachedRecovery's and create a vector mapping all of
the target GPRs and the source recovery. Then use that vector to get the
recovery for each register.

• jit/CallFrameShuffler.h:

(JSC::CallFrameShuffler::snapshot):

File:

• trunk/Source/JavaScriptCore/jit/CallFrameShuffler.h (modified) (3 diffs)

trunk/Source/JavaScriptCore/jit/CallFrameShuffler.h

@@ -114 +114 @@
         }
         data.args.resize(argCount());
+
+        Vector<ValueRecovery> registerArgRecoveries;
+#if USE(JSVALUE64)
+        // Find cached recoveries for all argument registers.
+        // We do this here, because a cached recovery may be the source for multiple
+        // argument registers, but it is only stored in one m_newRegister index.
+        if (data.argumentsInRegisters) {
+            unsigned maxArgumentRegister = std::min(static_cast<unsigned>(argCount()), NUMBER_OF_JS_FUNCTION_ARGUMENT_REGISTERS);
+            registerArgRecoveries.resize(maxArgumentRegister);
+            for (size_t i = 0; i < maxArgumentRegister; ++i) {
+                Reg reg { argumentRegisterForFunctionArgument(i) };
+                CachedRecovery* cachedRecovery { m_newRegisters[reg] };
+                if (cachedRecovery) {
+                    for (auto jsValueReg : cachedRecovery->gprTargets())
+                        registerArgRecoveries[jsFunctionArgumentForArgumentRegister(jsValueReg.gpr())] = cachedRecovery->recovery();
+                }
+            }
+        }
+#endif
+        
         for (size_t i = 0; i < argCount(); ++i) {
             if (argumentsLocation == StackArgs || i >= NUMBER_OF_JS_FUNCTION_ARGUMENT_REGISTERS)
@@ -119 +139 @@
             else {
                 Reg reg { argumentRegisterForFunctionArgument(i) };
-                CachedRecovery* cachedRecovery { m_newRegisters[reg] };
-                data.args[i] = cachedRecovery->recovery();
+                ASSERT(registerArgRecoveries[i]);
+                data.args[i] = registerArgRecoveries[i];
             }
         }
@@ -441 +461 @@
 #endif
 
-    // This stores, for each register, information about the recovery
-    // for the value that should eventually go into that register. The
-    // only registers that have a target recovery will be callee-save
-    // registers, as well as possibly one JSValueRegs for holding the
-    // callee.
+    // This stores information about the recovery for the value that
+    // should eventually go into that register. In some cases there
+    // are recoveries that have multiple targets. For those recoveries,
+    // only the first target register in the map has the recovery.
+    // We optimize the case where there are multiple targets for one
+    // recovery where one of those targets is also the source register.
+    // Restoring the first target becomes a nop and simplifies the logic
+    // of restoring the remaining targets.
     //
     // Once the correct value has been put into the registers, and
     // contrary to what we do with m_newFrame, we keep the entry in
     // m_newRegisters to simplify spilling.
+    //
+    // If a recovery has multiple target registers, we copy the value
+    // from the first target register to the remaining target registers
+    // at the end of the shuffling process.
     RegisterMap<CachedRecovery*> m_newRegisters;