Skip to content

Commit 1980442

Browse files
panvapanva
committed
lib: short-circuit WebIDL BufferSource SAB check
Avoid the C++ `isSharedArrayBuffer` binding call on the BufferSource and ArrayBufferView converter hot path by first checking whether the buffer's prototype chain contains `ArrayBuffer.prototype`. Every genuine (non-shared) ArrayBuffer is covered by the fast path; SharedArrayBuffers and prototype-tampered objects still fall through to the authoritative brand-checking binding, preserving the existing SharedArrayBuffer rejection semantics. While here, dispatch the view-kind (TypedArray vs DataView) from the primordial `%TypedArray%.prototype.@@toStringTag` getter directly, dropping the redundant `ArrayBufferIsView` check that `isDataView` would repeat at each call site. Signed-off-by: Filip Skokan <panva.ip@gmail.com>
1 parent a79e224 commit 1980442

File tree

3 files changed

+263
-6
lines changed

3 files changed

+263
-6
lines changed

‎benchmark/misc/webidl-buffer-source.js‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
'use strict';
2+
3+
const common = require('../common.js');
4+
5+
const bench = common.createBenchmark(main, {
6+
input: [
7+
'arraybuffer',
8+
'buffer',
9+
'dataview',
10+
'uint8array',
11+
'uint8clampedarray',
12+
'int8array',
13+
'uint16array',
14+
'int16array',
15+
'uint32array',
16+
'int32array',
17+
'float16array',
18+
'float32array',
19+
'float64array',
20+
'bigint64array',
21+
'biguint64array',
22+
],
23+
n: [1e6],
24+
}, { flags: ['--expose-internals'] });
25+
26+
function main({ n, input }) {
27+
const { converters } = require('internal/webidl');
28+
29+
let value;
30+
switch (input) {
31+
case 'arraybuffer': value = new ArrayBuffer(32); break;
32+
case 'buffer': value = Buffer.alloc(32); break;
33+
case 'dataview': value = new DataView(new ArrayBuffer(32)); break;
34+
case 'uint8array': value = new Uint8Array(32); break;
35+
case 'uint8clampedarray': value = new Uint8ClampedArray(32); break;
36+
case 'int8array': value = new Int8Array(32); break;
37+
case 'uint16array': value = new Uint16Array(16); break;
38+
case 'int16array': value = new Int16Array(16); break;
39+
case 'uint32array': value = new Uint32Array(8); break;
40+
case 'int32array': value = new Int32Array(8); break;
41+
case 'float16array': value = new Float16Array(16); break;
42+
case 'float32array': value = new Float32Array(8); break;
43+
case 'float64array': value = new Float64Array(4); break;
44+
case 'bigint64array': value = new BigInt64Array(4); break;
45+
case 'biguint64array': value = new BigUint64Array(4); break;
46+
}
47+
48+
const opts = { prefix: 'test', context: 'test' };
49+
50+
bench.start();
51+
for (let i = 0; i < n; i++)
52+
converters.BufferSource(value, opts);
53+
bench.end(n);
54+
}

‎lib/internal/webidl.js‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
const {
44
ArrayBufferIsView,
5+
ArrayBufferPrototype,
56
ArrayPrototypePush,
67
ArrayPrototypeToSorted,
78
DataViewPrototypeGetBuffer,
@@ -22,6 +23,7 @@ const {
2223
SymbolIterator,
2324
TypeError,
2425
TypedArrayPrototypeGetBuffer,
26+
TypedArrayPrototypeGetSymbolToStringTag,
2527
} = primordials;
2628

2729
const {
@@ -33,7 +35,6 @@ const {
3335
const { kEmptyObject } = require('internal/util');
3436
const {
3537
isArrayBuffer,
36-
isDataView,
3738
isSharedArrayBuffer,
3839
} = require('internal/util/types');
3940

@@ -390,9 +391,23 @@ function createInterfaceConverter(name, I) {
390391
};
391392
}
392393

393-
function getDataViewOrTypedArrayBuffer(V) {
394-
return isDataView(V) ?
395-
DataViewPrototypeGetBuffer(V) : TypedArrayPrototypeGetBuffer(V);
394+
// Returns the [[ViewedArrayBuffer]] of an ArrayBufferView without leaving JS.
395+
// `TypedArrayPrototypeGetSymbolToStringTag` returns the concrete TypedArray
396+
// name (e.g. 'Uint8Array') for any %TypedArray% instance, and `undefined`
397+
// for a DataView, so it doubles as a branch-free dispatch between the two
398+
// getters.
399+
function getViewedArrayBuffer(V) {
400+
return TypedArrayPrototypeGetSymbolToStringTag(V) !== undefined ?
401+
TypedArrayPrototypeGetBuffer(V) : DataViewPrototypeGetBuffer(V);
402+
}
403+
404+
// Returns `true` if `buffer` is a `SharedArrayBuffer`. The prototype-chain
405+
// check is a cheap V8 builtin that fast-accepts every non-tampered
406+
// `ArrayBuffer`; only buffers whose prototype chain has been detached or
407+
// reassigned (and real SABs) fall through to the authoritative brand check.
408+
function isSharedArrayBufferBacking(buffer) {
409+
return !ObjectPrototypeIsPrototypeOf(ArrayBufferPrototype, buffer) &&
410+
isSharedArrayBuffer(buffer);
396411
}
397412

398413
// https://webidl.spec.whatwg.org/#ArrayBufferView
@@ -402,7 +417,7 @@ converters.ArrayBufferView = (V, opts = kEmptyObject) => {
402417
'is not an ArrayBufferView.',
403418
opts);
404419
}
405-
if (isSharedArrayBuffer(getDataViewOrTypedArrayBuffer(V))) {
420+
if (isSharedArrayBufferBacking(getViewedArrayBuffer(V))) {
406421
throw makeException(
407422
'is a view on a SharedArrayBuffer, which is not allowed.',
408423
opts);
@@ -414,7 +429,7 @@ converters.ArrayBufferView = (V, opts = kEmptyObject) => {
414429
// https://webidl.spec.whatwg.org/#BufferSource
415430
converters.BufferSource = (V, opts = kEmptyObject) => {
416431
if (ArrayBufferIsView(V)) {
417-
if (isSharedArrayBuffer(getDataViewOrTypedArrayBuffer(V))) {
432+
if (isSharedArrayBufferBacking(getViewedArrayBuffer(V))) {
418433
throw makeException(
419434
'is a view on a SharedArrayBuffer, which is not allowed.',
420435
opts);

‎test/parallel/test-internal-webidl-buffer-source.js‎

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
// Flags: --expose-internals
2+
'use strict';
3+
4+
require('../common');
5+
const assert = require('assert');
6+
const { test } = require('node:test');
7+
8+
const { converters } = require('internal/webidl');
9+
10+
const TYPED_ARRAY_CTORS = [
11+
Uint8Array, Int8Array, Uint8ClampedArray,
12+
Uint16Array, Int16Array,
13+
Uint32Array, Int32Array,
14+
Float16Array, Float32Array, Float64Array,
15+
BigInt64Array, BigUint64Array,
16+
];
17+
18+
test('BufferSource accepts ArrayBuffer', () => {
19+
const ab = new ArrayBuffer(8);
20+
assert.strictEqual(converters.BufferSource(ab), ab);
21+
});
22+
23+
test('BufferSource accepts all TypedArray kinds', () => {
24+
for (const Ctor of TYPED_ARRAY_CTORS) {
25+
const ta = new Ctor(4);
26+
assert.strictEqual(converters.BufferSource(ta), ta);
27+
}
28+
});
29+
30+
test('BufferSource accepts Buffer', () => {
31+
const buf = Buffer.alloc(8);
32+
assert.strictEqual(converters.BufferSource(buf), buf);
33+
});
34+
35+
test('BufferSource accepts DataView', () => {
36+
const dv = new DataView(new ArrayBuffer(8));
37+
assert.strictEqual(converters.BufferSource(dv), dv);
38+
});
39+
40+
test('BufferSource accepts ArrayBuffer subclass instance', () => {
41+
class MyAB extends ArrayBuffer {}
42+
const sub = new MyAB(8);
43+
assert.strictEqual(converters.BufferSource(sub), sub);
44+
});
45+
46+
test('BufferSource accepts TypedArray with null prototype', () => {
47+
const ta = new Uint8Array(4);
48+
Object.setPrototypeOf(ta, null);
49+
assert.strictEqual(converters.BufferSource(ta), ta);
50+
});
51+
52+
test('BufferSource accepts DataView with null prototype', () => {
53+
const dv = new DataView(new ArrayBuffer(4));
54+
Object.setPrototypeOf(dv, null);
55+
assert.strictEqual(converters.BufferSource(dv), dv);
56+
});
57+
58+
test('BufferSource accepts ArrayBuffer with null prototype', () => {
59+
const ab = new ArrayBuffer(4);
60+
Object.setPrototypeOf(ab, null);
61+
assert.strictEqual(converters.BufferSource(ab), ab);
62+
});
63+
64+
test('BufferSource rejects SharedArrayBuffer', () => {
65+
assert.throws(
66+
() => converters.BufferSource(new SharedArrayBuffer(4)),
67+
{ code: 'ERR_INVALID_ARG_TYPE' },
68+
);
69+
});
70+
71+
test('BufferSource rejects SAB-backed TypedArray', () => {
72+
const view = new Uint8Array(new SharedArrayBuffer(4));
73+
assert.throws(
74+
() => converters.BufferSource(view),
75+
{ code: 'ERR_INVALID_ARG_TYPE' },
76+
);
77+
});
78+
79+
test('BufferSource rejects SAB-backed DataView', () => {
80+
const dv = new DataView(new SharedArrayBuffer(4));
81+
assert.throws(
82+
() => converters.BufferSource(dv),
83+
{ code: 'ERR_INVALID_ARG_TYPE' },
84+
);
85+
});
86+
87+
test('BufferSource accepts a detached ArrayBuffer', () => {
88+
const ab = new ArrayBuffer(8);
89+
structuredClone(ab, { transfer: [ab] });
90+
assert.strictEqual(ab.byteLength, 0);
91+
assert.strictEqual(converters.BufferSource(ab), ab);
92+
});
93+
94+
test('BufferSource rejects objects with a forged @@toStringTag', () => {
95+
const fake = { [Symbol.toStringTag]: 'Uint8Array' };
96+
assert.throws(
97+
() => converters.BufferSource(fake),
98+
{ code: 'ERR_INVALID_ARG_TYPE' },
99+
);
100+
});
101+
102+
for (const value of [null, undefined, 0, 1, 1n, '', 'x', true, Symbol('s'), [],
103+
{}, () => {}]) {
104+
test(`BufferSource rejects ${typeof value} ${String(value)}`, () => {
105+
assert.throws(
106+
() => converters.BufferSource(value),
107+
{ code: 'ERR_INVALID_ARG_TYPE' },
108+
);
109+
});
110+
}
111+
112+
test('ArrayBufferView accepts all TypedArray kinds', () => {
113+
for (const Ctor of TYPED_ARRAY_CTORS) {
114+
const ta = new Ctor(4);
115+
assert.strictEqual(converters.ArrayBufferView(ta), ta);
116+
}
117+
});
118+
119+
test('ArrayBufferView accepts DataView', () => {
120+
const dv = new DataView(new ArrayBuffer(8));
121+
assert.strictEqual(converters.ArrayBufferView(dv), dv);
122+
});
123+
124+
test('ArrayBufferView accepts TypedArray subclass instance', () => {
125+
class MyU8 extends Uint8Array {}
126+
const sub = new MyU8(4);
127+
assert.strictEqual(converters.ArrayBufferView(sub), sub);
128+
});
129+
130+
test('ArrayBufferView accepts TypedArray with null prototype', () => {
131+
const ta = new Uint8Array(4);
132+
Object.setPrototypeOf(ta, null);
133+
assert.strictEqual(converters.ArrayBufferView(ta), ta);
134+
});
135+
136+
test('ArrayBufferView accepts DataView with null prototype', () => {
137+
const dv = new DataView(new ArrayBuffer(4));
138+
Object.setPrototypeOf(dv, null);
139+
assert.strictEqual(converters.ArrayBufferView(dv), dv);
140+
});
141+
142+
test('ArrayBufferView rejects raw ArrayBuffer', () => {
143+
assert.throws(
144+
() => converters.ArrayBufferView(new ArrayBuffer(4)),
145+
{ code: 'ERR_INVALID_ARG_TYPE' },
146+
);
147+
});
148+
149+
test('ArrayBufferView rejects raw SharedArrayBuffer', () => {
150+
assert.throws(
151+
() => converters.ArrayBufferView(new SharedArrayBuffer(4)),
152+
{ code: 'ERR_INVALID_ARG_TYPE' },
153+
);
154+
});
155+
156+
test('ArrayBufferView rejects SAB-backed TypedArray', () => {
157+
const view = new Uint8Array(new SharedArrayBuffer(4));
158+
assert.throws(
159+
() => converters.ArrayBufferView(view),
160+
{ code: 'ERR_INVALID_ARG_TYPE' },
161+
);
162+
});
163+
164+
test('ArrayBufferView rejects SAB-backed DataView', () => {
165+
const dv = new DataView(new SharedArrayBuffer(4));
166+
assert.throws(
167+
() => converters.ArrayBufferView(dv),
168+
{ code: 'ERR_INVALID_ARG_TYPE' },
169+
);
170+
});
171+
172+
test('ArrayBufferView rejects objects with a forged @@toStringTag', () => {
173+
const fake = { [Symbol.toStringTag]: 'Uint8Array' };
174+
assert.throws(
175+
() => converters.ArrayBufferView(fake),
176+
{ code: 'ERR_INVALID_ARG_TYPE' },
177+
);
178+
});
179+
180+
for (const value of [null, undefined, 0, 1, 1n, '', 'x', true, Symbol('s'), [],
181+
{}, () => {}]) {
182+
test(`ArrayBufferView rejects ${typeof value} ${String(value)}`, () => {
183+
assert.throws(
184+
() => converters.ArrayBufferView(value),
185+
{ code: 'ERR_INVALID_ARG_TYPE' },
186+
);
187+
});
188+
}

0 commit comments

Comments
 (0)