[webkit-reviews] review granted: [Bug 126320] CStack: Need a separate stack limit for the JS stack and the C stack : [Attachment 220745] patch 4: fixed some C loop bit rot from patch 3.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jan 9 12:35:03 PST 2014


Geoffrey Garen <ggaren at apple.com> has granted Mark Lam <mark.lam at apple.com>'s
request for review:
Bug 126320: CStack: Need a separate stack limit for the JS stack and the C
stack
https://bugs.webkit.org/show_bug.cgi?id=126320

Attachment 220745: patch 4: fixed some C loop bit rot from patch 3.
https://bugs.webkit.org/attachment.cgi?id=220745&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=220745&action=review


>>> Source/JavaScriptCore/interpreter/JSStack.cpp:152
>>> +void JSStack::setHostZoneSize(size_t hostZoneSize)
>> 
>> It doesn't make sense for the JSStack class to have a concept of a "host
zone size". The whole point of the JSStack class is that, when it is in use, it
is segregated from the host stack, and does not contain any host stack data.
> 
> This is not true.  Some time back, you told me to remove the
JSStack::entryChecks() at all the Interpreter::execute*() entry points.  And
instead, we’ll have a host zone even in the C loop JSStack so that
doCallJavaScript() can push the VMEntrySentinel frame without requiring a stack
check first.  That’s why we have this host zone.

It sounds like you're using this zone for a few purposes: host code, bounded JS
usage, and error recovery. So, "host zone" is probably not a great name
anymore. How about "reserved zone".

r=me, with a follow-up patch for that.


More information about the webkit-reviews mailing list