[webkit-reviews] review granted: [Bug 182705] REGRESSION(225695) : com.apple.WebKit.WebContent at com.apple.JavaScriptCore: JSC::RegExp::match + 630 :: stack overflow : [Attachment 333718] Patch

[bugzilla-daemon at webkit.org]

Mark Lam <mark.lam at apple.com> has granted Michael Saboff <msaboff at apple.com>'s
request for review:
Bug 182705: REGRESSION(225695) : com.apple.WebKit.WebContent at
com.apple.JavaScriptCore: JSC::RegExp::match + 630 :: stack overflow
https://bugs.webkit.org/show_bug.cgi?id=182705

Attachment 333718: Patch

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




--- Comment #3 from Mark Lam <mark.lam at apple.com> ---
Comment on attachment 333718
  --> https://bugs.webkit.org/attachment.cgi?id=333718
Patch

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

r=me with suggested improvements.

> Source/JavaScriptCore/runtime/RegExpInlines.h:122
> +	   if (m_regExpJITCode.usesPatternContextBuffer()) {
> +	       patternContextBuffer = vm.acquireRegExpPatternContexBuffer();
> +	       patternContextBufferSize = VM::patternContextBufferSize;
> +	   }
>  #define EXTRA_JIT_PARAMS  , patternContextBuffer, patternContextBufferSize

This seems like a prime candidate for using the RAII pattern.  How about
something like this:

    RegExpPatternContextBuffer buffer(vm);
    #define EXTRA_JIT_PARAMS  , buffer.buffer(), buffer.size()

You can either make RegExpPatternContextBuffer an embedded class in VM, or make
it a friend of VM so that it can do the real work.

> Source/JavaScriptCore/runtime/RegExpInlines.h:135
> +#if ENABLE(YARR_JIT_ALL_PARENS_EXPRESSIONS)
> +	   if (m_regExpJITCode.usesPatternContextBuffer())
> +	       vm.releaseRegExpPatternContexBuffer();
> +#endif

This can be handled automatically by the RAII object.

> Source/JavaScriptCore/yarr/YarrJIT.h:102
> +#endif
> +
> +#if ENABLE(YARR_JIT_ALL_PARENS_EXPRESSIONS)

Why is this needed?