From 70cc5da0f11a024cf5be1ff20fd885556c1d2153 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 14 May 2018 17:30:27 +0200 Subject: [PATCH] lib,src: use V8 API for collection inspection Use a new public V8 API for inspecting weak collections and collection iterators, rather than using V8-internal functions to achieve this. This currently comes with a slight modification of the output for inspecting iterators generated by `Set().entries()`. Fixes: https://github.com/nodejs/node/issues/20409 PR-URL: https://github.com/nodejs/node/pull/20719 Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: Gus Caplan Reviewed-By: James M Snell Reviewed-By: Matteo Collina Reviewed-By: Ruben Bridgewater --- lib/console.js | 6 ++--- lib/internal/bootstrap/node.js | 17 ------------- lib/internal/v8.js | 39 ------------------------------ lib/util.js | 32 +++++++++--------------- node.gyp | 1 - src/node.cc | 6 ----- src/node_util.cc | 30 +++++++++++++++++++++++ test/parallel/test-util-inspect.js | 8 +++--- 8 files changed, 47 insertions(+), 92 deletions(-) delete mode 100644 lib/internal/v8.js diff --git a/lib/console.js b/lib/console.js index de2d11afe1e015..e8b89536b92a2b 100644 --- a/lib/console.js +++ b/lib/console.js @@ -29,7 +29,7 @@ const { ERR_INVALID_ARG_VALUE, }, } = require('internal/errors'); -const { previewMapIterator, previewSetIterator } = require('internal/v8'); +const { previewEntries } = process.binding('util'); const { Buffer: { isBuffer } } = require('buffer'); const util = require('util'); const { @@ -345,7 +345,7 @@ Console.prototype.table = function(tabularData, properties) { const mapIter = isMapIterator(tabularData); if (mapIter) - tabularData = previewMapIterator(tabularData); + tabularData = previewEntries(tabularData); if (mapIter || isMap(tabularData)) { const keys = []; @@ -367,7 +367,7 @@ Console.prototype.table = function(tabularData, properties) { const setIter = isSetIterator(tabularData); if (setIter) - tabularData = previewSetIterator(tabularData); + tabularData = previewEntries(tabularData); const setlike = setIter || isSet(tabularData); if (setlike) { diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index e23ec858f33cc8..a8485f5d1c2aad 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -29,7 +29,6 @@ // Do this good and early, since it handles errors. setupProcessFatal(); - setupV8(); setupProcessICUVersions(); setupGlobalVariables(); @@ -479,22 +478,6 @@ }; } - function setupV8() { - // Warm up the map and set iterator preview functions. V8 compiles - // functions lazily (unless --nolazy is set) so we need to do this - // before we turn off --allow_natives_syntax again. - const v8 = NativeModule.require('internal/v8'); - v8.previewMapIterator(new Map().entries()); - v8.previewSetIterator(new Set().entries()); - v8.previewWeakMap(new WeakMap(), 1); - v8.previewWeakSet(new WeakSet(), 1); - // Disable --allow_natives_syntax again unless it was explicitly - // specified on the command line. - const re = /^--allow[-_]natives[-_]syntax$/; - if (!process.execArgv.some((s) => re.test(s))) - process.binding('v8').setFlagsFromString('--noallow_natives_syntax'); - } - function setupProcessICUVersions() { const icu = process.binding('config').hasIntl ? process.binding('icu') : undefined; diff --git a/lib/internal/v8.js b/lib/internal/v8.js deleted file mode 100644 index 3102b45a2d4032..00000000000000 --- a/lib/internal/v8.js +++ /dev/null @@ -1,39 +0,0 @@ -'use strict'; - -// This file provides access to some of V8's native runtime functions. See -// https://github.com/v8/v8/wiki/Built-in-functions for further information -// about their implementation. -// They have to be loaded before anything else to make sure we deactivate them -// before executing any other code. Gaining access is achieved by using a -// specific flag that is used internally in the startup phase. - -// Clone the provided Map Iterator. -function previewMapIterator(it) { - return %MapIteratorClone(it); -} - -// Clone the provided Set Iterator. -function previewSetIterator(it) { - return %SetIteratorClone(it); -} - -// Retrieve all WeakMap instance key / value pairs up to `max`. `max` limits the -// number of key / value pairs returned. Make sure it is a positive number, -// otherwise V8 aborts. Passing through `0` returns all elements. -function previewWeakMap(weakMap, max) { - return %GetWeakMapEntries(weakMap, max); -} - -// Retrieve all WeakSet instance values up to `max`. `max` limits the -// number of key / value pairs returned. Make sure it is a positive number, -// otherwise V8 aborts. Passing through `0` returns all elements. -function previewWeakSet(weakSet, max) { - return %GetWeakSetValues(weakSet, max); -} - -module.exports = { - previewMapIterator, - previewSetIterator, - previewWeakMap, - previewWeakSet -}; diff --git a/lib/util.js b/lib/util.js index bd7a98694b91a9..f358adefdb25d6 100644 --- a/lib/util.js +++ b/lib/util.js @@ -30,18 +30,12 @@ const { const { TextDecoder, TextEncoder } = require('internal/encoding'); const { isBuffer } = require('buffer').Buffer; -const { - previewMapIterator, - previewSetIterator, - previewWeakMap, - previewWeakSet -} = require('internal/v8'); - const { getPromiseDetails, getProxyDetails, kPending, kRejected, + previewEntries } = process.binding('util'); const { internalBinding } = require('internal/bootstrap/loaders'); @@ -912,7 +906,7 @@ function formatMap(ctx, value, recurseTimes, keys) { function formatWeakSet(ctx, value, recurseTimes, keys) { const maxArrayLength = Math.max(ctx.maxArrayLength, 0); - const entries = previewWeakSet(value, maxArrayLength + 1); + const entries = previewEntries(value).slice(0, maxArrayLength + 1); const maxLength = Math.min(maxArrayLength, entries.length); let output = new Array(maxLength); for (var i = 0; i < maxLength; ++i) @@ -929,16 +923,14 @@ function formatWeakSet(ctx, value, recurseTimes, keys) { function formatWeakMap(ctx, value, recurseTimes, keys) { const maxArrayLength = Math.max(ctx.maxArrayLength, 0); - const entries = previewWeakMap(value, maxArrayLength + 1); - // Entries exist as [key1, val1, key2, val2, ...] - const remainder = entries.length / 2 > maxArrayLength; - const len = entries.length / 2 - (remainder ? 1 : 0); + const entries = previewEntries(value).slice(0, maxArrayLength + 1); + const remainder = entries.length > maxArrayLength; + const len = entries.length - (remainder ? 1 : 0); const maxLength = Math.min(maxArrayLength, len); let output = new Array(maxLength); for (var i = 0; i < len; i++) { - const pos = i * 2; - output[i] = `${formatValue(ctx, entries[pos], recurseTimes)} => ` + - formatValue(ctx, entries[pos + 1], recurseTimes); + output[i] = `${formatValue(ctx, entries[i][0], recurseTimes)} => ` + + formatValue(ctx, entries[i][1], recurseTimes); } // Sort all entries to have a halfway reliable output (if more entries than // retrieved ones exist, we can not reliably return the same output). @@ -950,9 +942,9 @@ function formatWeakMap(ctx, value, recurseTimes, keys) { return output; } -function formatCollectionIterator(preview, ctx, value, recurseTimes, keys) { +function formatCollectionIterator(ctx, value, recurseTimes, keys) { const output = []; - for (const entry of preview(value)) { + for (const entry of previewEntries(value)) { if (ctx.maxArrayLength === output.length) { output.push('... more items'); break; @@ -966,13 +958,11 @@ function formatCollectionIterator(preview, ctx, value, recurseTimes, keys) { } function formatMapIterator(ctx, value, recurseTimes, keys) { - return formatCollectionIterator(previewMapIterator, ctx, value, recurseTimes, - keys); + return formatCollectionIterator(ctx, value, recurseTimes, keys); } function formatSetIterator(ctx, value, recurseTimes, keys) { - return formatCollectionIterator(previewSetIterator, ctx, value, recurseTimes, - keys); + return formatCollectionIterator(ctx, value, recurseTimes, keys); } function formatPromise(ctx, value, recurseTimes, keys) { diff --git a/node.gyp b/node.gyp index d894fec7d92625..b4606ccc6f9cae 100644 --- a/node.gyp +++ b/node.gyp @@ -146,7 +146,6 @@ 'lib/internal/http2/core.js', 'lib/internal/http2/compat.js', 'lib/internal/http2/util.js', - 'lib/internal/v8.js', 'lib/internal/v8_prof_polyfill.js', 'lib/internal/v8_prof_processor.js', 'lib/internal/validators.js', diff --git a/src/node.cc b/src/node.cc index 47e5abec5047a4..960769663525a3 100644 --- a/src/node.cc +++ b/src/node.cc @@ -4355,12 +4355,6 @@ void Init(int* argc, } #endif - // Needed for access to V8 intrinsics. Disabled again during bootstrapping, - // see lib/internal/bootstrap/node.js. - const char allow_natives_syntax[] = "--allow_natives_syntax"; - V8::SetFlagsFromString(allow_natives_syntax, - sizeof(allow_natives_syntax) - 1); - // We should set node_is_initialized here instead of in node::Start, // otherwise embedders using node::Init to initialize everything will not be // able to set it and native modules will not load for them. diff --git a/src/node_util.cc b/src/node_util.cc index 662809fb851a98..2db68586459ab2 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -49,6 +49,35 @@ static void GetProxyDetails(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(ret); } +static void PreviewEntries(const FunctionCallbackInfo& args) { + if (!args[0]->IsObject()) + return; + + bool is_key_value; + Local entries; + if (!args[0].As()->PreviewEntries(&is_key_value).ToLocal(&entries)) + return; + if (!is_key_value) + return args.GetReturnValue().Set(entries); + + uint32_t length = entries->Length(); + CHECK_EQ(length % 2, 0); + + Environment* env = Environment::GetCurrent(args); + Local context = env->context(); + + Local pairs = Array::New(env->isolate(), length / 2); + for (uint32_t i = 0; i < length / 2; i++) { + Local pair = Array::New(env->isolate(), 2); + pair->Set(context, 0, entries->Get(context, i * 2).ToLocalChecked()) + .FromJust(); + pair->Set(context, 1, entries->Get(context, i * 2 + 1).ToLocalChecked()) + .FromJust(); + pairs->Set(context, i, pair).FromJust(); + } + args.GetReturnValue().Set(pairs); +} + // Side effect-free stringification that will never throw exceptions. static void SafeToString(const FunctionCallbackInfo& args) { auto context = args.GetIsolate()->GetCurrentContext(); @@ -188,6 +217,7 @@ void Initialize(Local target, env->SetMethod(target, "getPromiseDetails", GetPromiseDetails); env->SetMethod(target, "getProxyDetails", GetProxyDetails); env->SetMethod(target, "safeToString", SafeToString); + env->SetMethod(target, "previewEntries", PreviewEntries); env->SetMethod(target, "startSigintWatchdog", StartSigintWatchdog); env->SetMethod(target, "stopSigintWatchdog", StopSigintWatchdog); diff --git a/test/parallel/test-util-inspect.js b/test/parallel/test-util-inspect.js index 2b8e14e45f64e6..b80e932dd1c17f 100644 --- a/test/parallel/test-util-inspect.js +++ b/test/parallel/test-util-inspect.js @@ -19,14 +19,13 @@ // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE // USE OR OTHER DEALINGS IN THE SOFTWARE. -// Flags: --expose_internals 'use strict'; const common = require('../common'); const assert = require('assert'); const JSStream = process.binding('js_stream').JSStream; const util = require('util'); const vm = require('vm'); -const { previewMapIterator } = require('internal/v8'); +const { previewEntries } = process.binding('util'); assert.strictEqual(util.inspect(1), '1'); assert.strictEqual(util.inspect(false), 'false'); @@ -448,7 +447,7 @@ assert.strictEqual(util.inspect(-5e-324), '-5e-324'); { const map = new Map(); map.set(1, 2); - const vals = previewMapIterator(map.entries()); + const vals = previewEntries(map.entries()); const valsOutput = []; for (const o of vals) { valsOutput.push(o); @@ -935,8 +934,7 @@ if (typeof Symbol !== 'undefined') { const aSet = new Set([1, 3]); assert.strictEqual(util.inspect(aSet.keys()), '[Set Iterator] { 1, 3 }'); assert.strictEqual(util.inspect(aSet.values()), '[Set Iterator] { 1, 3 }'); - assert.strictEqual(util.inspect(aSet.entries()), - '[Set Iterator] { [ 1, 1 ], [ 3, 3 ] }'); + assert.strictEqual(util.inspect(aSet.entries()), '[Set Iterator] { 1, 3 }'); // Make sure the iterator doesn't get consumed. const keys = aSet.keys(); assert.strictEqual(util.inspect(keys), '[Set Iterator] { 1, 3 }');