We have 3 options here: (1) Allow clients to allocate 0 bytes and return NULL when they do. This is less than ideal because it adds an extra null check to the fast path for CopiedSpace allocation. (2) Allow clients to allocate 0 bytes and do no special checks (i.e. return a valid pointer to some CopiedBlock). This is the worst of the three options because clients are already not allowed to copy allocations of size 0, so they would have a valid pointer that they could do nothing with and which would eventually point to invalid memory when the CopiedBlock was thrown away without updating the pointer. All in all, not a good idea. (3) Disallow clients from allocating 0 bytes. Enforce with a RELEASE_ASSERT in C++ code and breakpoints in JIT code. This is probably the way to go. Clients who care about 0-byte allocations must handle that case themselves, but we don't punish anybody else for the rare case that somebody decides to allocate a 0-length typed array. It also makes the allocation and copying cases consistent for CopiedSpace: no 0-byte allocations, no 0-byte copying.
<rdar://problem/15378335>
Created attachment 215936 [details] Patch
(In reply to comment #2) > Created an attachment (id=215936) [details] > Patch Forgot to svn add the new test, uploading new version...
Comment on attachment 215936 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=215936&action=review r=me > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4712 > + slowCases.append(m_jit.branchTest32(MacroAssembler::Zero, sizeGPR)); I think it's probably reasonably common to make a typed array and then append to it. Instead of a slow case, I think this should ultimately be a branch around the allocation code, followed by a store of 0 to JSArrayBufferView::offsetOfVector(). Can you file a follow-up bug?
(In reply to comment #4) > (From update of attachment 215936 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=215936&action=review > > r=me > > > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4712 > > + slowCases.append(m_jit.branchTest32(MacroAssembler::Zero, sizeGPR)); > > I think it's probably reasonably common to make a typed array and then append to it. Instead of a slow case, I think this should ultimately be a branch around the allocation code, followed by a store of 0 to JSArrayBufferView::offsetOfVector(). Can you file a follow-up bug? Is it possible to have a dynamically sized TypedArray? I thought they had a fixed size.
Committed r158583: <http://trac.webkit.org/changeset/158583>
Reopening because Phil has beef.
Marking as blocking bug 122679, because this prevents running a WebCrypto test suite.
Can we track the remaining issues in a separate bug? It doesn't seem right that this bug blocks WebCrypto at this point.
(In reply to comment #9) > Can we track the remaining issues in a separate bug? It doesn't seem right that this bug blocks WebCrypto at this point. Filed bug 124799.