Skip to content

Commit

Permalink
util: use private symbols in JS land directly
Browse files Browse the repository at this point in the history
Instead of calling into C++ to use the private symbols, use an
ObjectTemplate to create an object that holds the symbols and
use them directly from JS land.

PR-URL: #45379
Backport-PR-URL: #45674
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Signed-off-by: Daeyeon Jeong <[email protected]>
  • Loading branch information
joyeecheung authored and danielleadams committed Jan 3, 2023
1 parent 7e386f8 commit 3f18a83
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 94 deletions.
8 changes: 4 additions & 4 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,9 @@ const {
exposeInterface,
} = require('internal/util');
const {
exiting_aliased_Uint32Array,
getHiddenValue,
privateSymbols: {
exiting_aliased_Uint32Array,
},
} = internalBinding('util');

setupProcessObject();
Expand All @@ -88,8 +89,7 @@ setupBuffer();

process.domain = null;
{
const exitingAliasedUint32Array =
getHiddenValue(process, exiting_aliased_Uint32Array);
const exitingAliasedUint32Array = process[exiting_aliased_Uint32Array];
ObjectDefineProperty(process, '_exiting', {
__proto__: null,
get() {
Expand Down
8 changes: 5 additions & 3 deletions lib/internal/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,11 @@ const {
utf8Write,
getZeroFillToggle
} = internalBinding('buffer');

const {
untransferable_object_private_symbol,
setHiddenValue,
privateSymbols: {
untransferable_object_private_symbol,
},
} = internalBinding('util');

// Temporary buffers to convert numbers.
Expand Down Expand Up @@ -1048,7 +1050,7 @@ function addBufferPrototypeMethods(proto) {
function markAsUntransferable(obj) {
if ((typeof obj !== 'object' && typeof obj !== 'function') || obj === null)
return; // This object is a primitive and therefore already untransferable.
setHiddenValue(obj, untransferable_object_private_symbol, true);
obj[untransferable_object_private_symbol] = true;
}

// A toggle used to access the zero fill setting of the array buffer allocator
Expand Down
14 changes: 6 additions & 8 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -816,16 +816,14 @@ const fatalExceptionStackEnhancers = {
}
};

const {
privateSymbols: {
arrow_message_private_symbol,
}
} = internalBinding('util');
// Ensures the printed error line is from user code.
let _kArrowMessagePrivateSymbol, _setHiddenValue;
function setArrowMessage(err, arrowMessage) {
if (!_kArrowMessagePrivateSymbol) {
({
arrow_message_private_symbol: _kArrowMessagePrivateSymbol,
setHiddenValue: _setHiddenValue,
} = internalBinding('util'));
}
_setHiddenValue(err, _kArrowMessagePrivateSymbol, arrowMessage);
err[arrow_message_private_symbol] = arrowMessage;
}

// Hide stack lines before the first user code line.
Expand Down
15 changes: 7 additions & 8 deletions lib/internal/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ const {
} = require('internal/errors');
const { signals } = internalBinding('constants').os;
const {
getHiddenValue,
setHiddenValue,
arrow_message_private_symbol: kArrowMessagePrivateSymbolIndex,
decorated_private_symbol: kDecoratedPrivateSymbolIndex,
privateSymbols: {
arrow_message_private_symbol,
decorated_private_symbol,
},
sleep: _sleep,
toUSVString: _toUSVString,
} = internalBinding('util');
Expand Down Expand Up @@ -140,15 +140,14 @@ function deprecate(fn, msg, code) {
}

function decorateErrorStack(err) {
if (!(isError(err) && err.stack) ||
getHiddenValue(err, kDecoratedPrivateSymbolIndex) === true)
if (!(isError(err) && err.stack) || err[decorated_private_symbol])
return;

const arrow = getHiddenValue(err, kArrowMessagePrivateSymbolIndex);
const arrow = err[arrow_message_private_symbol];

if (arrow) {
err.stack = arrow + err.stack;
setHiddenValue(err, kDecoratedPrivateSymbolIndex, true);
err[decorated_private_symbol] = true;
}
}

Expand Down
61 changes: 12 additions & 49 deletions src/node_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ using v8::Object;
using v8::ONLY_CONFIGURABLE;
using v8::ONLY_ENUMERABLE;
using v8::ONLY_WRITABLE;
using v8::Private;
using v8::Promise;
using v8::PropertyFilter;
using v8::Proxy;
Expand Down Expand Up @@ -159,44 +158,6 @@ static void PreviewEntries(const FunctionCallbackInfo<Value>& args) {
Array::New(env->isolate(), ret, arraysize(ret)));
}

inline Local<Private> IndexToPrivateSymbol(Environment* env, uint32_t index) {
#define V(name, _) &Environment::name,
static Local<Private> (Environment::*const methods[])() const = {
PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(V)
};
#undef V
CHECK_LT(index, arraysize(methods));
return (env->*methods[index])();
}

static void GetHiddenValue(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

CHECK(args[0]->IsObject());
CHECK(args[1]->IsUint32());

Local<Object> obj = args[0].As<Object>();
uint32_t index = args[1].As<Uint32>()->Value();
Local<Private> private_symbol = IndexToPrivateSymbol(env, index);
Local<Value> ret;
if (obj->GetPrivate(env->context(), private_symbol).ToLocal(&ret))
args.GetReturnValue().Set(ret);
}

static void SetHiddenValue(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

CHECK(args[0]->IsObject());
CHECK(args[1]->IsUint32());

Local<Object> obj = args[0].As<Object>();
uint32_t index = args[1].As<Uint32>()->Value();
Local<Private> private_symbol = IndexToPrivateSymbol(env, index);
bool ret;
if (obj->SetPrivate(env->context(), private_symbol, args[2]).To(&ret))
args.GetReturnValue().Set(ret);
}

static void Sleep(const FunctionCallbackInfo<Value>& args) {
CHECK(args[0]->IsUint32());
uint32_t msec = args[0].As<Uint32>()->Value();
Expand Down Expand Up @@ -386,8 +347,6 @@ static void ToUSVString(const FunctionCallbackInfo<Value>& args) {
}

void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
registry->Register(GetHiddenValue);
registry->Register(SetHiddenValue);
registry->Register(GetPromiseDetails);
registry->Register(GetProxyDetails);
registry->Register(PreviewEntries);
Expand All @@ -412,16 +371,22 @@ void Initialize(Local<Object> target,
Environment* env = Environment::GetCurrent(context);
Isolate* isolate = env->isolate();

#define V(name, _) \
target->Set(context, \
FIXED_ONE_BYTE_STRING(env->isolate(), #name), \
Integer::NewFromUnsigned(env->isolate(), index++)).Check();
{
uint32_t index = 0;
Local<v8::ObjectTemplate> tmpl = v8::ObjectTemplate::New(isolate);
#define V(PropertyName, _) \
tmpl->Set(FIXED_ONE_BYTE_STRING(env->isolate(), #PropertyName), \
env->PropertyName());

PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(V)
}
#undef V

target
->Set(context,
FIXED_ONE_BYTE_STRING(isolate, "privateSymbols"),
tmpl->NewInstance(context).ToLocalChecked())
.Check();
}

#define V(name) \
target->Set(context, \
FIXED_ONE_BYTE_STRING(env->isolate(), #name), \
Expand All @@ -432,8 +397,6 @@ void Initialize(Local<Object> target,
V(kRejected);
#undef V

SetMethodNoSideEffect(context, target, "getHiddenValue", GetHiddenValue);
SetMethod(context, target, "setHiddenValue", SetHiddenValue);
SetMethodNoSideEffect(
context, target, "getPromiseDetails", GetPromiseDetails);
SetMethodNoSideEffect(context, target, "getProxyDetails", GetProxyDetails);
Expand Down
15 changes: 8 additions & 7 deletions test/parallel/test-internal-util-decorate-error-stack.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@ const fixtures = require('../common/fixtures');
const assert = require('assert');
const internalUtil = require('internal/util');
const { internalBinding } = require('internal/test/binding');
const binding = internalBinding('util');
const {
privateSymbols: {
arrow_message_private_symbol,
decorated_private_symbol,
}
} = internalBinding('util');
const spawnSync = require('child_process').spawnSync;

const kArrowMessagePrivateSymbolIndex = binding.arrow_message_private_symbol;
const kDecoratedPrivateSymbolIndex = binding.decorated_private_symbol;

const decorateErrorStack = internalUtil.decorateErrorStack;

// Verify that decorateErrorStack does not throw with non-objects.
Expand Down Expand Up @@ -73,9 +75,8 @@ const arrowMessage = 'arrow_message';
err = new Error('foo');
originalStack = err.stack;

binding.setHiddenValue(err, kArrowMessagePrivateSymbolIndex, arrowMessage);
err[arrow_message_private_symbol] = arrowMessage;
decorateErrorStack(err);

assert.strictEqual(err.stack, `${arrowMessage}${originalStack}`);
assert.strictEqual(
binding.getHiddenValue(err, kDecoratedPrivateSymbolIndex), true);
assert.strictEqual(err[decorated_private_symbol], true);
24 changes: 9 additions & 15 deletions test/parallel/test-util-internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,30 +7,24 @@ const fixtures = require('../common/fixtures');
const { internalBinding } = require('internal/test/binding');

const {
getHiddenValue,
setHiddenValue,
arrow_message_private_symbol: kArrowMessagePrivateSymbolIndex
privateSymbols: {
arrow_message_private_symbol,
},
} = internalBinding('util');

assert.strictEqual(
getHiddenValue({}, kArrowMessagePrivateSymbolIndex),
undefined);

const obj = {};
assert.strictEqual(
setHiddenValue(obj, kArrowMessagePrivateSymbolIndex, 'bar'),
true);
assert.strictEqual(
getHiddenValue(obj, kArrowMessagePrivateSymbolIndex),
'bar');
assert.strictEqual(obj[arrow_message_private_symbol], undefined);

obj[arrow_message_private_symbol] = 'bar';
assert.strictEqual(obj[arrow_message_private_symbol], 'bar');
assert.deepStrictEqual(Reflect.ownKeys(obj), []);

let arrowMessage;

try {
require(fixtures.path('syntax', 'bad_syntax'));
} catch (err) {
arrowMessage =
getHiddenValue(err, kArrowMessagePrivateSymbolIndex);
arrowMessage = err[arrow_message_private_symbol];
}

assert.match(arrowMessage, /bad_syntax\.js:1/);

0 comments on commit 3f18a83

Please sign in to comment.