Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with nested structs #21

Closed
xtldr opened this issue Apr 29, 2021 · 7 comments
Closed

Issue with nested structs #21

xtldr opened this issue Apr 29, 2021 · 7 comments

Comments

@xtldr
Copy link
Contributor

xtldr commented Apr 29, 2021

The code below triggers reading freed memory:

use libffi::middle::Type;

fn main() {
    let inner_struct = Type::structure(vec!(Type::c_int()));

    println!("Inner struct: {:?}", inner_struct);

    let outer = Type::structure(vec!(inner_struct, Type::c_int()));

    let mut offsets = Vec::new();
    offsets.resize_with(2, Default::default);
    
    unsafe {
        libffi::raw::ffi_get_struct_offsets(
            libffi::low::ffi_abi_FFI_DEFAULT_ABI,
            outer.as_raw_ptr(),
            offsets.as_mut_ptr())
    };
    println!("Offsets: {:?}", offsets);
}

The output is:

Offsets: [0, 94089412172272]

When running the program through valgrind --tool=memcheck I get the output:

Inner struct: Type(0x4a6ab00)

==4739== Invalid read of size 8
==4739==    at 0x118215: ffi_get_struct_offsets (in /home/jesho/tmp/fffi/target/debug/fffi)
==4739==    by 0x110B6E: fffi::main (src/main.rs:16)
[...]
==4739==  Address 0x4a6ab00 is 0 bytes inside a block of size 24 free'd
==4739==    at 0x48399AB: free (vg_replace_malloc.c:538)
==4739==    by 0x114371: libffi::middle::types::ffi_type_destroy (libffi-rs/libffi-rs/src/middle/types.rs:169)
==4739==    by 0x114980: <libffi::middle::types::Type as core::ops::drop::Drop>::drop (libffi-rs/libffi-rs/src/middle/types.rs:175)
==4739==    by 0x11465E: core::ptr::drop_in_place<libffi::middle::types::Type> (mod.rs:179)
==4739==    by 0x113FAC: libffi::middle::types::ffi_type_array_create (libffi-rs/libffi-rs/src/middle/types.rs:106)
==4739==    by 0x1113D1: libffi::middle::types::ffi_type_struct_create (types.rs:130)
==4739==    by 0x111437: libffi::middle::types::Type::structure (types.rs:408)
==4739==    by 0x110AF6: fffi::main (src/main.rs:10)

inner_struct is dropped in libffi::middle::types::ffi_type_array_create can be accessed through outer.

If I change the loop in ffi_type_array_create to:

    for (i, element) in elements.enumerate() {
        *new.add(i) = *element.0;
        std::mem::forget(element); // Forget element
    }

I get the correct output ("Offsets: [0, 4]"). Not sure if this introduces leaks, but Valgrind doesn't complain.

@yorickpeterse
Copy link
Collaborator

I'm not sure if the forget() call is the right approach here. I would expect it to leak as element won't have its destructor run. @tov do you perhaps know more about this?

@xtldr
Copy link
Contributor Author

xtldr commented May 3, 2021

The problem is that the low::ffi_type of element is copied to new, but then element is dropped and the ffi_type deallocated.
I tried changing the loop to *new.add(i) = ffi_type_clone(*element.0); and it also works.

Since Type::drop only deallocates it's type and in this case the type is already copied, I think it should be safe, but I might be wrong.

@yorickpeterse
Copy link
Collaborator

@xtldr Ah, in that case feel free to set up a pull request and I'll include the changes :)

@xtldr
Copy link
Contributor Author

xtldr commented May 4, 2021

@yorickpeterse I have set up a pull request. I'm new to GitHub, so if there are any problems with it please let me know.

@yorickpeterse
Copy link
Collaborator

@xtldr It seems you submitted the PR to your own fork (xtldr#1), instead of this project. I'm not sure if you can change an existing PR, so you may need to set up a new one. IIRC the way to do that is to go to this project, then create a PR from there.

@xtldr
Copy link
Contributor Author

xtldr commented May 5, 2021

@yorickpeterse Yeah, I got confused when Github offered to create a pull request when I used the online editor for creating the patch. I did a new pull request against this repo, and it seems ok.

@yorickpeterse
Copy link
Collaborator

Fixed in #23, which has been released on crates.io

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants