Bug 235217 - JSStringJoiner's constructor should take a size_t length.
Summary: JSStringJoiner's constructor should take a size_t length.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-01-13 20:14 PST by Mark Lam
Modified: 2022-01-19 09:46 PST (History)
8 users (show)

See Also:


Attachments
proposed patch. (4.07 KB, patch)
2022-01-13 20:20 PST, Mark Lam
ysuzuki: review+
Details | Formatted Diff | Diff
[fast-cq] patch for landing. (4.14 KB, patch)
2022-01-13 23:04 PST, Mark Lam
no flags Details | Formatted Diff | Diff
Patch for landing (1.01 KB, patch)
2022-01-19 07:40 PST, Michael Catanzaro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2022-01-13 20:14:08 PST
This allows underlying code to do a proper limit check on the length.

rdar://87538657
Comment 1 Mark Lam 2022-01-13 20:20:28 PST
Created attachment 449131 [details]
proposed patch.
Comment 2 Mark Lam 2022-01-13 23:04:16 PST
Created attachment 449142 [details]
[fast-cq] patch for landing.
Comment 3 Mark Lam 2022-01-14 16:42:36 PST
Comment on attachment 449142 [details]
[fast-cq] patch for landing.

Thanks for the review.  Landing now.
Comment 4 EWS 2022-01-14 16:45:31 PST
Committed r288037 (246063@main): <https://commits.webkit.org/246063@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 449142 [details].
Comment 5 Michael Catanzaro 2022-01-17 07:32:35 PST
Hi, there is a problem with this test when running with cloop:

Running stress/max-typed-array-length-toString.js.default
stress/max-typed-array-length-toString.js.default: Exception: FAILED: ReferenceError: Can't find variable: WebAssembly
stress/max-typed-array-length-toString.js.default: ERROR: Unexpected exit code: 3
FAIL: stress/max-typed-array-length-toString.js.default
Running stress/max-typed-array-length-toString.js.bytecode-cache
stress/max-typed-array-length-toString.js.bytecode-cache: Exception: FAILED: ReferenceError: Can't find variable: WebAssembly

I think WebAssembly is incompatible with cloop, is that right? Can we skip the test somehow in this case?
Comment 6 Mark Lam 2022-01-18 10:15:45 PST
(In reply to Michael Catanzaro from comment #5)
> I think WebAssembly is incompatible with cloop, is that right? Can we skip
> the test somehow in this case?

Fixed in r288120: <http://trac.webkit.org/r288120>.
Comment 7 Michael Catanzaro 2022-01-18 13:55:48 PST
Thanks Mark!
Comment 8 Michael Catanzaro 2022-01-19 06:32:02 PST
The test is actually still failing:

Crashing because current footprint: 637390848 exceeds limit: 629145600

I will skip it if memory limited.
Comment 9 Michael Catanzaro 2022-01-19 07:40:47 PST
Reopening to attach new patch.
Comment 10 Michael Catanzaro 2022-01-19 07:40:50 PST
Created attachment 449483 [details]
Patch for landing
Comment 11 EWS 2022-01-19 09:46:23 PST
Committed r288213 (246174@main): <https://commits.webkit.org/246174@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 449483 [details].