Changeset 159706 in webkit

Timestamp:

Nov 22, 2013, 12:31:38 PM (13 years ago)

Author:

mark.lam@apple.com

Message:

Ensure that arity fixups honor stack alignment requirements.
​https://bugs.webkit.org/show_bug.cgi?id=124756.

Reviewed by Geoffrey Garen.

The LLINT and all the JITs rely on CommonSlowPaths::arityCheckFor() to
compute the arg count adjustment for the arity fixup. We take advantage
of this choke point and introduce the stack alignment padding there in
the guise of additional args.

The only cost of this approach is that the padding will also be
initialized to undefined values as if they were args. Since arity fixups
are considered a slow path that is rarely taken, this cost is not a
concern.

• runtime/CommonSlowPaths.h:

(JSC::CommonSlowPaths::arityCheckFor):

• runtime/VM.h:

(JSC::VM::isSafeToRecurse):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• runtime/CommonSlowPaths.h (modified) (2 diffs)
• runtime/VM.h (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2013-11-22  Mark Lam  <mark.lam@apple.com>
+
+        Ensure that arity fixups honor stack alignment requirements.
+        https://bugs.webkit.org/show_bug.cgi?id=124756.
+
+        Reviewed by Geoffrey Garen.
+
+        The LLINT and all the JITs rely on CommonSlowPaths::arityCheckFor() to
+        compute the arg count adjustment for the arity fixup. We take advantage
+        of this choke point and introduce the stack alignment padding there in
+        the guise of additional args.
+
+        The only cost of this approach is that the padding will also be
+        initialized to undefined values as if they were args. Since arity fixups
+        are considered a slow path that is rarely taken, this cost is not a
+        concern.
+
+        * runtime/CommonSlowPaths.h:
+        (JSC::CommonSlowPaths::arityCheckFor):
+        * runtime/VM.h:
+        (JSC::VM::isSafeToRecurse):
+
 2013-11-21  Filip Pizlo  <fpizlo@apple.com>
 

trunk/Source/JavaScriptCore/runtime/CommonSlowPaths.h

@@ -32 +32 @@
 #include "JSStackInlines.h"
 #include "NameInstance.h"
+#include "StackAlignment.h"
+#include "VM.h"
 #include <wtf/Platform.h>
+#include <wtf/StdLibExtras.h>
 
 #if ENABLE(JIT) || ENABLE(LLINT)
@@ -54 +57 @@
     int argumentCountIncludingThis = exec->argumentCountIncludingThis();
     
-    // This ensures enough space for the worst case scenario of zero arguments passed by the caller.
-    if (!stack->grow(exec->registers() - newCodeBlock->numParameters() + virtualRegisterForLocal(newCodeBlock->m_numCalleeRegisters).offset()))
+    ASSERT(argumentCountIncludingThis < newCodeBlock->numParameters());
+    int missingArgumentCount = newCodeBlock->numParameters() - argumentCountIncludingThis;
+    int paddedMissingArgumentCount = WTF::roundUpToMultipleOf(stackAlignmentRegisters(), missingArgumentCount);
+
+#if USE(SEPARATE_C_AND_JS_STACK)
+    if (!stack->grow(exec->registers() - paddedMissingArgumentCount))
         return -1;
-    
-    ASSERT(argumentCountIncludingThis < newCodeBlock->numParameters());
-    
-    // Too few arguments, return the number of missing arguments so the caller can
-    // grow the frame in place and fill in undefined values for the missing args.
-    return(newCodeBlock->numParameters() - argumentCountIncludingThis);
+#else
+    UNUSED_PARAM(stack);
+    if (!exec->vm().isSafeToRecurse(paddedMissingArgumentCount * sizeof(Register)))
+        return -1;
+#endif // USE(SEPARATE_C_AND_JS_STACK)
+
+    return paddedMissingArgumentCount;
 }
 

trunk/Source/JavaScriptCore/runtime/VM.h

@@ -370 +370 @@
         void* stackLimit() { return m_stackLimit; }
         void setStackLimit(void* limit) { m_stackLimit = limit; }
-        bool isSafeToRecurse() const
+        bool isSafeToRecurse(size_t neededStackInBytes = 0) const
         {
             ASSERT(wtfThreadData().stack().isGrowingDownward());
-            void* curr;
-            return &curr >= m_stackLimit;
+            int8_t* curr = reinterpret_cast<int8_t*>(&curr);
+            int8_t* limit = reinterpret_cast<int8_t*>(m_stackLimit);
+            return curr >= limit && static_cast<size_t>(curr - limit) >= neededStackInBytes;
         }