Bug 192833 – String overflow in JSC::createError results in ASSERT in WTF::makeString

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Tadeu Zagallo]

...

[Tadeu Zagallo]

<rdar://problem/45706868>

[Tadeu Zagallo]

Created attachment 357625 [details]
Patch

[Mark Lam]

Comment on attachment 357625 [details]
Patch

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

r=me

> JSTests/stress/string-overflow-createError.js:3
> +bar = '2.3023e-320'
> +foo = bar.padEnd(2147483644, 1);
> +foo(true, 1).value;

Does this test throw an exception?  If so, don't you have to put it in a try catch statement?

[EWS Watchlist]

Comment on attachment 357625 [details]
Patch

Attachment 357625 [details] did not pass jsc-ews (mac):
Output: https://webkit-queues.webkit.org/results/10465801

New failing tests:
stress/string-overflow-createError.js.dfg-eager-no-cjit-validate
stress/string-overflow-createError.js.no-cjit-validate-phases
stress/string-overflow-createError.js.ftl-eager
stress/string-overflow-createError.js.ftl-no-cjit-no-inline-validate
stress/string-overflow-createError.js.ftl-eager-no-cjit
stress/string-overflow-createError.js.dfg-eager
stress/string-overflow-createError.js.ftl-no-cjit-no-put-stack-validate
stress/string-overflow-createError.js.ftl-no-cjit-validate-sampling-profiler
stress/string-overflow-createError.js.ftl-no-cjit-b3o1
stress/string-overflow-createError.js.dfg-maximal-flush-validate-no-cjit
stress/string-overflow-createError.js.no-llint
stress/string-overflow-createError.js.default
stress/string-overflow-createError.js.ftl-eager-no-cjit-b3o1
stress/string-overflow-createError.js.no-cjit-collect-continuously
stress/string-overflow-createError.js.ftl-no-cjit-small-pool
stress/string-overflow-createError.js.no-ftl
apiTests

[Mark Lam]

Comment on attachment 357625 [details]
Patch

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

>> JSTests/stress/string-overflow-createError.js:3
>> +foo(true, 1).value;
> 
> Does this test throw an exception?  If so, don't you have to put it in a try catch statement?

EWS bot is not happy.  It does look like you need to wrap this in a try-catch.  Something like this:

var exception;
try {
    bar = '2.3023e-320'
    foo = bar.padEnd(2147483644, 1);
    foo(true, 1).value;
} catch (e) {
    exception = e;
}

if (exception != "Error: Out of memory")
    throw "FAILED";

[Tadeu Zagallo]

Comment on attachment 357625 [details]
Patch

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

>>> JSTests/stress/string-overflow-createError.js:3
>>> +foo(true, 1).value;
>> 
>> Does this test throw an exception?  If so, don't you have to put it in a try catch statement?
> 
> EWS bot is not happy.  It does look like you need to wrap this in a try-catch.  Something like this:
> 
> var exception;
> try {
>     bar = '2.3023e-320'
>     foo = bar.padEnd(2147483644, 1);
>     foo(true, 1).value;
> } catch (e) {
>     exception = e;
> }
> 
> if (exception != "Error: Out of memory")
>     throw "FAILED";

Oops... I should have checked that. I will update, thanks!

[Tadeu Zagallo]

Created attachment 357666 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 357666 [details]
Patch for landing

Clearing flags on attachment: 357666

Committed r239375: <https://trac.webkit.org/changeset/239375>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.