Bug 191771 - All users of ArrayBuffer should agree on the same max size
Summary: All users of ArrayBuffer should agree on the same max size
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-11-16 14:15 PST by Filip Pizlo
Modified: 2018-11-19 06:03 PST (History)
8 users (show)

See Also:

Attachments
the patch (9.61 KB, patch)
2018-11-16 14:17 PST, Filip Pizlo 		no flags Details | Formatted Diff | Diff
the patch (9.76 KB, patch)
2018-11-16 14:22 PST, Filip Pizlo 		mark.lam: review+
Details | Formatted Diff | Diff
the patch (11.31 KB, patch)
2018-11-16 15:31 PST, Filip Pizlo 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) 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 Filip Pizlo 2018-11-16 14:15:46 PST 
Patch forthcoming.
Comment 1 Filip Pizlo 2018-11-16 14:17:56 PST 
Created attachment 355119 [details]
the patch
Comment 2 Filip Pizlo 2018-11-16 14:22:12 PST 
Created attachment 355121 [details]
the patch

Fixed some tests.
Comment 3 Mark Lam 2018-11-16 14:33:09 PST 
<rdar://problem/46124570>
Comment 4 Mark Lam 2018-11-16 14:34:29 PST 
Comment on attachment 355121 [details]
the patch

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

r=me

> Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119
> +    RELEASE_ASSERT(m_sizeInBytes <= MAX_ARRAY_BUFFER_SIZE);

nit: This can be a debug ASSERT because we already have a size check above to ensure this.
Comment 5 Filip Pizlo 2018-11-16 15:31:51 PST 
Created attachment 355143 [details]
the patch

It turns out that we have to allow large maximum and then fail when you try to grow.  Otherwise we fail some spec test.
Comment 6 Filip Pizlo 2018-11-16 16:43:03 PST 
Landed in https://trac.webkit.org/changeset/238326/webkit
Comment 7 Guillaume Emont 2018-11-19 06:03:49 PST 
Skipped the new tests in https://trac.webkit.org/r238373 for memory limited devices, as they use 800M+ memory (which I assume is expected for these tests).