neon-bindings / neon

Rust bindings for writing safe and fast native Node.js modules.
https://www.neon-bindings.com/
Apache License 2.0
8k stars 283 forks source link

Seg fault when linked with a C library #1007

Closed mdrokz closed 10 months ago

mdrokz commented 10 months ago

Hi thanks for the wonderful crate Im currently trying to use neon with a C library for scanning fingerprints so i can integrate it into my frontend with neon bindings. but i get a segfault everytime i run the init method. I have it working with a normal rust binary but with neon it doesnt work here's my rust code

build.rs

use std::path::PathBuf;

use std::env;

fn main() {
    let out_path = PathBuf::from(env::var("OUT_DIR").expect("No out dir found"));

    let bindings = bindgen::Builder::default()
        .clang_args(&["-x", "c++", "-std=c++11"])
        .header("../include/sgfplib.h")
        .generate()
        .expect("Unable to generate bindings");

    bindings
        .write_to_file(&out_path.join("bindings.rs"))
        .expect("Couldn't write bindings!");

    println!("cargo:rustc-link-search=native=/usr/local/lib");
    println!("cargo:rustc-link-lib=sgfplib");
    println!("cargo:rustc-link-lib=jpeg");
    println!("cargo:rustc-link-lib=stdc++");
}

lib.rs

pub fn init_device<'a>(mut cx: FunctionContext<'a>) -> JsResult<'a, JsBoolean> {

    println!("Init device");

    let mut sgfpm: MaybeUninit<SGFPM> = MaybeUninit::uninit();

    let mut sgfpm_ptr = sgfpm.as_mut_ptr();

    unsafe {
        CreateSGFPMObject(&mut sgfpm_ptr as *mut _);
    }

    println!("sgfpm_ptr: {:?}", sgfpm_ptr);

    if sgfpm_ptr.is_null() {
        println!("Failed to create SGFPM object");
        // return Err("Failed to create SGFPM object".to_string());
    }

    unsafe {
        println!("Init device");
        let err = SGFPM_Init(sgfpm_ptr as *mut c_void, SGFDxDeviceName_SG_DEV_AUTO.into());

        println!("err: {:?}", err);

        if err != SGFDxErrorCode_SGFDX_ERROR_NONE.into() {
            // return Err(format!("Init: Failed : ErrorCode = {}", err));
        }

        let err = SGFPM_OpenDevice(sgfpm_ptr as *mut c_void, 0);

        if err != SGFDxErrorCode_SGFDX_ERROR_NONE.into() {
            // return Err(format!("OpenDevice: Failed : ErrorCode = {}", err));
        }

        SGFPM_SetBrightness(sgfpm_ptr as *mut c_void, 50);

        let mut p_info: MaybeUninit<SGDeviceInfoParam> = MaybeUninit::uninit();
        let err = SGFPM_GetDeviceInfo(sgfpm_ptr as *mut c_void, p_info.as_mut_ptr());

        if err != SGFDxErrorCode_SGFDX_ERROR_NONE.into() {
            // return Err(format!("GetDeviceInfo: Failed : ErrorCode = {}", err));
        }

        let p_info = p_info.assume_init();

        let device_info = DeviceInfo {
            image_width: p_info.ImageWidth,
            image_height: p_info.ImageHeight,
            com_speed: p_info.ComSpeed,
            com_port: p_info.ComPort,
        };
    }

    Ok(cx.boolean(true))
}

#[neon::main]
fn main(mut cx: ModuleContext) -> NeonResult<()> {
    cx.export_function("initDevice", init_device)?;

    Ok(())
}

When i invoke this from the node side like this

const {initDevice} = require('.');

const r = initDevice();

if(r){
    console.log('Device initialized');
} else {
    console.log('Device not initialized');
}

I get this error

LD_LIBRARY_PATH=/usr/local/lib node index.js
Init device
sgfpm_ptr: 0x55acc2f7a730
Init device
fish: Job 1, 'LD_LIBRARY_PATH=/usr/local/lib …' terminated by signal SIGSEGV (Address boundary error)
kjvalencik commented 10 months ago

It's strange that there's only a SIGSEV when running under Node, but the issue seems unrelated. I see a couple of bugs in the unsafe code:

The code is attempts to write a pointer into space allocated for the struct SGFPM instead of a pointer. It then tries to use the pointer to the MaybeUninit as if it were the pointer to SGFPM. This is going to be garbage data.

A couple of recommendations:

The combination of these two will make type errors with pointers much less likely in unsafe code since they become compiler errors.

I don't have a good way to test, but I think it should look something like this:

    let mut sgfpm = MaybeUninit::uninit();

    unsafe {
        CreateSGFPMObject(sgfpm.as_mut_ptr());
    }

    let sgfpm = sgfpm.assume_init();

    if sgfpm.is_null() {
        return cx.throw_error("Failed to create SGFPMObject");
    }

    unsafe {
        // Not sure if this should be `sgfpm`, `&sgfpm` or `&mut sgfpm`
        let err = SGFPM_Init(sgfpm, SGFDxDeviceName_SG_DEV_AUTO.into());
    }
mdrokz commented 10 months ago

It's strange that there's only a SIGSEV when running under Node, but the issue seems unrelated. I see a couple of bugs in the unsafe code:

  • sgfpm should be a LPSGFPM (pointer to SGFPM) and not a SGFPM.
  • Calling as_mut_ptr() gives you a pointer to the MaybeUninit space, not the underlying pointer

The code is attempts to write a pointer into space allocated for the struct SGFPM instead of a pointer. It then tries to use the pointer to the MaybeUninit as if it were the pointer to SGFPM. This is going to be garbage data.

A couple of recommendations:

  • Let Rust infer the type of MaybeUninit
  • Use .cast() instead of as

The combination of these two will make type errors with pointers much less likely in unsafe code since they become compiler errors.

I don't have a good way to test, but I think it should look something like this:

    let mut sgfpm = MaybeUninit::uninit();

    unsafe {
        CreateSGFPMObject(sgfpm.as_mut_ptr());
    }

    let sgfpm = sgfpm.assume_init();

    if sgfpm.is_null() {
        return cx.throw_error("Failed to create SGFPMObject");
    }

    unsafe {
        // Not sure if this should be `sgfpm`, `&sgfpm` or `&mut sgfpm`
        let err = SGFPM_Init(sgfpm, SGFDxDeviceName_SG_DEV_AUTO.into());
    }

Hey thanks for the suggestions i didnt know you could do that i was trying to figure out how to use MaybeUninit properly. I tried this & it still didnt work so i debugged with gdb and it seems to crash at this function in libusb

LD_LIBRARY_PATH=/usr/local/lib/ gdb node

Thread 1 "node" received signal SIGSEGV, Segmentation fault.
0x00007ffff403cf3f in usb_control_msg (dev=0x0, bmRequestType=64, bRequest=<optimized out>, wValue=<optimized out>, wIndex=0, bytes=0x0, size=0, timeout=2000)
    at /usr/src/debug/libusb-compat-0.1-0.1.7-9.fc37.x86_64/libusb/core.c:878
878             r = libusb_control_transfer(dev->handle, bmRequestType & 0xff,    

Its weird that it only happens with node & my rust binary works fine do you have any idea what interaction there is with node & libusb ?

kjvalencik commented 10 months ago

I'm not aware of anything that would cause this in Node. Is the source available that I could run and test?

mdrokz commented 10 months ago

I'm not aware of anything that would cause this in Node. Is the source available that I could run and test?

no let me quickly set up a repo for you

mdrokz commented 10 months ago

I'm not aware of anything that would cause this in Node. Is the source available that I could run and test?

I set up a repo here https://github.com/mdrokz/secugen_rs

just set "cargo:rustc-link-search=native=/usr/local/lib" to your cloned repo path i have the libs in there.

kjvalencik commented 10 months ago

@mdrokz Looks like libsgimage.so (and possibly others) is missing.

kjvalencik commented 10 months ago

I found some libs on line and got a missing CreateISensor symbol. If I create a stub for that, I get further and there's no SIGSEGV.

Init device
Init device
err: 6
Device initialized

I'm not sure how else to help. I don't think this is a Node/Neon issue.

mdrokz commented 10 months ago

I found some libs on line and got a missing CreateISensor symbol. If I create a stub for that, I get further and there's no SIGSEGV.

Init device
Init device
err: 6
Device initialized

I'm not sure how else to help. I don't think this is a Node/Neon issue.

Oh it worked for you i guess its probably a system related issue with the libs. Sorry for taking your time i will close this now i will test it on other systems.