[robobun]

Draft: needs oven-sh/WebKit#739 (pinned as a preview build) and a maintainer decision. Stacked on #44214.

Problem

• Since 1.4.0, Array.from.call(CString, [1, 2, 3]) and an array whose Symbol.species is bun:ffi CString abort the process: panic(main thread): abort() called, from RELEASE_ASSERT(baseValue.isObject()). 1.3.14 threw a TypeError or returned a String object.
• constructFFICString (src/jsc/bindings/JSFFICString.cpp) returns a string primitive by contract (bun:ffi: use the engine-native FFI when available #35246). The engine uses a construct result as an object.

Fix

• CString takes JSCell::getConstructData ([JSC] An InternalFunction subclass may keep a new handler and report no construct data WebKit#739), so the engine sees no constructor. new CString(ptr) does not ask, and returns the string.
• Array.of and Array.from return a plain array. Symbol.species, Reflect.construct, bind, Proxy and extends throw a TypeError.
• super() does not ask either, so the construct handler throws a TypeError when new.target is not CString.
• Verified: test/js/bun/ffi/ffi-error-messages.test.ts. With src/ at the base, 6 of the 9 new tests fail. Self-reviewed: 17 concerns raised, 14 addressed.

Background

• new on a native function jumps to its construct handler. Everything else reads the construct data of the function first.
• Considered a result check in Interpreter::executeConstruct. The handler has read the length as a pointer by then, and the Array.from builtin never gets there.

Downsides

• Needs a maintainer decision: Reflect.construct(CString, [ptr]) and new on a bound or proxied CString return the string today, and now throw.
• new CString(ptr): 401 to 404 instructions. Release text: +1,024 bytes.
• 79 other native functions still abort in Array.from.call(f, [1, 2, 3]) (Don't register native functions as their own constructors #34575, bun:test: mock functions match Jest for Proxy receivers, new, and mock.instances #41398). new CString(3) still reads an invalid pointer.

Notes

The decision to make. new CString(ptr) and CString(ptr) do not change. Every other way to construct CString changes, because IsConstructor(CString) is false after this PR. If the forms below must keep their result, this PR is the wrong tool.

form	1.3.14	1.4.0 to main, release	main, debug ASAN	this PR
new CString(ptr)	String object	"hello"	"hello"	"hello"
CString(ptr)	TypeError	"hello"	"hello"	"hello"
Reflect.construct(CString, [ptr])	String object	"hello"	"hello"	TypeError
Reflect.construct through an alias, rc(CString, [ptr])	String object	"hello"	ASSERTION FAILED: cell->isObjectSlow()	TypeError
new (CString.bind(null))(ptr)	String object	"hello"	ASSERTION FAILED: cell->isObjectSlow()	TypeError
new (new Proxy(CString, {}))(ptr)	String object	"hello"	ASSERTION FAILED: cell->isObjectSlow()	TypeError
class X extends CString {}	works	TypeError	TypeError	TypeError

The literal Reflect.construct(...) call does not assert, because the engine compiles it to bytecode that checks IsConstructor and then constructs like new. GitHub code search finds no file that imports bun:ffi and uses one of the changed forms, and 456 files that use new CString(. #25231 and #25257 show that new CString(ptr) itself must keep working.

Door matrix. 86 expressions, one process each, Linux x64.

build	exits with a crash	aborts	segfaults
1.3.14	16	0	16
1.4.0	28	12	16
main, release	28	12	16
main, debug ASAN	36	20	16
this PR, release	0	0	0
this PR, debug ASAN	0	0	0

On the debug ASAN build of main, the 20 aborts are ASSERTION FAILED: cell->isObjectSlow() and the 16 segfaults are AddressSanitizer reports. The release build and the debug ASAN build of this PR give the same result for each of the 86 expressions.

The 12 aborts came with 1.4.0, when new CString() started to return a primitive. They read no pointer:

expression	1.3.14	main	this PR
Array.from.call(CString, [1, 2, 3]), also with a map function	TypeError	abort	[1,2,3]
Array.fromAsync.call(CString, [1, 2])	TypeError	abort	[1,2]
Array species: filter, flatMap, flat	String object	abort	TypeError
Array species: concat	TypeError	abort	TypeError
Array.of.call(CString.bind(null, ptr), 1, 2)	TypeError	abort	[1,2]
class re-parented onto CString with a field: new X(), super(ptr)	String object	abort	TypeError
class re-parented onto CString: Array.from.call(X, [1, 2, 3])	TypeError	abort	TypeError
class re-parented onto CString as a species: filter	String object	abort	TypeError

The 16 segfaults are older than 1.4.0. The engine passes a length as argument 0 and CString reads it as a pointer, which is the same read as new CString(3). They are closed here as a side effect, because the engine no longer calls the handler:

expression	1.3.14 and main	this PR
Array.of.call(CString, 1, 2, 3)	Segmentation fault at address 0x3	[1,2,3]
Array.from.call(CString, { length: 2, 0: "a", 1: "b" }), Array.fromAsync with the same	segfault at 0x2	["a","b"]
Uint8Array.of.call(CString, 1), Uint8Array.from.call(CString, [1])	segfault at 0x1	TypeError
Array species: map, slice, splice, own constructor property	segfault at the length	TypeError
TypedArray species: slice, map, filter	segfault at 0x4	TypeError
ArrayBuffer and SharedArrayBuffer species slice	segfault at 0x4	TypeError
Reflect.construct(CString, [3])	segfault at 0x3	TypeError
class re-parented onto CString: Array.of.call(X, 1, 2, 3)	segfault at 0x3	TypeError

Results that change with no crash on main:

expression	main	this PR
Array.of.call(CString), Array.from.call(CString, []), Array.from.call(CString, { length: 0 }), Array.fromAsync.call(CString, [])	TypeError Attempted to assign to readonly property.	[]
Array species filter and map of an empty array	""	TypeError
Reflect.construct(CString, [])	""	TypeError
Reflect.construct(CString, [ptr]), also aliased, also with Object as new.target	"hello"	TypeError
Reflect.construct(Object, [], CString)	an object	TypeError
new (CString.bind(null))(ptr), new (CString.bind(null, ptr))()	"hello"	TypeError
new (new Proxy(CString, {}))(ptr)	"hello"	TypeError
class re-parented onto CString: new X(), super(ptr), new X(ptr), Reflect.construct(X, [])	"" or "hello"	TypeError
Bun.inspect(CString)	[class CString]	[Function: CString]

The 21 direct forms and the calls through bind, Proxy, apply and map give the same result on main and on this PR.

What does not change. new CString(3), CString(3) and [1, 2, 3].map(CString) read an invalid pointer and crash, on main and on this PR. That is the documented pointer policy (#23249).

The rest of the class. A walk of globalThis and the builtin modules finds 121 native functions on main that are constructors and have no own prototype. Array.from.call(f, [1, 2, 3]) aborts for 80 of them, one process each. With this PR it is 120 and 79: CString is the one that left. A plain call f() ends the process for none of them. bun:test mocks and cc() symbols are of the same kind and are not in the walk. #34575 and #41398 cover those with another remedy: they remove the construct handler, so new f() throws. CString cannot take that remedy. The method-table slot of this PR does not apply to them, because a function made with JSFunction::create does not read its construct data from the method table.

Demand. Found by audit. No issue, and no crash report that I can see, names these doors. #31393 and #31219 are the precedent for a fix of this kind with no report.

The test proof. Debug ASAN build, bun bd test test/js/bun/ffi/ffi-error-messages.test.ts. With src/ and packages/ at the base of this PR (#44214): 6 fail and 44 pass. The 6 are Array.of, Array.from, Symbol.species, Reflect.construct, every other door and CString prints as a function. At the head: 50 pass. The 3 other new tests pass on both builds, because they check what must not change.

Which build each number is from. Every number of this PR in these notes is from the head of the branch, 165664d.

Measurements. Release builds, Linux x64, the preview WebKit in all three: the handlers of main, the first commit of this PR (one shared body for the two handlers), and the handlers of this PR. Instructions are counted with gdb: it single-steps the third call of the handler from its entry to its return.

	main	first commit	this PR
new CString(ptr), 46 bytes	401	401	404
new CString(ptr), 5 bytes	387	387	390
new CString(ptr, 0, 5)	370	370	373
new CString(0)	41	41	44
new CString(null)	48	48	51
new CString()	35	35	38
CString(ptr), 46 bytes	405	401	401
constructFFICString, bytes	943	943	979
callFFICString, bytes	10	943	943
throwForeignNewTarget, bytes			41
InternalFunction::getConstructData, bytes	38	38	38
text of the release binary (size)	80,692,188	80,693,212	80,693,212

• The guard is 3 instructions on every new CString(...). A call is 4 instructions shorter, because callFFICString no longer forwards to constructFFICString.
• Text: the shared body adds 1,024 bytes, because the call handler is a full copy of it. The guard adds 36 bytes to constructFFICString and 41 bytes for throwForeignNewTarget, and the text of the binary does not grow.
• The engine side adds 0 bytes: a build of the handlers of main has 80,692,188 bytes of text with the pinned WebKit and with the preview build. [JSC] An InternalFunction subclass may keep a new handler and report no construct data WebKit#739 changes one assertion, and the $vm helper is not in a release build (0 symbols).
• Direct path: 0 entries into InternalFunction::getConstructData, JSCell::getConstructData, JSC::construct, Interpreter::executeConstruct and the two checkpoint OSR exit handlers in 250,000 direct constructs and calls. That is 50,000 each of new CString(ptr), CString(ptr), new CString(...args), a new C(...args) site that alternates a class and CString, and a cc() symbol that returns cstring. The construct handler ran 175,000 times and the call handler 50,000 times. Control: 1,000 aliased Reflect.construct(Map, []) enter getConstructData and executeConstruct 1,000 times each.
• Results that are not the expected string across BUN_JSC_useJIT=0, BUN_JSC_useDFGJIT=0, BUN_JSC_useFTLJIT=0 and the default, 1,200,000 constructs and calls each: 0.
• Allocations per new CString(ptr) and per CString(ptr): 1 mi_malloc (2,002 entries for 2,000 results) and 1 GC cell, on main and on this PR.
• A closed door costs what a thrown TypeError costs: 3 GC cells, and 4 mi_malloc (Reflect.construct, species) or 6 mi_malloc (new on a bound or proxied CString, the same as new Math.max()).
• Wall clock, 20 interleaved pairs, release, each the best of 5 rounds of 1,000,000 new CString(ptr): the median is 50.00 ns on main and 48.17 ns on this PR. The 20 runs of main spread over 39.82 ns (load average above 250), so this number shows nothing. perf and valgrind are not available on the machine.
• WebKit prebuilts for autobuild-preview-pr-739-42d31a6b: 42 of 42. Copies of the old WEBKIT_VERSION left in the repo: 0.

Where the tests are. ffi-error-messages.test.ts has the existing CString argument errors block. The four doors of the report (Array.of, Array.from, Symbol.species, Reflect.construct) have one child process each, because a crash takes the process down. The 19 other doors share one child that prints a line when each door returns.

Other suites. Debug ASAN build: regression/issue/25231.test.ts, addr32.test.ts, ffi-viewSource-non-object.test.ts pass. Release build: ffi-error-messages.test.ts 50 pass, cc.test.ts 35 pass, fuzzy-wuzzy.test.ts 153 pass, ffi.test.js 147 pass and 1 timeout (ptr argument: ArrayBuffer cells through an FTL-compiled call site), which a release build of the handlers of main has too on the same machine. bun-types.test.ts 22 pass.

Self-review, not taken (3 of the 17 concerns).

1. Two concerns ask for a test that constructs every native function and keeps an inventory of the ones that return a primitive. It is a guard for the whole class and has no code in common with this PR. It needs its own PR.
2. One concern asks for a docs note that ptrs.map(CString) passes the index as byteOffset. It is about the call path, which this PR does not change.

Self-review, taken. The type-info flag of the first version is gone (5 instructions in getConstructData for every native constructor, and one type-info bit). The print of CString is fixed in #44214. The Problem section leads with the aborts and not with the segfault at 0x3. The claims about debug builds are measured on the current engine. The PR stays a draft until a maintainer decides, and the question is also on #34575.

Not in this PR. bench/ffi does not build in the repo: cargo metadata fails with current package believes it's in a workspace when it's not, because the root Cargo.toml is a workspace now.

---

no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/ffi/ffi-error-messages.test.ts

[robobun]

Status: draft. It waits for oven-sh/WebKit#739 and for a maintainer decision on the forms that start to throw (see Downsides).

How to reproduce on 1.4.3, one process each:

bun -e 'const { CString } = require("bun:ffi"); Array.from.call(CString, [1, 2, 3])'
# panic(main thread): abort() called

bun -e 'const { CString } = require("bun:ffi"); class A extends Array { static get [Symbol.species]() { return CString } }; new A(1, 2, 3).filter(() => true)'
# panic(main thread): abort() called

bun -e 'const { CString } = require("bun:ffi"); Array.of.call(CString, 1, 2, 3)'
# panic(main thread): Segmentation fault at address 0x3

With this PR the first and the third line return [1, 2, 3], and the second throws a TypeError. new CString(ptr) and CString(ptr) return the same string as before.

Related: oven-sh/WebKit#739 (engine side), #44214 (the base of this PR), #34575 (the other native functions of the same class).