From 08ed2311a42f8b042d0042ab456ee114d19cc7a3 Mon Sep 17 00:00:00 2001 From: Vladimir Kurchatkin Date: Sun, 22 Feb 2015 21:07:57 +0300 Subject: [PATCH 1/4] smalloc: consistent error messages --- lib/smalloc.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/smalloc.js b/lib/smalloc.js index b6eb21fbc3298e..499372031985bc 100644 --- a/lib/smalloc.js +++ b/lib/smalloc.js @@ -54,9 +54,10 @@ function alloc(n, obj, type) { if (type < 1 || type > 9) throw new TypeError('unknown external array type: ' + type); if (Array.isArray(obj)) - throw new TypeError('Arrays are not supported'); + throw new TypeError('obj cannot be an array'); if (n > kMaxLength) - throw new RangeError('n > kMaxLength'); + throw new RangeError('Attempt to allocate array larger than maximum ' + + 'size: 0x' + kMaxLength.toString(16) + ' elements'); return smalloc.alloc(obj, n, type); } From 2c94f6e9110c7c470becd239c53991f43df3dc57 Mon Sep 17 00:00:00 2001 From: Vladimir Kurchatkin Date: Sun, 22 Feb 2015 21:33:38 +0300 Subject: [PATCH 2/4] smalloc: validate arguments in js --- lib/smalloc.js | 39 ++++++++++++++++++++++++++++++++++----- src/smalloc.cc | 17 +---------------- 2 files changed, 35 insertions(+), 21 deletions(-) diff --git a/lib/smalloc.js b/lib/smalloc.js index 499372031985bc..9ebbaf402d17df 100644 --- a/lib/smalloc.js +++ b/lib/smalloc.js @@ -5,9 +5,9 @@ const kMaxLength = smalloc.kMaxLength; const util = require('util'); exports.alloc = alloc; -exports.copyOnto = smalloc.copyOnto; +exports.copyOnto = copyOnto; exports.dispose = dispose; -exports.hasExternalData = smalloc.hasExternalData; +exports.hasExternalData = hasExternalData; // don't allow kMaxLength to accidentally be overwritten. it's a lot less // apparent when a primitive is accidentally changed. @@ -50,11 +50,18 @@ function alloc(n, obj, type) { throw new TypeError('obj must be an Object'); } + if (Array.isArray(obj)) + throw new TypeError('obj cannot be an array'); + if (obj instanceof Buffer) + throw new TypeError('obj cannot be a Buffer'); + if (smalloc.isTypedArray(obj)) + throw new TypeError('obj cannot be a typed array'); + if (smalloc.hasExternalData(obj)) + throw new TypeError('object already has external array data'); + // 1 == v8::kExternalUint8Array, 9 == v8::kExternalUint8ClampedArray if (type < 1 || type > 9) throw new TypeError('unknown external array type: ' + type); - if (Array.isArray(obj)) - throw new TypeError('obj cannot be an array'); if (n > kMaxLength) throw new RangeError('Attempt to allocate array larger than maximum ' + 'size: 0x' + kMaxLength.toString(16) + ' elements'); @@ -71,7 +78,29 @@ function dispose(obj) { if (smalloc.isTypedArray(obj)) throw new TypeError('obj cannot be a typed array'); if (!smalloc.hasExternalData(obj)) - throw new Error('obj has no external array data'); + throw new TypeError('obj has no external array data'); smalloc.dispose(obj); } + + +function copyOnto(source, sourceStart, dest, destStart, copyLength) { + if (util.isPrimitive(source)) + throw new TypeError('source must be an Object'); + if (util.isPrimitive(dest)) + throw new TypeError('dest must be an Object'); + if (!smalloc.hasExternalData(source)) + throw new TypeError('source has no external array data'); + if (!smalloc.hasExternalData(dest)) + throw new TypeError('dest has no external array data'); + + return smalloc.copyOnto(source, sourceStart, dest, destStart, copyLength); +} + + +function hasExternalData(obj) { + if (util.isPrimitive(obj)) + return false; + + return smalloc.hasExternalData(obj); +} diff --git a/src/smalloc.cc b/src/smalloc.cc index 89461959dd8912..eb343d2f793d9b 100644 --- a/src/smalloc.cc +++ b/src/smalloc.cc @@ -152,19 +152,9 @@ size_t ExternalArraySize(enum ExternalArrayType type) { void CopyOnto(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - if (!args[0]->IsObject()) - return env->ThrowTypeError("source must be an object"); - if (!args[2]->IsObject()) - return env->ThrowTypeError("dest must be an object"); - Local source = args[0].As(); Local dest = args[2].As(); - if (!source->HasIndexedPropertiesInExternalArrayData()) - return env->ThrowError("source has no external array data"); - if (!dest->HasIndexedPropertiesInExternalArrayData()) - return env->ThrowError("dest has no external array data"); - size_t source_start = args[1]->Uint32Value(); size_t dest_start = args[3]->Uint32Value(); size_t copy_length = args[4]->Uint32Value(); @@ -268,10 +258,6 @@ void Alloc(const FunctionCallbackInfo& args) { Local obj = args[0].As(); - // can't perform this check in JS - if (obj->HasIndexedPropertiesInExternalArrayData()) - return env->ThrowTypeError("object already has external array data"); - size_t length = args[1]->Uint32Value(); enum ExternalArrayType array_type; @@ -410,8 +396,7 @@ void Alloc(Environment* env, void HasExternalData(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - args.GetReturnValue().Set(args[0]->IsObject() && - HasExternalData(env, args[0].As())); + args.GetReturnValue().Set(HasExternalData(env, args[0].As())); } From 3acee6e0da678e5cb881fd2d4a673070ed3d9c06 Mon Sep 17 00:00:00 2001 From: Vladimir Kurchatkin Date: Sun, 22 Feb 2015 23:01:08 +0300 Subject: [PATCH 3/4] smalloc: export constants from C++ --- lib/smalloc.js | 23 ++++------------------- src/smalloc.cc | 34 ++++++++++++++++++++++++++++++++++ test/parallel/test-smalloc.js | 20 ++++++++++++++++++++ 3 files changed, 58 insertions(+), 19 deletions(-) diff --git a/lib/smalloc.js b/lib/smalloc.js index 9ebbaf402d17df..f181cb60cf2d7c 100644 --- a/lib/smalloc.js +++ b/lib/smalloc.js @@ -2,6 +2,8 @@ const smalloc = process.binding('smalloc'); const kMaxLength = smalloc.kMaxLength; +const kMinType = smalloc.kMinType; +const kMaxType = smalloc.kMaxType; const util = require('util'); exports.alloc = alloc; @@ -15,24 +17,8 @@ Object.defineProperty(exports, 'kMaxLength', { enumerable: true, value: kMaxLength, writable: false }); -// enumerated values for different external array types -var Types = {}; - -// Must match enum v8::ExternalArrayType. -Object.defineProperties(Types, { - 'Int8': { enumerable: true, value: 1, writable: false }, - 'Uint8': { enumerable: true, value: 2, writable: false }, - 'Int16': { enumerable: true, value: 3, writable: false }, - 'Uint16': { enumerable: true, value: 4, writable: false }, - 'Int32': { enumerable: true, value: 5, writable: false }, - 'Uint32': { enumerable: true, value: 6, writable: false }, - 'Float': { enumerable: true, value: 7, writable: false }, - 'Double': { enumerable: true, value: 8, writable: false }, - 'Uint8Clamped': { enumerable: true, value: 9, writable: false } -}); - Object.defineProperty(exports, 'Types', { - enumerable: true, value: Types, writable: false + enumerable: true, value: Object.freeze(smalloc.types), writable: false }); @@ -59,8 +45,7 @@ function alloc(n, obj, type) { if (smalloc.hasExternalData(obj)) throw new TypeError('object already has external array data'); - // 1 == v8::kExternalUint8Array, 9 == v8::kExternalUint8ClampedArray - if (type < 1 || type > 9) + if (type < kMinType || type > kMaxType) throw new TypeError('unknown external array type: ' + type); if (n > kMaxLength) throw new RangeError('Attempt to allocate array larger than maximum ' + diff --git a/src/smalloc.cc b/src/smalloc.cc index eb343d2f793d9b..e9de5cdbbea361 100644 --- a/src/smalloc.cc +++ b/src/smalloc.cc @@ -10,6 +10,20 @@ #include #define ALLOC_ID (0xA10C) +#define EXTERNAL_ARRAY_TYPES(V) \ + V(Int8, kExternalInt8Array) \ + V(Uint8, kExternalUint8Array) \ + V(Int16, kExternalInt16Array) \ + V(Uint16, kExternalUint16Array) \ + V(Int32, kExternalInt32Array) \ + V(Uint32, kExternalUint32Array) \ + V(Float, kExternalFloat32Array) \ + V(Double, kExternalFloat64Array) \ + V(Uint8Clamped, kExternalUint8ClampedArray) + +#define MIN(a, b) ((a) < (b) ? (a) : (b)) +#define MAX(a, b) ((a) > (b) ? (a) : (b)) + namespace node { namespace smalloc { @@ -548,6 +562,7 @@ void Initialize(Handle exports, Handle unused, Handle context) { Environment* env = Environment::GetCurrent(context); + Isolate* isolate = env->isolate(); env->SetMethod(exports, "copyOnto", CopyOnto); env->SetMethod(exports, "sliceOnto", SliceOnto); @@ -562,6 +577,25 @@ void Initialize(Handle exports, exports->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "kMaxLength"), Uint32::NewFromUnsigned(env->isolate(), kMaxLength)); + Local types = Object::New(isolate); + + uint32_t kMinType = ~0; + uint32_t kMaxType = 0; + #define V(name, value) \ + types->Set(FIXED_ONE_BYTE_STRING(env->isolate(), #name), \ + Uint32::NewFromUnsigned(env->isolate(), v8::value)); \ + kMinType = MIN(kMinType, v8::value); \ + kMaxType = MAX(kMinType, v8::value); + + EXTERNAL_ARRAY_TYPES(V) + #undef V + + exports->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "types"), types); + exports->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "kMinType"), + Uint32::NewFromUnsigned(env->isolate(), kMinType)); + exports->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "kMaxType"), + Uint32::NewFromUnsigned(env->isolate(), kMaxType)); + HeapProfiler* heap_profiler = env->isolate()->GetHeapProfiler(); heap_profiler->SetWrapperClassInfoProvider(ALLOC_ID, WrapperInfo); } diff --git a/test/parallel/test-smalloc.js b/test/parallel/test-smalloc.js index 61ffe3b84da88c..ef1aa60bad6838 100644 --- a/test/parallel/test-smalloc.js +++ b/test/parallel/test-smalloc.js @@ -309,3 +309,23 @@ assert.throws(function() { assert.throws(function() { smalloc.dispose({}); }); + + +// Types should be immutable +assert.deepStrictEqual(Object.getOwnPropertyDescriptor(smalloc, 'Types'), { + value: smalloc.Types, + writable: false, + enumerable: true, + configurable: false +}); + +var types = Object.keys(smalloc.Types); +var Types = smalloc.Types; + +for (var i = 0; i < types.length; i++) + assert.deepStrictEqual(Object.getOwnPropertyDescriptor(Types, types[i]), { + value: Types[types[i]], + writable: false, + enumerable: true, + configurable: false + }); From 1b3d36260d67abe939eab56baa3bfb0a94d6600e Mon Sep 17 00:00:00 2001 From: Vladimir Kurchatkin Date: Tue, 3 Mar 2015 17:38:00 +0300 Subject: [PATCH 4/4] smalloc: add asserts --- src/smalloc.cc | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/smalloc.cc b/src/smalloc.cc index e9de5cdbbea361..319c3937605404 100644 --- a/src/smalloc.cc +++ b/src/smalloc.cc @@ -166,9 +166,15 @@ size_t ExternalArraySize(enum ExternalArrayType type) { void CopyOnto(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); + ASSERT(args[0]->IsObject()); + ASSERT(args[2]->IsObject()); + Local source = args[0].As(); Local dest = args[2].As(); + ASSERT(source->HasIndexedPropertiesInExternalArrayData()); + ASSERT(dest->HasIndexedPropertiesInExternalArrayData()); + size_t source_start = args[1]->Uint32Value(); size_t dest_start = args[3]->Uint32Value(); size_t copy_length = args[4]->Uint32Value(); @@ -270,8 +276,12 @@ void SliceOnto(const FunctionCallbackInfo& args) { void Alloc(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); + ASSERT(args[0]->IsObject()); + Local obj = args[0].As(); + ASSERT(!obj->HasIndexedPropertiesInExternalArrayData()); + size_t length = args[1]->Uint32Value(); enum ExternalArrayType array_type; @@ -410,6 +420,7 @@ void Alloc(Environment* env, void HasExternalData(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); + ASSERT(args[0]->IsObject()); args.GetReturnValue().Set(HasExternalData(env, args[0].As())); }