Bug 192833 - String overflow in JSC::createError results in ASSERT in WTF::makeString
Summary: String overflow in JSC::createError results in ASSERT in WTF::makeString
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tadeu Zagallo
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-12-18 15:27 PST by Tadeu Zagallo
Modified: 2018-12-19 03:33 PST (History)
9 users (show)

See Also:


Attachments
Patch (2.98 KB, patch)
2018-12-18 15:42 PST, Tadeu Zagallo
no flags Details | Formatted Diff | Diff
Patch for landing (3.06 KB, patch)
2018-12-19 02:53 PST, Tadeu Zagallo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tadeu Zagallo 2018-12-18 15:27:51 PST
...
Comment 1 Tadeu Zagallo 2018-12-18 15:31:50 PST
<rdar://problem/45706868>
Comment 2 Tadeu Zagallo 2018-12-18 15:42:31 PST
Created attachment 357625 [details]
Patch
Comment 3 Mark Lam 2018-12-18 16:12:23 PST
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?
Comment 4 EWS Watchlist 2018-12-18 17:33:09 PST
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
Comment 5 Mark Lam 2018-12-18 17:35:46 PST
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";
Comment 6 Tadeu Zagallo 2018-12-19 02:50:20 PST
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!
Comment 7 Tadeu Zagallo 2018-12-19 02:53:58 PST
Created attachment 357666 [details]
Patch for landing
Comment 8 WebKit Commit Bot 2018-12-19 03:33:19 PST
Comment on attachment 357666 [details]
Patch for landing

Clearing flags on attachment: 357666

Committed r239375: <https://trac.webkit.org/changeset/239375>
Comment 9 WebKit Commit Bot 2018-12-19 03:33:21 PST
All reviewed patches have been landed.  Closing bug.