Summary: | String overflow in JSC::createError results in ASSERT in WTF::makeString | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Tadeu Zagallo <tzagallo> | ||||||
Component: | JavaScriptCore | Assignee: | Tadeu Zagallo <tzagallo> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, ews-watchlist, guijemont, guijemont+jsc-armv7-ews, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Tadeu Zagallo
2018-12-18 15:27:51 PST
Created attachment 357625 [details]
Patch
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 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 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 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! Created attachment 357666 [details]
Patch for landing
Comment on attachment 357666 [details] Patch for landing Clearing flags on attachment: 357666 Committed r239375: <https://trac.webkit.org/changeset/239375> All reviewed patches have been landed. Closing bug. |