Size of unions is miscalculated by node-ffi-napi

[shepmaster]

I created a dynamic library with a function that takes an integer and returns a union that is eight bytes. Using that function via ref-union-di and node-ffi-napi causes memory corruption on Windows x86_64.

As a minimal reproduction, I have a Rust project for the library:

src/lib.rs

use std::os::raw::c_int;

#[repr(C)]
#[derive(Copy, Clone)]
pub struct EightBytes {
    d0: u32,
    d1: u32,
}

#[repr(C)]
pub union TheUnion {
    id: u8,
    eight_bytes: EightBytes,
}

#[no_mangle]
pub extern "C" fn return_a_union(value: c_int) -> TheUnion {
    dbg!(value);

    let eight_bytes = EightBytes { d0: 0, d1: 0 };
    TheUnion { eight_bytes }
}

Cargo.toml

[package]
name = "union-return"
version = "0.1.0"
edition = "2021"

[lib]
crate-type = ["cdylib"]

[dependencies]

And basic JS usage:

index.js

var ref = require('ref-napi');
var ffi = require('ffi-napi');
var Struct = require('ref-struct-di')(ref);
var Union = require('ref-union-di')(ref);

const EightBytes = new Struct({
    d0: ref.types.uint32,
    d1: ref.types.uint32,
});

const TheUnion = new Union({
    id: ref.types.uint8,
    eight_bytes: EightBytes,
});

var theLibrary = ffi.Library('../target/debug/union_return.dll', {
  'return_a_union': [TheUnion, [ref.types.int]],
});

theLibrary.return_a_union(42);

package.json

{
  "name": "usage",
  "version": "1.0.0",
  "main": "index.js",
  "license": "MIT",
  "private": true,
  "dependencies": {
    "ffi-napi": "^4.0.3",
    "ref-napi": "^3.0.3",
    "ref-struct-di": "^1.1.1",
    "ref-union-di": "^1.0.1"
  }
}

Rust version 1.61.0

rustc 1.61.0 (fe5b13d68 2022-05-18)
binary: rustc
commit-hash: fe5b13d681f25ee6474be29d748c65adcd91f69e
commit-date: 2022-05-18
host: x86_64-pc-windows-msvc
release: 1.61.0
LLVM version: 14.0.0

Node version v18.3.0

Building the library and executing it yields

> node .\index.js
[src\lib.rs:18] value = -198616544

> node .\index.js
[src\lib.rs:18] value = 627422272

> node .\index.js
[src\lib.rs:18] value = -386156992

> node .\index.js
[src\lib.rs:18] value = -492129904

> node .\index.js
[src\lib.rs:18] value = -7508400

> node .\index.js
[src\lib.rs:18] value = -1442674912

> node .\index.js
[src\lib.rs:18] value = 1543752848

> node .\index.js
[src\lib.rs:18] value = 632273872

[shepmaster]

Tracing through the code, this library appends each union member to the fields property:

ref-union-di/lib/union.js

Line 154 in 858792b

this.fields[name] = field

However, node-ffi-napi will check for and then iterate over all of the elements of the fields property, treating them as multiple arguments.

#14 reworks this to only expose the biggest field via fields, keeping the whole set as allFields.

The libffi manual indicates that this isn't the best way either. Instead, it suggests (emphasis mine):

One simple way to do this is to ensue [sic] that each element type is laid out. Then, give
the new structure type a single element; the size of the largest element; and the largest
alignment seen as well.

This example uses the ffi_prep_cif trick to ensure that each element type is laid out.

A Stack Overflow post from one of the libffi contributors suggests that ffi_get_struct_offsets could be used instead of ffi_prep_cif.

[shepmaster]

One (ugly!) workaround is to artificially increase the size of the union to exceed eight bytes. For example:

#[repr(C)]
#[derive(Copy, Clone)]
pub struct EightBytes {
    d0: u32,
    d1: u32,
}

#[repr(C)]
#[derive(Copy, Clone)]
pub struct ChunkyBoi {
    d0: u32,
    d1: u32,
    d2: u32,
    d3: u32,
}

#[repr(C)]
pub union TheUnion {
    id: u8,
    eight_bytes: EightBytes,
    chunky_boi: ChunkyBoi, // Not needed, just to workaround the issue
}

const EightBytes = new Struct({
    d0: ref.types.uint32,
    d1: ref.types.uint32,
});

const ChunkyBoi = new Struct({
    d0: ref.types.uint32,
    d1: ref.types.uint32,
    d2: ref.types.uint32,
    d3: ref.types.uint32,
});

const TheUnion = new Union({
    id: ref.types.uint8,
    eight_bytes: EightBytes,
    chunky_boi: ChunkyBoi, // Not needed, just to workaround the issue
});