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
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

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].