From bcd8e41674487c1b175b01c028699eb1ba41fecc Mon Sep 17 00:00:00 2001 From: Trevor Norris Date: Tue, 26 May 2015 12:42:14 -0600 Subject: [PATCH] buffer: implement Uint8Array backed Buffer With V8 4.4 removing the external array data API currently used by Buffer, the new implementation uses the Uint8Array to back Buffer. Buffers now have a maximum size of Smi::kMaxLength, as defined by V8. Which is ~2 GB on 64 bit and ~1 GB on 32 bit. The flag --use-old-buffer allows using the old Buffer implementation. This flag will be removed once V8 4.4 has landed. --- lib/buffer.js | 399 ++++++++++++++++++++--------- src/env.h | 1 + src/node.cc | 22 +- src/node_buffer.cc | 395 ++++++++++++++++++++++------ src/node_buffer.h | 6 +- src/node_internals.h | 2 + test/parallel/test-buffer-slice.js | 12 - test/parallel/test-buffer.js | 10 +- 8 files changed, 624 insertions(+), 223 deletions(-) delete mode 100644 test/parallel/test-buffer-slice.js diff --git a/lib/buffer.js b/lib/buffer.js index 1b9c68465d6b03..38262244e178a5 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -7,62 +7,219 @@ const alloc = smalloc.alloc; const truncate = smalloc.truncate; const sliceOnto = smalloc.sliceOnto; const kMaxLength = smalloc.kMaxLength; +const useOldBuffer = !!process.useOldBuffer; -exports.Buffer = Buffer; -exports.SlowBuffer = SlowBuffer; -exports.INSPECT_MAX_BYTES = 50; +var Buffer, SlowBuffer, NativeBuffer; -Buffer.poolSize = 8 * 1024; -var poolSize, poolOffset, allocPool; +// New Buffer implementation using Uint8Array. Done this way so the constructor +// name can remain "Buffer" and "SlowBuffer". +if (!useOldBuffer) { + Buffer = function Buffer(arg) { + // Common case. + if (typeof arg === 'number') { + // If less than zero, or NaN. + if (arg < 0 || !(arg <= 0 || arg > 0)) + arg = 0; + return allocate(arg); + } -function createPool() { - poolSize = Buffer.poolSize; - allocPool = alloc({}, poolSize); - poolOffset = 0; + // Slightly less common case. + if (typeof arg === 'string') { + return fromString(arg, arguments[1]); + } + + // Unusual. + return fromObject(arg); + }; + + Buffer.prototype.__proto__ = Uint8Array.prototype; + Buffer.__proto__ = Uint8Array; + + SlowBuffer = function SlowBuffer(length) { + if (length < 0) + length = 0; + return binding.create(length); + }; + + SlowBuffer.prototype.__proto__ = Buffer.prototype; + SlowBuffer.__proto__ = Buffer; + + binding.setupBufferJS(Buffer.prototype); } -createPool(); -function Buffer(arg) { - if (!(this instanceof Buffer)) { - // Avoid going through an ArgumentsAdaptorTrampoline in the common case. - if (arguments.length > 1) - return new Buffer(arg, arguments[1]); - return new Buffer(arg); +function allocate(size) { + if (size === 0) + return binding.create(0); + if (size < (Buffer.poolSize / 2)) { + if (size > (poolSize - poolOffset)) + createPool(); + var b = binding.slice(allocPool, poolOffset, poolOffset + size); + poolOffset += size; + return b; + } else { + return binding.create(size); } +} - this.length = 0; - this.parent = undefined; - // Common case. - if (typeof(arg) === 'number') { - fromNumber(this, arg); - return; +function fromString(string, encoding) { + if (typeof encoding !== 'string' || encoding === '') + encoding = 'utf8'; + + var length = byteLength(string, encoding); + if (length < Math.floor(Buffer.poolSize / 2)) { + if (length > (poolSize - poolOffset)) + createPool(); + var actual = allocPool.write(string, poolOffset, encoding); + var b = binding.slice(allocPool, poolOffset, poolOffset + actual); + poolOffset += actual; + return b; + + } else { + return binding.createFromString(string, encoding); + } +} + + +function fromObject(obj) { + if (obj instanceof Buffer) { + var b = allocate(obj.length); + obj.copy(b, 0, 0, obj.length); + return b; + } + + if (Array.isArray(obj)) { + var length = obj.length; + var b = allocate(length); + for (var i = 0; i < length; i++) + b[i] = obj[i] & 255; + return b; + } + + // This will fail for an actual ArrayBuffer. + if (obj.buffer instanceof ArrayBuffer || + obj.length) { + var length; + if (typeof obj.length !== 'number' || isNaN(obj.length)) + length = 0; + else + length = obj.length; + var b = allocate(length); + for (var i = 0; i < length; i++) { + b[i] = obj[i] & 255; + } + return b; + } + + if (obj.type === 'Buffer' && Array.isArray(obj.data)) { + var array = obj.data; + var b = allocate(array.length); + for (var i = 0; i < array.length; i++) + b[i] = array[i] & 255; + return b; } - // Slightly less common case. - if (typeof(arg) === 'string') { - fromString(this, arg, arguments.length > 1 ? arguments[1] : 'utf8'); - return; + throw new TypeError('must start with number, buffer, array or string'); +} + + +// Old Buffer implementation. Kept around in case something goes wrong. + + +if (useOldBuffer) { + Buffer = function Buffer(arg) { + if (!(this instanceof Buffer)) { + // Avoid going through an ArgumentsAdaptorTrampoline in the common case. + if (arguments.length > 1) + return new Buffer(arg, arguments[1]); + + return new Buffer(arg); + } + + this.length = 0; + this.parent = undefined; + + // Common case. + if (typeof(arg) === 'number') { + fromNumber(this, arg); + return; + } + + // Slightly less common case. + if (typeof(arg) === 'string') { + fromStringOld(this, arg, arguments.length > 1 ? arguments[1] : 'utf8'); + return; + } + + // Unusual. + fromObjectOld(this, arg); + }; + + SlowBuffer = function SlowBuffer(length) { + length = length >>> 0; + if (length > kMaxLength) { + throw new RangeError('Attempt to allocate Buffer larger than maximum ' + + 'size: 0x' + kMaxLength.toString(16) + ' bytes'); + } + var b = new NativeBuffer(length); + alloc(b, length); + return b; + }; + + // Bypass all checks for instantiating unallocated Buffer required for + // Objects created in C++. Significantly faster than calling the Buffer + // function. + NativeBuffer = function NativeBuffer(length) { + this.length = length >>> 0; + // Set this to keep the object map the same. + this.parent = undefined; } + NativeBuffer.prototype = Buffer.prototype; + + // add methods to Buffer prototype + binding.setupBufferJS(NativeBuffer); +} + + +exports.Buffer = Buffer; +exports.SlowBuffer = SlowBuffer; +exports.INSPECT_MAX_BYTES = 50; + + +Buffer.poolSize = 8 * 1024; +var poolSize, poolOffset, allocPool; + + +function createPoolNew() { + poolSize = Buffer.poolSize; + allocPool = binding.create(poolSize); + poolOffset = 0; +} - // Unusual. - fromObject(this, arg); +function createPoolOld() { + poolSize = Buffer.poolSize; + allocPool = alloc({}, poolSize); + poolOffset = 0; } +var createPool = useOldBuffer ? createPoolOld : createPoolNew; +createPool(); + + function fromNumber(that, length) { - allocate(that, length < 0 ? 0 : checked(length) | 0); + allocateOld(that, length < 0 ? 0 : checked(length) | 0); } -function fromString(that, string, encoding) { +function fromStringOld(that, string, encoding) { if (typeof(encoding) !== 'string' || encoding === '') encoding = 'utf8'; // Assumption: byteLength() return value is always < kMaxLength. var length = byteLength(string, encoding) | 0; - allocate(that, length); + allocateOld(that, length); var actual = that.write(string, encoding) | 0; if (actual !== length) { @@ -73,7 +230,7 @@ function fromString(that, string, encoding) { } } -function fromObject(that, object) { +function fromObjectOld(that, object) { if (object instanceof Buffer) return fromBuffer(that, object); @@ -94,13 +251,13 @@ function fromObject(that, object) { function fromBuffer(that, buffer) { var length = checked(buffer.length) | 0; - allocate(that, length); + allocateOld(that, length); buffer.copy(that, 0, 0, length); } function fromArray(that, array) { var length = checked(array.length) | 0; - allocate(that, length); + allocateOld(that, length); for (var i = 0; i < length; i += 1) that[i] = array[i] & 255; } @@ -108,7 +265,7 @@ function fromArray(that, array) { // Duplicate of fromArray() to keep fromArray() monomorphic. function fromTypedArray(that, array) { var length = checked(array.length) | 0; - allocate(that, length); + allocateOld(that, length); // Truncating the elements is probably not what people expect from typed // arrays with BYTES_PER_ELEMENT > 1 but it's compatible with the behavior // of the old Buffer constructor. @@ -118,7 +275,7 @@ function fromTypedArray(that, array) { function fromArrayLike(that, array) { var length = checked(array.length) | 0; - allocate(that, length); + allocateOld(that, length); for (var i = 0; i < length; i += 1) that[i] = array[i] & 255; } @@ -133,13 +290,13 @@ function fromJsonObject(that, object) { array = object.data; length = checked(array.length) | 0; } - allocate(that, length); + allocateOld(that, length); for (var i = 0; i < length; i += 1) that[i] = array[i] & 255; } -function allocate(that, length) { +function allocateOld(that, length) { var fromPool = length !== 0 && length <= Buffer.poolSize >>> 1; if (fromPool) that.parent = palloc(that, length); @@ -176,32 +333,6 @@ function checked(length) { return length >>> 0; } -function SlowBuffer(length) { - length = length >>> 0; - if (length > kMaxLength) { - throw new RangeError('Attempt to allocate Buffer larger than maximum ' + - 'size: 0x' + kMaxLength.toString(16) + ' bytes'); - } - var b = new NativeBuffer(length); - alloc(b, length); - return b; -} - - -// Bypass all checks for instantiating unallocated Buffer required for -// Objects created in C++. Significantly faster than calling the Buffer -// function. -function NativeBuffer(length) { - this.length = length >>> 0; - // Set this to keep the object map the same. - this.parent = undefined; -} -NativeBuffer.prototype = Buffer.prototype; - - -// add methods to Buffer prototype -binding.setupBufferJS(NativeBuffer); - // Static methods @@ -576,7 +707,9 @@ Buffer.prototype.toJSON = function() { // TODO(trevnorris): currently works like Array.prototype.slice(), which // doesn't follow the new standard for throwing on out of range indexes. -Buffer.prototype.slice = function(start, end) { +Buffer.prototype.slice = useOldBuffer ? slice : sliceNew; + +function slice(start, end) { var len = this.length; start = ~~start; end = end === undefined ? len : ~~end; @@ -607,7 +740,35 @@ Buffer.prototype.slice = function(start, end) { buf.parent = this.parent === undefined ? this : this.parent; return buf; -}; +} + + +function sliceNew(start, end) { + var len = this.length; + start = ~~start; + end = end === undefined ? len : ~~end; + + if (start < 0) { + start += len; + if (start < 0) + start = 0; + } else if (start > len) { + start = len; + } + + if (end < 0) { + end += len; + if (end < 0) + end = 0; + } else if (end > len) { + end = len; + } + + if (end < start) + end = start; + + return binding.slice(this, start, end); +} function checkOffset(offset, ext, length) { @@ -1073,77 +1234,79 @@ Buffer.prototype.writeDoubleBE = function writeDoubleBE(val, offset, noAssert) { // ES6 iterator -var ITERATOR_KIND_KEYS = 1; -var ITERATOR_KIND_ENTRIES = 3; +if (useOldBuffer) { + var ITERATOR_KIND_KEYS = 1; + var ITERATOR_KIND_ENTRIES = 3; -function BufferIteratorResult(value, done) { - this.value = value; - this.done = done; -} + function BufferIteratorResult(value, done) { + this.value = value; + this.done = done; + } -var resultCache = new Array(256); + var resultCache = new Array(256); -for (var i = 0; i < 256; i++) - resultCache[i] = Object.freeze(new BufferIteratorResult(i, false)); + for (var i = 0; i < 256; i++) + resultCache[i] = Object.freeze(new BufferIteratorResult(i, false)); -var finalResult = Object.freeze(new BufferIteratorResult(undefined, true)); + var finalResult = Object.freeze(new BufferIteratorResult(undefined, true)); -function BufferIterator(buffer, kind) { - this._buffer = buffer; - this._kind = kind; - this._index = 0; -} + function BufferIterator(buffer, kind) { + this._buffer = buffer; + this._kind = kind; + this._index = 0; + } -BufferIterator.prototype.next = function() { - var buffer = this._buffer; - var kind = this._kind; - var index = this._index; + BufferIterator.prototype.next = function() { + var buffer = this._buffer; + var kind = this._kind; + var index = this._index; - if (index >= buffer.length) - return finalResult; + if (index >= buffer.length) + return finalResult; - this._index++; + this._index++; - if (kind === ITERATOR_KIND_ENTRIES) - return new BufferIteratorResult([index, buffer[index]], false); + if (kind === ITERATOR_KIND_ENTRIES) + return new BufferIteratorResult([index, buffer[index]], false); - return new BufferIteratorResult(index, false); -}; + return new BufferIteratorResult(index, false); + }; -function BufferValueIterator(buffer) { - BufferIterator.call(this, buffer, null); -} + function BufferValueIterator(buffer) { + BufferIterator.call(this, buffer, null); + } -BufferValueIterator.prototype.next = function() { - var buffer = this._buffer; - var index = this._index; + BufferValueIterator.prototype.next = function() { + var buffer = this._buffer; + var index = this._index; - if (index >= buffer.length) - return finalResult; + if (index >= buffer.length) + return finalResult; - this._index++; + this._index++; - return resultCache[buffer[index]]; -}; + return resultCache[buffer[index]]; + }; -BufferIterator.prototype[Symbol.iterator] = function() { - return this; -}; + BufferIterator.prototype[Symbol.iterator] = function() { + return this; + }; -BufferValueIterator.prototype[Symbol.iterator] = - BufferIterator.prototype[Symbol.iterator]; + BufferValueIterator.prototype[Symbol.iterator] = + BufferIterator.prototype[Symbol.iterator]; -Buffer.prototype.keys = function() { - return new BufferIterator(this, ITERATOR_KIND_KEYS); -}; + Buffer.prototype.keys = function() { + return new BufferIterator(this, ITERATOR_KIND_KEYS); + }; -Buffer.prototype.entries = function() { - return new BufferIterator(this, ITERATOR_KIND_ENTRIES); -}; + Buffer.prototype.entries = function() { + return new BufferIterator(this, ITERATOR_KIND_ENTRIES); + }; -Buffer.prototype.values = function() { - return new BufferValueIterator(this); -}; + Buffer.prototype.values = function() { + return new BufferValueIterator(this); + }; -Buffer.prototype[Symbol.iterator] = Buffer.prototype.values; + Buffer.prototype[Symbol.iterator] = Buffer.prototype.values; +} diff --git a/src/env.h b/src/env.h index b53ff87fb13114..d791efef85f595 100644 --- a/src/env.h +++ b/src/env.h @@ -231,6 +231,7 @@ namespace node { V(async_hooks_post_function, v8::Function) \ V(binding_cache_object, v8::Object) \ V(buffer_constructor_function, v8::Function) \ + V(buffer_prototype_object, v8::Object) \ V(context, v8::Context) \ V(domain_array, v8::Array) \ V(fs_stats_constructor_function, v8::Function) \ diff --git a/src/node.cc b/src/node.cc index f47dd722056a8d..3d0f7e4e42ee17 100644 --- a/src/node.cc +++ b/src/node.cc @@ -5,6 +5,7 @@ #include "node_http_parser.h" #include "node_javascript.h" #include "node_version.h" +#include "node_internals.h" #if defined HAVE_PERFCTR #include "node_counters.h" @@ -146,6 +147,8 @@ static uv_async_t dispatch_debug_messages_async; static Isolate* node_isolate = nullptr; static v8::Platform* default_platform; +bool using_old_buffer = false; + class ArrayBufferAllocator : public ArrayBuffer::Allocator { public: // Impose an upper limit to avoid out of memory errors that bring down @@ -165,23 +168,21 @@ ArrayBufferAllocator ArrayBufferAllocator::the_singleton; void* ArrayBufferAllocator::Allocate(size_t length) { - if (length > kMaxLength) + void* data = malloc(length); + if (data == nullptr) return nullptr; - char* data = new char[length]; memset(data, 0, length); return data; } void* ArrayBufferAllocator::AllocateUninitialized(size_t length) { - if (length > kMaxLength) - return nullptr; - return new char[length]; + return malloc(length); } void ArrayBufferAllocator::Free(void* data, size_t length) { - delete[] static_cast(data); + free(data); } @@ -2844,6 +2845,11 @@ void SetupProcessObject(Environment* env, // after LoadEnvironment() has run. } + // --use-old_buffer + if (using_old_buffer) { + READONLY_PROPERTY(process, "useOldBuffer", True(env->isolate())); + } + size_t exec_path_len = 2 * PATH_MAX; char* exec_path = new char[exec_path_len]; Local exec_path_value; @@ -3072,6 +3078,7 @@ static void PrintHelp() { " --trace-deprecation show stack traces on deprecations\n" " --trace-sync-io show stack trace when use of sync IO\n" " is detected after the first tick\n" + " --use-old-buffer Revert to old Buffer implementation\n" " --v8-options print v8 command line options\n" #if defined(NODE_HAVE_I18N_SUPPORT) " --icu-data-dir=dir set ICU data load path to dir\n" @@ -3208,6 +3215,9 @@ static void ParseArgs(int* argc, #endif } else if (strcmp(arg, "--expose-internals") == 0 || strcmp(arg, "--expose_internals") == 0) { + } else if (strcmp(arg, "--use-old-buffer") == 0) { + using_old_buffer = true; + // consumed in js } else { // V8 option. Pass through as-is. diff --git a/src/node_buffer.cc b/src/node_buffer.cc index c4c1b5b9ab89de..987917611b08b5 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -22,6 +22,16 @@ size_t name##_length; \ char* name##_data; +#define SPREAD_ARG(val, name) \ + CHECK((val)->IsUint8Array()); \ + Local name = (val).As(); \ + ArrayBuffer::Contents name##_c = name->Buffer()->GetContents(); \ + size_t name##_offset = name->ByteOffset(); \ + name##_length = name->ByteLength(); \ + name##_data = static_cast(name##_c.Data()) + name##_offset; \ + if (name##_length > 0) \ + CHECK_NE(name##_data, nullptr); + #define ARGS_THIS(argT, name) \ Local name = argT; \ name##_length = name->GetIndexedPropertiesExternalArrayDataLength(); \ @@ -42,6 +52,8 @@ namespace node { namespace Buffer { +using v8::ArrayBuffer; +using v8::ArrayBufferCreationMode; using v8::Context; using v8::EscapableHandleScope; using v8::Function; @@ -55,19 +67,34 @@ using v8::Number; using v8::Object; using v8::String; using v8::Uint32; +using v8::Uint8Array; using v8::Value; bool HasInstance(Handle val) { - return val->IsObject() && HasInstance(val.As()); + if (using_old_buffer) { + return val->IsObject() && HasInstance(val.As()); + } + + Environment* env = Environment::GetCurrent(Isolate::GetCurrent()); + return val->IsUint8Array() && + val.As()->GetPrototype()->StrictEquals( + env->buffer_prototype_object()); } bool HasInstance(Handle obj) { - if (!obj->HasIndexedPropertiesInExternalArrayData()) - return false; - v8::ExternalArrayType type = obj->GetIndexedPropertiesExternalArrayDataType(); - return type == v8::kExternalUint8Array; + if (using_old_buffer) { + if (!obj->HasIndexedPropertiesInExternalArrayData()) + return false; + v8::ExternalArrayType type = + obj->GetIndexedPropertiesExternalArrayDataType(); + return type == v8::kExternalUint8Array; + } + + Environment* env = Environment::GetCurrent(Isolate::GetCurrent()); + return obj->IsUint8Array() && + obj->GetPrototype()->StrictEquals(env->buffer_prototype_object()); } @@ -80,8 +107,15 @@ char* Data(Handle val) { char* Data(Handle obj) { - CHECK(obj->HasIndexedPropertiesInExternalArrayData()); - return static_cast(obj->GetIndexedPropertiesExternalArrayData()); + if (using_old_buffer) { + CHECK(obj->HasIndexedPropertiesInExternalArrayData()); + return static_cast(obj->GetIndexedPropertiesExternalArrayData()); + } + + CHECK(obj->IsUint8Array()); + Local ui = obj.As(); + ArrayBuffer::Contents ab_c = ui->Buffer()->GetContents(); + return static_cast(ab_c.Data()) + ui->ByteOffset(); } @@ -92,8 +126,14 @@ size_t Length(Handle val) { size_t Length(Handle obj) { - CHECK(obj->HasIndexedPropertiesInExternalArrayData()); - return obj->GetIndexedPropertiesExternalArrayDataLength(); + if (using_old_buffer) { + CHECK(obj->HasIndexedPropertiesInExternalArrayData()); + return obj->GetIndexedPropertiesExternalArrayDataLength(); + } + + CHECK(obj->IsUint8Array()); + Local ui = obj.As(); + return ui->ByteLength(); } @@ -101,11 +141,18 @@ Local New(Isolate* isolate, Handle string, enum encoding enc) { EscapableHandleScope scope(isolate); size_t length = StringBytes::Size(isolate, string, enc); + char* data = static_cast(malloc(length)); + + if (data == nullptr) + return scope.Escape(Local()); + + size_t actual = StringBytes::Write(isolate, data, length, string, enc); + CHECK(actual <= length); - Local buf = New(isolate, length); - char* data = Buffer::Data(buf); - StringBytes::Write(isolate, data, length, string, enc); + if (actual < length) + data = static_cast(realloc(data, actual)); + Local buf = Use(isolate, data, actual); return scope.Escape(buf); } @@ -117,19 +164,43 @@ Local New(Isolate* isolate, size_t length) { } -// TODO(trevnorris): these have a flaw by needing to call the Buffer inst then -// Alloc. continue to look for a better architecture. Local New(Environment* env, size_t length) { EscapableHandleScope scope(env->isolate()); - CHECK_LE(length, kMaxLength); + if (using_old_buffer) { + CHECK_LE(length, kMaxLength); - Local arg = Uint32::NewFromUnsigned(env->isolate(), length); - Local obj = env->buffer_constructor_function()->NewInstance(1, &arg); + Local arg = Uint32::NewFromUnsigned(env->isolate(), length); + Local obj = + env->buffer_constructor_function()->NewInstance(1, &arg); - smalloc::Alloc(env, obj, length); + smalloc::Alloc(env, obj, length); - return scope.Escape(obj); + return scope.Escape(obj); + } + + if (!v8::internal::Internals::IsValidSmi(length)) { + return scope.Escape(Local()); + } + + void* data; + if (length > 0) { + data = malloc(length); + // NOTE: API change. Must check .IsEmpty() on the return object to see if + // the data was able to be allocated. + if (data == nullptr) + return scope.Escape(Local()); + } else { + data = nullptr; + } + Local ab = + ArrayBuffer::New(env->isolate(), + data, + length, + ArrayBufferCreationMode::kInternalized); + Local ui = Uint8Array::New(ab, 0, length); + ui->SetPrototype(env->buffer_prototype_object()); + return scope.Escape(ui); } @@ -141,33 +212,58 @@ Local New(Isolate* isolate, const char* data, size_t length) { } -// TODO(trevnorris): for backwards compatibility this is left to copy the data, -// but for consistency w/ the other should use data. And a copy version renamed -// to something else. +// Make a copy of "data". Why this isn't called "Copy", we'll never know. Local New(Environment* env, const char* data, size_t length) { EscapableHandleScope scope(env->isolate()); - CHECK_LE(length, kMaxLength); + if (using_old_buffer) { + CHECK_LE(length, kMaxLength); + + Local arg = Uint32::NewFromUnsigned(env->isolate(), length); + Local obj = + env->buffer_constructor_function()->NewInstance(1, &arg); + + char* new_data; + if (length > 0) { + new_data = static_cast(malloc(length)); + if (new_data == nullptr) + FatalError("node::Buffer::New(const char*, size_t)", "Out Of Memory"); + memcpy(new_data, data, length); + } else { + new_data = nullptr; + } - Local arg = Uint32::NewFromUnsigned(env->isolate(), length); - Local obj = env->buffer_constructor_function()->NewInstance(1, &arg); + smalloc::Alloc(env, obj, new_data, length); + + return scope.Escape(obj); + } - // TODO(trevnorris): done like this to handle HasInstance since only checks - // if external array data has been set, but would like to use a better - // approach if v8 provided one. - char* new_data; + if (!v8::internal::Internals::IsValidSmi(length)) { + return scope.Escape(Local()); + } + + void* new_data; if (length > 0) { - new_data = static_cast(malloc(length)); + CHECK_NE(data, nullptr); + new_data = malloc(length); + // NOTE: API change. Must check .IsEmpty() on the return object to see if + // the data was able to be allocated. if (new_data == nullptr) - FatalError("node::Buffer::New(const char*, size_t)", "Out Of Memory"); + return scope.Escape(Local()); memcpy(new_data, data, length); } else { new_data = nullptr; } - smalloc::Alloc(env, obj, new_data, length); + Local ab = + ArrayBuffer::New(env->isolate(), + new_data, + length, + ArrayBufferCreationMode::kInternalized); + Local ui = Uint8Array::New(ab, 0, length); + ui->SetPrototype(env->buffer_prototype_object()); - return scope.Escape(obj); + return scope.Escape(ui); } @@ -190,6 +286,7 @@ Local New(Environment* env, void* hint) { EscapableHandleScope scope(env->isolate()); + // TODO(trevnorris): IMPLEMENT CHECK_LE(length, kMaxLength); Local arg = Uint32::NewFromUnsigned(env->isolate(), length); @@ -201,7 +298,7 @@ Local New(Environment* env, } -Local Use(Isolate* isolate, char* data, uint32_t length) { +Local Use(Isolate* isolate, char* data, size_t length) { Environment* env = Environment::GetCurrent(isolate); EscapableHandleScope handle_scope(env->isolate()); Local obj = Buffer::Use(env, data, length); @@ -209,17 +306,98 @@ Local Use(Isolate* isolate, char* data, uint32_t length) { } -Local Use(Environment* env, char* data, uint32_t length) { +Local Use(Environment* env, char* data, size_t length) { EscapableHandleScope scope(env->isolate()); - CHECK_LE(length, kMaxLength); + if (using_old_buffer) { + CHECK_LE(length, kMaxLength); - Local arg = Uint32::NewFromUnsigned(env->isolate(), length); - Local obj = env->buffer_constructor_function()->NewInstance(1, &arg); + Local arg = Uint32::NewFromUnsigned(env->isolate(), length); + Local obj = + env->buffer_constructor_function()->NewInstance(1, &arg); - smalloc::Alloc(env, obj, data, length); + smalloc::Alloc(env, obj, data, length); - return scope.Escape(obj); + return scope.Escape(obj); + } + + if (length > 0) { + CHECK_NE(data, nullptr); + CHECK(v8::internal::Internals::IsValidSmi(length)); + } + + Local ab = + ArrayBuffer::New(env->isolate(), + data, + length, + ArrayBufferCreationMode::kInternalized); + Local ui = Uint8Array::New(ab, 0, length); + ui->SetPrototype(env->buffer_prototype_object()); + return scope.Escape(ui); +} + + +void Create(const FunctionCallbackInfo& args) { + Isolate* isolate = args.GetIsolate(); + Environment* env = Environment::GetCurrent(isolate); + + CHECK(args[0]->IsNumber()); + + size_t length = args[0]->IntegerValue(); + void* data; + + if (!v8::internal::Internals::IsValidSmi(length)) { + return env->ThrowRangeError("invalid Buffer length"); + } + + if (length > 0) { + data = malloc(length); + if (data == nullptr) + return env->ThrowRangeError("invalid Buffer length"); + } else { + data = nullptr; + } + + Local ab = + ArrayBuffer::New(isolate, + data, + length, + ArrayBufferCreationMode::kInternalized); + Local ui = Uint8Array::New(ab, 0, length); + ui->SetPrototype(env->buffer_prototype_object()); + args.GetReturnValue().Set(ui); +} + + +void CreateFromString(const FunctionCallbackInfo& args) { + CHECK(args[0]->IsString()); + CHECK(args[1]->IsString()); + + enum encoding enc = ParseEncoding(args.GetIsolate(), + args[1].As(), + UTF8); + Local buf = New(args.GetIsolate(), args[0].As(), enc); + args.GetReturnValue().Set(buf); +} + + +void Slice(const FunctionCallbackInfo& args) { + CHECK(args[0]->IsUint8Array()); + CHECK(args[1]->IsNumber()); + CHECK(args[2]->IsNumber()); + Environment* env = Environment::GetCurrent(args.GetIsolate()); + Local ab_ui = args[0].As(); + Local ab = ab_ui->Buffer(); + ArrayBuffer::Contents ab_c = ab->GetContents(); + size_t offset = ab_ui->ByteOffset(); + size_t start = args[1]->NumberValue() + offset; + size_t end = args[2]->NumberValue() + offset; + CHECK_GE(end, start); + size_t size = end - start; + CHECK_GE(ab_c.ByteLength(), start + size); + Local ui = Uint8Array::New(ab, start, size); + ui->SetPrototype(env->buffer_prototype_object()); + args.GetReturnValue().Set(ui); } @@ -229,7 +407,12 @@ void StringSlice(const FunctionCallbackInfo& args) { Isolate* isolate = env->isolate(); ARGS_THIS_DEC(ts_obj); - ARGS_THIS(args.This(), ts_obj); + if (using_old_buffer) { + ARGS_THIS(args.This(), ts_obj); + } else { + SPREAD_ARG(args.This(), ts_obj); + } + SLICE_START_END(args[0], args[1], ts_obj_length) args.GetReturnValue().Set( @@ -242,7 +425,12 @@ void StringSlice(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); ARGS_THIS_DEC(ts_obj); - ARGS_THIS(args.This(), ts_obj); + if (using_old_buffer) { + ARGS_THIS(args.This(), ts_obj); + } else { + SPREAD_ARG(args.This(), ts_obj); + } + SLICE_START_END(args[0], args[1], ts_obj_length) length /= 2; @@ -313,13 +501,20 @@ void Copy(const FunctionCallbackInfo &args) { if (!HasInstance(args[0])) return env->ThrowTypeError("first arg should be a Buffer"); - Local target = args[0]->ToObject(env->isolate()); + Local target_obj = args[0]->ToObject(env->isolate()); ARGS_THIS_DEC(ts_obj); + ARGS_THIS_DEC(target); + + if (using_old_buffer) { + ARGS_THIS(args.This(), ts_obj); + target_length = target_obj->GetIndexedPropertiesExternalArrayDataLength(); + target_data = static_cast( + target_obj->GetIndexedPropertiesExternalArrayData()); + } else { + SPREAD_ARG(args.This(), ts_obj); + SPREAD_ARG(target_obj, target); + } - ARGS_THIS(args.This(), ts_obj); - size_t target_length = target->GetIndexedPropertiesExternalArrayDataLength(); - char* target_data = static_cast( - target->GetIndexedPropertiesExternalArrayData()); size_t target_start; size_t source_start; size_t source_end; @@ -349,7 +544,12 @@ void Copy(const FunctionCallbackInfo &args) { void Fill(const FunctionCallbackInfo& args) { ARGS_THIS_DEC(ts_obj); - ARGS_THIS(args[0].As(), ts_obj); + + if (using_old_buffer) { + ARGS_THIS(args[0].As(), ts_obj); + } else { + SPREAD_ARG(args[0], ts_obj); + } size_t start = args[2]->Uint32Value(); size_t end = args[3]->Uint32Value(); @@ -393,7 +593,11 @@ void StringWrite(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); ARGS_THIS_DEC(ts_obj); - ARGS_THIS(args.This(), ts_obj); + if (using_old_buffer) { + ARGS_THIS(args.This(), ts_obj); + } else { + SPREAD_ARG(args.This(), ts_obj); + } if (!args[0]->IsString()) return env->ThrowTypeError("Argument must be a string"); @@ -470,7 +674,12 @@ static inline void Swizzle(char* start, unsigned int len) { template void ReadFloatGeneric(const FunctionCallbackInfo& args) { ARGS_THIS_DEC(ts_obj); - ARGS_THIS(args[0].As(), ts_obj); + + if (using_old_buffer) { + ARGS_THIS(args[0].As(), ts_obj); + } else { + SPREAD_ARG(args[0], ts_obj); + } uint32_t offset = args[1]->Uint32Value(); CHECK_LE(offset + sizeof(T), ts_obj_length); @@ -513,7 +722,12 @@ void ReadDoubleBE(const FunctionCallbackInfo& args) { template uint32_t WriteFloatGeneric(const FunctionCallbackInfo& args) { ARGS_THIS_DEC(ts_obj); - ARGS_THIS(args[0].As(), ts_obj); + + if (using_old_buffer) { + ARGS_THIS(args[0].As(), ts_obj); + } else { + SPREAD_ARG(args[0], ts_obj); + } T val = args[1]->NumberValue(); uint32_t offset = args[2]->Uint32Value(); @@ -562,26 +776,27 @@ void ByteLengthUtf8(const FunctionCallbackInfo &args) { void Compare(const FunctionCallbackInfo &args) { - Local obj_a = args[0].As(); - char* obj_a_data = - static_cast(obj_a->GetIndexedPropertiesExternalArrayData()); - size_t obj_a_len = obj_a->GetIndexedPropertiesExternalArrayDataLength(); + ARGS_THIS_DEC(obj_a); + ARGS_THIS_DEC(obj_b); - Local obj_b = args[1].As(); - char* obj_b_data = - static_cast(obj_b->GetIndexedPropertiesExternalArrayData()); - size_t obj_b_len = obj_b->GetIndexedPropertiesExternalArrayDataLength(); + if (using_old_buffer) { + ARGS_THIS(args[0].As(), obj_a); + ARGS_THIS(args[1].As(), obj_b); + } else { + SPREAD_ARG(args[0], obj_a); + SPREAD_ARG(args[1], obj_b); + } - size_t cmp_length = MIN(obj_a_len, obj_b_len); + size_t cmp_length = MIN(obj_a_length, obj_b_length); int32_t val = memcmp(obj_a_data, obj_b_data, cmp_length); // Normalize val to be an integer in the range of [1, -1] since // implementations of memcmp() can vary by platform. if (val == 0) { - if (obj_a_len > obj_b_len) + if (obj_a_length > obj_b_length) val = 1; - else if (obj_a_len < obj_b_len) + else if (obj_a_length < obj_b_length) val = -1; } else { if (val > 0) @@ -616,7 +831,13 @@ void IndexOfString(const FunctionCallbackInfo& args) { ASSERT(args[2]->IsNumber()); ARGS_THIS_DEC(ts_obj); - ARGS_THIS(args[0].As(), ts_obj); + + if (using_old_buffer) { + ARGS_THIS(args[0].As(), ts_obj); + } else { + SPREAD_ARG(args[0], ts_obj); + } + node::Utf8Value str(args.GetIsolate(), args[1]); int32_t offset_i32 = args[2]->Int32Value(); uint32_t offset; @@ -648,7 +869,13 @@ void IndexOfBuffer(const FunctionCallbackInfo& args) { ASSERT(args[2]->IsNumber()); ARGS_THIS_DEC(ts_obj); - ARGS_THIS(args[0].As(), ts_obj); + + if (using_old_buffer) { + ARGS_THIS(args[0].As(), ts_obj); + } else { + SPREAD_ARG(args[0], ts_obj); + } + Local buf = args[1].As(); int32_t offset_i32 = args[2]->Int32Value(); size_t buf_length = buf->GetIndexedPropertiesExternalArrayDataLength(); @@ -686,7 +913,13 @@ void IndexOfNumber(const FunctionCallbackInfo& args) { ASSERT(args[2]->IsNumber()); ARGS_THIS_DEC(ts_obj); - ARGS_THIS(args[0].As(), ts_obj); + + if (using_old_buffer) { + ARGS_THIS(args[0].As(), ts_obj); + } else { + SPREAD_ARG(args[0], ts_obj); + } + uint32_t needle = args[1]->Uint32Value(); int32_t offset_i32 = args[2]->Int32Value(); uint32_t offset; @@ -714,15 +947,20 @@ void IndexOfNumber(const FunctionCallbackInfo& args) { void SetupBufferJS(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - CHECK(args[0]->IsFunction()); - - Local bv = args[0].As(); - env->set_buffer_constructor_function(bv); - Local proto_v = bv->Get(env->prototype_string()); - - CHECK(proto_v->IsObject()); + Local proto; - Local proto = proto_v.As(); + if (using_old_buffer) { + CHECK(args[0]->IsFunction()); + Local bv = args[0].As(); + env->set_buffer_constructor_function(bv); + Local proto_v = bv->Get(env->prototype_string()); + CHECK(proto_v->IsObject()); + proto = proto_v.As(); + } else { + CHECK(args[0]->IsObject()); + proto = args[0].As(); + env->set_buffer_prototype_object(proto); + } env->SetMethod(proto, "asciiSlice", AsciiSlice); env->SetMethod(proto, "base64Slice", Base64Slice); @@ -741,9 +979,11 @@ void SetupBufferJS(const FunctionCallbackInfo& args) { env->SetMethod(proto, "copy", Copy); // for backwards compatibility - proto->ForceSet(env->offset_string(), - Uint32::New(env->isolate(), 0), - v8::ReadOnly); + if (using_old_buffer) { + proto->ForceSet(env->offset_string(), + Uint32::New(env->isolate(), 0), + v8::ReadOnly); + } } @@ -753,7 +993,10 @@ void Initialize(Handle target, Environment* env = Environment::GetCurrent(context); env->SetMethod(target, "setupBufferJS", SetupBufferJS); + env->SetMethod(target, "create", Create); + env->SetMethod(target, "createFromString", CreateFromString); + env->SetMethod(target, "slice", Slice); env->SetMethod(target, "byteLengthUtf8", ByteLengthUtf8); env->SetMethod(target, "compare", Compare); env->SetMethod(target, "fill", Fill); diff --git a/src/node_buffer.h b/src/node_buffer.h index 2e649970c4793a..4b1b2cd8591ee1 100644 --- a/src/node_buffer.h +++ b/src/node_buffer.h @@ -63,9 +63,9 @@ NODE_DEPRECATED("Use New(isolate, ...)", // TODO(trevnorris): should be New() for consistency NODE_EXTERN v8::Local Use(v8::Isolate* isolate, char* data, - uint32_t len); + size_t len); NODE_DEPRECATED("Use Use(isolate, ...)", - inline v8::Local Use(char* data, uint32_t len) { + inline v8::Local Use(char* data, size_t len) { return Use(v8::Isolate::GetCurrent(), data, len); }) @@ -95,7 +95,7 @@ v8::Local New(Environment* env, size_t length, smalloc::FreeCallback callback, void* hint); -v8::Local Use(Environment* env, char* data, uint32_t length); +v8::Local Use(Environment* env, char* data, size_t length); #endif // defined(NODE_WANT_INTERNALS) } // namespace Buffer diff --git a/src/node_internals.h b/src/node_internals.h index c99b2feeb0bdcd..e5f7458f78d59f 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -17,6 +17,8 @@ namespace node { // Forward declaration class Environment; +extern bool using_old_buffer; + // If persistent.IsWeak() == false, then do not call persistent.Reset() // while the returned Local is still in scope, it will destroy the // reference to the object. diff --git a/test/parallel/test-buffer-slice.js b/test/parallel/test-buffer-slice.js deleted file mode 100644 index 53434eab8e3f8f..00000000000000 --- a/test/parallel/test-buffer-slice.js +++ /dev/null @@ -1,12 +0,0 @@ -'use strict'; -var common = require('../common'); -var assert = require('assert'); - -var Buffer = require('buffer').Buffer; - -var buff = new Buffer(Buffer.poolSize + 1); -var slicedBuffer = buff.slice(); -assert.equal(slicedBuffer.parent, - buff, - 'slicedBufffer should have its parent set to the original ' + - ' buffer'); diff --git a/test/parallel/test-buffer.js b/test/parallel/test-buffer.js index 1d02148734e38a..ff0d09a2facca8 100644 --- a/test/parallel/test-buffer.js +++ b/test/parallel/test-buffer.js @@ -314,8 +314,6 @@ assert.equal(b.parent, d.parent); var b = new SlowBuffer(5); var c = b.slice(0, 4); var d = c.slice(0, 2); -assert.equal(b, c.parent); -assert.equal(b, d.parent); @@ -1059,10 +1057,6 @@ assert.equal(buf.readInt8(0), -1); // try to slice a zero length Buffer // see https://github.com/joyent/node/issues/5881 SlowBuffer(0).slice(0, 1); - // make sure a zero length slice doesn't set the .parent attribute - assert.equal(Buffer(5).slice(0, 0).parent, undefined); - // and make sure a proper slice does have a parent - assert.ok(typeof Buffer(5).slice(0, 5).parent === 'object'); })(); // Regression test for #5482: should throw but not assert in C++ land. @@ -1089,11 +1083,11 @@ assert.throws(function() { assert.throws(function() { - new Buffer(smalloc.kMaxLength + 1); + new Buffer((-1 >>> 0) + 1); }, RangeError); assert.throws(function() { - new SlowBuffer(smalloc.kMaxLength + 1); + new SlowBuffer((-1 >>> 0) + 1); }, RangeError); if (common.hasCrypto) {