Title: [161036] branches/jsCStack/Source/_javascript_Core
- Revision
- 161036
- Author
- [email protected]
- Date
- 2013-12-23 16:25:14 -0800 (Mon, 23 Dec 2013)
Log 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):
Modified Paths
Diff
Modified: branches/jsCStack/Source/_javascript_Core/ChangeLog (161035 => 161036)
--- branches/jsCStack/Source/_javascript_Core/ChangeLog 2013-12-24 00:20:47 UTC (rev 161035)
+++ branches/jsCStack/Source/_javascript_Core/ChangeLog 2013-12-24 00:25:14 UTC (rev 161036)
@@ -1,5 +1,34 @@
2013-12-23 Mark Lam <[email protected]>
+ 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 <[email protected]>
+
CStack: Cosmetic: rename JSStack::m_commitEnd to m_commitTop.
https://bugs.webkit.org/show_bug.cgi?id=126186
Modified: branches/jsCStack/Source/_javascript_Core/interpreter/JSStack.cpp (161035 => 161036)
--- branches/jsCStack/Source/_javascript_Core/interpreter/JSStack.cpp 2013-12-24 00:20:47 UTC (rev 161035)
+++ branches/jsCStack/Source/_javascript_Core/interpreter/JSStack.cpp 2013-12-24 00:25:14 UTC (rev 161036)
@@ -77,19 +77,19 @@
m_reservation.deallocate();
}
-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;
}
// Compute the chunk size of additional memory to commit, and see if we
// 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;
@@ -98,7 +98,7 @@
m_reservation.commit(reinterpret_cast<char*>(m_commitTop) - delta, delta);
addToCommittedByteCount(delta);
m_commitTop = reinterpret_cast_ptr<Register*>(reinterpret_cast<char*>(m_commitTop) - delta);
- setStackLimit(newEnd);
+ setStackLimit(newTopOfStack);
return true;
}
Modified: branches/jsCStack/Source/_javascript_Core/interpreter/JSStack.h (161035 => 161036)
--- branches/jsCStack/Source/_javascript_Core/interpreter/JSStack.h 2013-12-24 00:20:47 UTC (rev 161035)
+++ branches/jsCStack/Source/_javascript_Core/interpreter/JSStack.h 2013-12-24 00:25:14 UTC (rev 161036)
@@ -160,13 +160,13 @@
void installTrapsAfterFrame(CallFrame*) { }
#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)
void enableErrorStackReserve();
Modified: branches/jsCStack/Source/_javascript_Core/interpreter/JSStackInlines.h (161035 => 161036)
--- branches/jsCStack/Source/_javascript_Core/interpreter/JSStackInlines.h 2013-12-24 00:20:47 UTC (rev 161035)
+++ branches/jsCStack/Source/_javascript_Core/interpreter/JSStackInlines.h 2013-12-24 00:25:14 UTC (rev 161036)
@@ -166,33 +166,35 @@
// this case, we're shrinking all the way to the beginning since there
// 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)
+inline bool JSStack::grow(Register* newTopOfStack)
{
- Register* newEnd = topOfStack - 1;
+ Register* newEnd = newTopOfStack - 1;
if (newEnd >= m_end)
return true;
- return growSlowCase(newEnd);
+ return growSlowCase(newTopOfStack);
}
-inline void JSStack::setStackLimit(Register* newEnd)
+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
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
