Changeset 161036 in webkit

Timestamp:

Dec 23, 2013, 4:25:14 PM (11 years ago)

Author:

mark.lam@apple.com

Message:

CStack: Fix JSStack::grow(), shrink(), growSlowCase(), and setStackLimit().
​https://bugs.webkit.org/show_bug.cgi?id=126188.

Not yet reviewed.

These functions were inappropriately mixing "end" and "top" pointer idioms.
Specifically:

1. growSlowCase() was comparing a newEnd pointer against m_commitTop, and using this to compute the size that the stack needs to grow.
2. shrink() was wrongly computing excess capacity by subtracting baseOfStack() (which is at high memory) from m_commitTop (which points to lower memory). Also, baseOfStack() is an "end" pointer while m_commitTop is a "top" pointer. This is a mismatch.

To fix this and simplify the code a bit, I changed all of these functions
to take a newTopOfStack pointer instead of a newEnd pointer, and adjusted
their callers where needed to pass the appropropriate pointer values.

• interpreter/JSStack.cpp:

(JSC::JSStack::growSlowCase):

• interpreter/JSStack.h:
• interpreter/JSStackInlines.h:

(JSC::JSStack::popFrame):
(JSC::JSStack::shrink):
(JSC::JSStack::grow):
(JSC::JSStack::setStackLimit):

Location:

branches/jsCStack/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• interpreter/JSStack.cpp (modified) (3 diffs)
• interpreter/JSStack.h (modified) (1 diff)
• interpreter/JSStackInlines.h (modified) (1 diff)

branches/jsCStack/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2013-12-23  Mark Lam  <mark.lam@apple.com>
+
+        CStack: Fix JSStack::grow(), shrink(), growSlowCase(), and setStackLimit().
+        https://bugs.webkit.org/show_bug.cgi?id=126188.
+
+        Not yet reviewed.
+
+        These functions were inappropriately mixing "end" and "top" pointer idioms.
+        Specifically:
+        1. growSlowCase() was comparing a newEnd pointer against m_commitTop, and
+           using this to compute the size that the stack needs to grow.
+        2. shrink() was wrongly computing excess capacity by subtracting
+           baseOfStack() (which is at high memory) from m_commitTop (which points
+           to lower memory). Also, baseOfStack() is an "end" pointer while
+           m_commitTop is a "top" pointer. This is a mismatch.
+
+        To fix this and simplify the code a bit, I changed all of these functions
+        to take a newTopOfStack pointer instead of a newEnd pointer, and adjusted
+        their callers where needed to pass the appropropriate pointer values.
+
+        * interpreter/JSStack.cpp:
+        (JSC::JSStack::growSlowCase):
+        * interpreter/JSStack.h:
+        * interpreter/JSStackInlines.h:
+        (JSC::JSStack::popFrame):
+        (JSC::JSStack::shrink):
+        (JSC::JSStack::grow):
+        (JSC::JSStack::setStackLimit):
+
 2013-12-23  Mark Lam  <mark.lam@apple.com>
 

branches/jsCStack/Source/JavaScriptCore/interpreter/JSStack.cpp

@@ -78 +78 @@
 }
 
-bool JSStack::growSlowCase(Register* newEnd)
+bool JSStack::growSlowCase(Register* newTopOfStack)
 {
     // If we have already committed enough memory to satisfy this request,
     // just update the end pointer and return.
-    if (newEnd >= m_commitTop) {
-        setStackLimit(newEnd);
+    if (newTopOfStack >= m_commitTop) {
+        setStackLimit(newTopOfStack);
         return true;
     }
@@ -90 +90 @@
     // have it is still within our budget. If not, we'll fail to grow and
     // return false.
-    long delta = roundUpAllocationSize(reinterpret_cast<char*>(m_commitTop) - reinterpret_cast<char*>(newEnd), commitSize);
+    long delta = roundUpAllocationSize(reinterpret_cast<char*>(m_commitTop) - reinterpret_cast<char*>(newTopOfStack), commitSize);
     if (reinterpret_cast<char*>(m_commitTop) - delta <= reinterpret_cast<char*>(m_useableTop))
         return false;
@@ -99 +99 @@
     addToCommittedByteCount(delta);
     m_commitTop = reinterpret_cast_ptr<Register*>(reinterpret_cast<char*>(m_commitTop) - delta);
-    setStackLimit(newEnd);
+    setStackLimit(newTopOfStack);
     return true;
 }

branches/jsCStack/Source/JavaScriptCore/interpreter/JSStack.h

@@ -161 +161 @@
 #endif
 
-        bool grow(Register* topOfStack);
-        bool growSlowCase(Register*);
-        void shrink(Register*);
+        bool grow(Register* newTopOfStack);
+        bool growSlowCase(Register* newTopOfStack);
+        void shrink(Register* newTopOfStack);
         void releaseExcessCapacity();
         void addToCommittedByteCount(long);
 
-        void setStackLimit(Register* newEnd);
+        void setStackLimit(Register* newTopOfStack);
 #endif // ENABLE(LLINT_C_LOOP)
 

branches/jsCStack/Source/JavaScriptCore/interpreter/JSStackInlines.h

@@ -167 +167 @@
     // are no more frames on the stack.
     if (!callerFrame)
-        shrink(baseOfStack());
+        shrink(highAddress());
 
     installTrapsAfterFrame(callerFrame);
 }
 
-inline void JSStack::shrink(Register* newEnd)
-{
+inline void JSStack::shrink(Register* newTopOfStack)
+{
+    Register* newEnd = newTopOfStack - 1;
     if (newEnd >= m_end)
         return;
-    setStackLimit(newEnd);
-    if (m_end == baseOfStack() && (m_commitTop - baseOfStack()) >= maxExcessCapacity)
+    setStackLimit(newTopOfStack);
+    if (m_end == baseOfStack() && (highAddress() - m_commitTop) >= maxExcessCapacity)
         releaseExcessCapacity();
 }
 
-inline bool JSStack::grow(Register* topOfStack)
-{
-    Register* newEnd = topOfStack - 1;
+inline bool JSStack::grow(Register* newTopOfStack)
+{
+    Register* newEnd = newTopOfStack - 1;
     if (newEnd >= m_end)
         return true;
-    return growSlowCase(newEnd);
-}
-
-inline void JSStack::setStackLimit(Register* newEnd)
-{
+    return growSlowCase(newTopOfStack);
+}
+
+inline void JSStack::setStackLimit(Register* newTopOfStack)
+{
+    Register* newEnd = newTopOfStack - 1;
     m_end = newEnd;
 #if ENABLE(LLINT_C_LOOP)
-    m_vm.setJSStackLimit(newEnd + 1);
+    m_vm.setJSStackLimit(newTopOfStack);
 #endif
 }