Bug 233299 - [JSC] TypedArray GetArrayLength should not use Reuse
Summary: [JSC] TypedArray GetArrayLength should not use Reuse
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-11-17 17:59 PST by Yusuke Suzuki
Modified: 2021-11-24 14:36 PST (History)
9 users (show)

See Also:


Attachments
Patch (2.69 KB, patch)
2021-11-17 18:02 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (1.05 KB, patch)
2021-11-23 08:07 PST, Michael Catanzaro
no flags Details | Formatted Diff | Diff
Patch (1.01 KB, patch)
2021-11-24 05:59 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 Yusuke Suzuki 2021-11-17 17:59:34 PST
[JSC] TypedArray GetArrayLength should not use Reuse
Comment 1 Yusuke Suzuki 2021-11-17 18:02:21 PST
Created attachment 444621 [details]
Patch
Comment 2 Yusuke Suzuki 2021-11-17 18:03:25 PST
rdar://85502079
Comment 3 Robin Morisset 2021-11-17 18:04:11 PST
Comment on attachment 444621 [details]
Patch

r=me
Comment 4 Yusuke Suzuki 2021-11-17 21:01:57 PST
Committed r285978 (244375@main): <https://commits.webkit.org/244375@main>
Comment 5 Michael Catanzaro 2021-11-19 06:00:07 PST
This new test is failing on cloop on every architecture. Example failure:

Running stress/get-array-length-reuse.js.default
stress/get-array-length-reuse.js.default: Crashing because current footprint: 632360960 exceeds limit: 629145600
stress/get-array-length-reuse.js.default: test_script_9640: line 2: 3125653 Aborted                 (core dumped) ( "$@" /var/lib/jenkins/workspace/WebKit-JSC/label/x86_64/WebKitBuild/Release/bin/jsc --useFTLJIT\=false --useFunctionDotArguments\=true --validateExceptionChecks\=true --useDollarVM\=true --maxPerThreadStackUsage\=1572864 --useFTLJIT\=true get-array-length-reuse.js )
stress/get-array-length-reuse.js.default: ERROR: Unexpected exit code: 134
FAIL: stress/get-array-length-reuse.js.default

Seems it runs successfully, then crashes at the very end because the artificial memory limit was exceeded?

The tests are failing on the JSC EWS with a different error:

Exception: RangeError: length too large
Comment 6 Michael Catanzaro 2021-11-22 06:11:13 PST
Reopening because the test is broken in two different ways. Yusuke, any ideas?
Comment 7 Michael Catanzaro 2021-11-23 06:33:45 PST
I'm going to start by skipping the entire test:

//@ skip

temporarily, until Yusuke gets a chance to look at this.

I considered:

//@ skip if $memoryLimited

but I'm not sure that the EWS failure is actually related to memory, or that the EWS is a memory-limited environment.
Comment 8 Michael Catanzaro 2021-11-23 08:07:45 PST
Created attachment 445035 [details]
Patch
Comment 9 Yusuke Suzuki 2021-11-23 20:15:34 PST
Comment on attachment 445035 [details]
Patch

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

> JSTests/stress/get-array-length-reuse.js:2
> +//@ skip
> +// FIXME: Unskip: https://bugs.webkit.org/show_bug.cgi?id=233299#c5

We should make it `//@ skip if $memoryLimited`.
Comment 10 Michael Catanzaro 2021-11-24 05:59:37 PST
Created attachment 445094 [details]
Patch
Comment 11 EWS 2021-11-24 14:36:54 PST
Committed r286153 (244537@main): <https://commits.webkit.org/244537@main>

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