Commit 4c1fa6d3 authored by mhahnenberg@apple.com's avatar mhahnenberg@apple.com

JSArrayBufferViews of length 0 allocate 0 CopiedSpace bytes, which is invalid

https://bugs.webkit.org/show_bug.cgi?id=123746

Reviewed by Geoffrey Garen.

Source/JavaScriptCore: 

This patch disallows clients from allocating 0 bytes in CopiedSpace. We enforce this invariant 
with an ASSERT in C++ code and a breakpoint in JIT code. Clients who care about 0-byte 
allocations (like JSArrayBufferViews) 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.
 
Also added a check so that JSArrayBufferViews don't try to copy their m_vector backing store when 
their length is 0. Also sprinkled several ASSERTs throughout the JSArrayBufferView code to make sure that 
when length is 0 m_vector is null.

* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileNewTypedArray):
* dfg/DFGSpeculativeJIT.h:
(JSC::DFG::SpeculativeJIT::emitAllocateBasicStorage):
* heap/CopiedSpaceInlines.h:
(JSC::CopiedSpace::tryAllocate):
* runtime/ArrayBuffer.h:
(JSC::ArrayBuffer::create):
* runtime/JSArrayBufferView.cpp:
(JSC::JSArrayBufferView::ConstructionContext::ConstructionContext):
* runtime/JSGenericTypedArrayViewInlines.h:
(JSC::::visitChildren):
(JSC::::copyBackingStore):
(JSC::::slowDownAndWasteMemory):

LayoutTests: 

Added a test to make sure that we don't crash when allocating a typed array with 0 length.

* js/script-tests/typedarray-zero-size.js: Added.
(foo):
* js/typedarray-zero-size-expected.txt: Added.
* js/typedarray-zero-size.html: Added.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@158583 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 7537b6dc
2013-11-04 Mark Hahnenberg <mhahnenberg@apple.com>
JSArrayBufferViews of length 0 allocate 0 CopiedSpace bytes, which is invalid
https://bugs.webkit.org/show_bug.cgi?id=123746
Reviewed by Geoffrey Garen.
Added a test to make sure that we don't crash when allocating a typed array with 0 length.
* js/script-tests/typedarray-zero-size.js: Added.
(foo):
* js/typedarray-zero-size-expected.txt: Added.
* js/typedarray-zero-size.html: Added.
2013-11-04 Alexey Proskuryakov <ap@apple.com>
Implement generateKey for HMAC and AES-CBC
......
description(
"Tests that creating TypedArrays of length 0 doesn't cause us to crash."
);
var array = new Uint8Array(0);
function foo() {
return new Uint16Array(0);
}
var result = 0;
for (var i = 1; i < 10001; i++) {
var newArray = foo();
var otherArray = new Array(i);
for (var j = 0; j < i; ++j)
otherArray[j] = j;
result += otherArray[i - 1];
}
if (result != (10000 * 9999) / 2)
throw "Bad result: " + result;
Tests that creating TypedArrays of length 0 doesn't cause us to crash.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS successfullyParsed is true
TEST COMPLETE
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
<html>
<head>
<script src="../resources/js-test-pre.js"></script>
</head>
<body>
<script src="script-tests/typedarray-zero-size.js"></script>
<script src="../resources/js-test-post.js"></script>
</body>
</html>
2013-11-04 Mark Hahnenberg <mhahnenberg@apple.com>
JSArrayBufferViews of length 0 allocate 0 CopiedSpace bytes, which is invalid
https://bugs.webkit.org/show_bug.cgi?id=123746
Reviewed by Geoffrey Garen.
This patch disallows clients from allocating 0 bytes in CopiedSpace. We enforce this invariant
with an ASSERT in C++ code and a breakpoint in JIT code. Clients who care about 0-byte
allocations (like JSArrayBufferViews) 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.
Also added a check so that JSArrayBufferViews don't try to copy their m_vector backing store when
their length is 0. Also sprinkled several ASSERTs throughout the JSArrayBufferView code to make sure that
when length is 0 m_vector is null.
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileNewTypedArray):
* dfg/DFGSpeculativeJIT.h:
(JSC::DFG::SpeculativeJIT::emitAllocateBasicStorage):
* heap/CopiedSpaceInlines.h:
(JSC::CopiedSpace::tryAllocate):
* runtime/ArrayBuffer.h:
(JSC::ArrayBuffer::create):
* runtime/JSArrayBufferView.cpp:
(JSC::JSArrayBufferView::ConstructionContext::ConstructionContext):
* runtime/JSGenericTypedArrayViewInlines.h:
(JSC::::visitChildren):
(JSC::::copyBackingStore):
(JSC::::slowDownAndWasteMemory):
2013-11-04 Julien Brianceau <jbriance@cisco.com>
[sh4] Refactor jumps in baseline JIT to return label after the jump.
......
......@@ -4709,6 +4709,7 @@ void SpeculativeJIT::compileNewTypedArray(Node* node)
slowCases.append(m_jit.branch32(
MacroAssembler::Above, sizeGPR, TrustedImm32(JSArrayBufferView::fastSizeLimit)));
slowCases.append(m_jit.branchTest32(MacroAssembler::Zero, sizeGPR));
m_jit.move(sizeGPR, scratchGPR);
m_jit.lshift32(TrustedImm32(logElementSize(type)), scratchGPR);
......
......@@ -2052,7 +2052,15 @@ public:
MacroAssembler::Jump emitAllocateBasicStorage(SizeType size, GPRReg resultGPR)
{
CopiedAllocator* copiedAllocator = &m_jit.vm()->heap.storageAllocator();
// It's invalid to allocate zero bytes in CopiedSpace.
#ifndef NDEBUG
m_jit.move(size, resultGPR);
MacroAssembler::Jump nonZeroSize = m_jit.branchTest32(MacroAssembler::NonZero, resultGPR);
m_jit.breakpoint();
nonZeroSize.link(&m_jit);
#endif
m_jit.loadPtr(&copiedAllocator->m_currentRemaining, resultGPR);
MacroAssembler::Jump slowPath = m_jit.branchSubPtr(JITCompiler::Signed, size, resultGPR);
m_jit.storePtr(resultGPR, &copiedAllocator->m_currentRemaining);
......
......@@ -150,6 +150,7 @@ inline void CopiedSpace::allocateBlock()
inline CheckedBoolean CopiedSpace::tryAllocate(size_t bytes, void** outPtr)
{
ASSERT(!m_heap->vm()->isInitializingObject());
ASSERT(bytes);
if (!m_allocator.tryAllocate(bytes, outPtr))
return tryAllocateSlowCase(bytes, outPtr);
......
......@@ -160,6 +160,7 @@ PassRefPtr<ArrayBuffer> ArrayBuffer::create(const void* source, unsigned byteLen
if (!contents.m_data)
return 0;
RefPtr<ArrayBuffer> buffer = adoptRef(new ArrayBuffer(contents));
ASSERT(!byteLength || source);
memcpy(buffer->data(), source, byteLength);
return buffer.release();
}
......
......@@ -45,9 +45,10 @@ JSArrayBufferView::ConstructionContext::ConstructionContext(
{
if (length <= fastSizeLimit) {
// Attempt GC allocation.
void* temp;
void* temp = 0;
size_t size = sizeOf(length, elementSize);
if (!vm.heap.tryAllocateStorage(0, size, &temp))
// CopiedSpace only allows non-zero size allocations.
if (size && !vm.heap.tryAllocateStorage(0, size, &temp))
return;
m_structure = structure;
......
......@@ -441,7 +441,8 @@ void JSGenericTypedArrayView<Adaptor>::visitChildren(JSCell* cell, SlotVisitor&
switch (thisObject->m_mode) {
case FastTypedArray: {
visitor.copyLater(thisObject, TypedArrayVectorCopyToken, thisObject->m_vector, thisObject->byteSize());
if (thisObject->m_vector)
visitor.copyLater(thisObject, TypedArrayVectorCopyToken, thisObject->m_vector, thisObject->byteSize());
break;
}
......@@ -469,6 +470,7 @@ void JSGenericTypedArrayView<Adaptor>::copyBackingStore(
if (token == TypedArrayVectorCopyToken
&& visitor.checkIfShouldCopy(thisObject->m_vector)) {
ASSERT(thisObject->m_vector);
void* oldVector = thisObject->m_vector;
void* newVector = visitor.allocateNewSpace(thisObject->byteSize());
memcpy(newVector, oldVector, thisObject->byteSize());
......@@ -505,6 +507,7 @@ ArrayBuffer* JSGenericTypedArrayView<Adaptor>::slowDownAndWasteMemory(JSArrayBuf
if (thisObject->m_mode == FastTypedArray
&& !thisObject->m_butterfly && size >= sizeof(IndexingHeader)) {
ASSERT(thisObject->m_vector);
// Reuse already allocated memory if at all possible.
thisObject->m_butterfly =
static_cast<IndexingHeader*>(thisObject->m_vector)->butterfly();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment