[webkit-reviews] review granted: [Bug 203936] Add a stack overflow check in Yarr::ByteCompiler::emitDisjunction(). : [Attachment 382998] proposed patch.

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has granted Mark Lam <mark.lam at apple.com>'s
request for review:
Bug 203936: Add a stack overflow check in
Yarr::ByteCompiler::emitDisjunction().
https://bugs.webkit.org/show_bug.cgi?id=203936

Attachment 382998: proposed patch.

https://bugs.webkit.org/attachment.cgi?id=382998&action=review




--- Comment #2 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 382998
  --> https://bugs.webkit.org/attachment.cgi?id=382998
proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=382998&action=review

r=me

> Source/JavaScriptCore/ChangeLog:8
> +

can you add a test that fails on x86 by making the stack limit small?
(Also worth noting that we already have an instance of this failing elsewhere
too)

> Source/JavaScriptCore/yarr/YarrInterpreter.cpp:2416
> +    inline bool isSafeToRecurse() { return currentStackPointer() >=
m_stackLimit; }

please ASSERT(Thread::current().stack().isGrowingDownward());

or maybe turn VM's isSafeToRecurse into a static public function and call that.
(Or abstract it in WTF somewhere)