Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions src/node_ffi.cc
Original file line number Diff line number Diff line change
Expand Up @@ -286,9 +286,10 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction(

// Creating a callable emits a trampoline, allocates an FFIFunctionInfo, and
// on the SharedBuffer path allocates an ArrayBuffer, so reuse the one already
// handed out for this symbol. `PrepareFunction()` rejects a request that uses
// a different signature, so a hit always describes the same signature. An
// empty handle means the wrapper was collected; fall through and rebuild.
// handed out for this symbol. `PrepareFunction()` and `GetFunctions()` reject
// a request that uses a different signature, so a hit always describes the
// same signature. An empty handle means the wrapper was collected; fall
// through and rebuild.
auto cached = function_wrappers_.find(name);
if (cached != function_wrappers_.end()) {
if (!cached->second.IsEmpty()) {
Expand Down Expand Up @@ -964,6 +965,23 @@ void DynamicLibrary::GetFunctions(const FunctionCallbackInfo<Value>& args) {
});
}

// The signature getters above run user code that may have requested an
// earlier symbol with a different signature. Check before caching anything
// so that a rejected call leaves the library unchanged.
for (const auto& item : pending) {
auto existing = lib->functions_.find(item.name);
if (existing != lib->functions_.end() &&
!SignaturesMatch(
*existing->second, item.fn->return_type, item.fn->args)) {
THROW_ERR_INVALID_ARG_VALUE(
env,
"Function %s"
" was already requested with a different signature",
item.name);
return;
}
}

for (const auto& item : pending) {
if (item.should_cache_symbol) {
lib->symbols_.emplace(item.name, item.fn->ptr);
Expand Down
48 changes: 48 additions & 0 deletions test/ffi/test-ffi-dynamic-library.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,54 @@ test('getFunction caches signatures consistently', () => {
}
});

test('getFunctions rejects a signature changed by a getter', () => {
const lib = new ffi.DynamicLibrary(libraryPath);
const conflicting = { arguments: ['f64', 'f64'], return: 'f64' };

try {
let fn;
assert.throws(() => {
lib.getFunctions({
add_i32: fixtureSymbols.add_i32,
get add_i64() {
fn = lib.getFunction('add_i32', conflicting);
return fixtureSymbols.add_i64;
},
});
}, {
code: 'ERR_INVALID_ARG_VALUE',
message: /Function add_i32 was already requested with a different signature/,
});

// The rejected call must not cache any of its own entries.
assert.deepStrictEqual(Object.keys(lib.functions), ['add_i32']);
assert.strictEqual(lib.getFunction('add_i32', conflicting), fn);
} finally {
lib.close();
}
});

test('getFunctions accepts a matching signature requested by a getter', () => {
const lib = new ffi.DynamicLibrary(libraryPath);

try {
let fn;
const functions = lib.getFunctions({
add_i32: fixtureSymbols.add_i32,
get add_i64() {
fn = lib.getFunction('add_i32', fixtureSymbols.add_i32);
return fixtureSymbols.add_i64;
},
});

assert.strictEqual(functions.add_i32, fn);
assert.strictEqual(functions.add_i32(20, 22), 42);
assert.strictEqual(functions.add_i64(20n, 22n), 42n);
} finally {
lib.close();
}
});

test('resolving the same symbol reuses one function', () => {
const lib = new ffi.DynamicLibrary(libraryPath);
const definitions = { add_i32: fixtureSymbols.add_i32 };
Expand Down
Loading