Memory corruption when using libloading

Hi there,

I have been using libloading to dynamically load libraries in a plugin style use case.

I have run into memory corruption issues, which I narrowed down to BTreeMap not playing nicely across the FFI boundary. The strange thing is HashMap and Vec will work across the boundary. I can work around it by using a Vec but I would like to have some of an understanding as to why BTreeMap would fail it.

I have reproduced the crash in a bare minimum repo. To reproduce you must build the dependency crate:

cd dep
cargo build
cd ..
cargo run

Any information would be really useful to help my understanding.

  1. remove Cargo.lock from repo (for example .gitignore)
  2. Why You using unsafe and ask about unsafe error?

Weird!

I would expect all to work, but also, none are guaranteed to work. I haven't examined the compiled code, but as a first guess, I would blame BTreeMap for it being the least trivial data structure. Both Vec and HashMap (at least last time I checked) store their internal data in one place, and then have a big allocated array of data elements. BTreeMap, on the other hand, has a whole set of different allocated nodes, and each node stores both an array of pointers and an array of data. This is an excellent case study on it's implementation, if you're interested.

Maybe the fact that BTreeMap stores pointers and data together makes it particularly susceptible to the compiler deciding to rearrange its field?

Really though, if your goal is to get this working, I wouldn't recommend spending too much effort on this. None of these structures are guaranteed to work across different binaries, and trying to guess/debug why they fail will likely read to exploring compiler and stdlib internals, and maybe finding a workaround which breaks later.

The only things that should work here are #[repr(C)] structures. Repr rust things, including structures defined by the standard library, are subject to being arbitrarily different between different binaries. Rustc can and will optimize things differently, reorder strict fields, change podding, etc. It's undefined behavior to try this with any of these data structures, so you're rolling the dice every compile, even doing this with something as simple as Vec. Undefined behavior is undefined.

If your goal is exploring why this fails, then by all means, go ahead! I recommend using gdb to step through the outer program, and examining what a BTreeMap looks like in memory in the inner and outer binaries.

If you want to make this work, and reliably work, I'd recommend adding a dependency on abi_stable. It has some macros, along with a whole set of #[repr(C)] replacements for std types and collections. Admittedly no BTreeMap, but at least it has other types which ore guaranteed to work.

3 Likes

I would object to this: for binaries and examples like this one, having Cargo.lock is extremely useful. Having it allows anyone to exactly replicate the crates the author used to compile with. Even the cargo guide itself recommends keeping Cargo.lock included (and out of .gitignore) for binary crates.

If this were a published library, I'd agree with you. But it's a crashing binary, and for that, I would always expect Cargo.lock to be included.

See also: https://doc.rust-lang.org/cargo/faq.html#why-do-binaries-have-cargolock-in-version-control-but-not-libraries

This is similar to intertwining free() and malloc() with Rust allocations, but worse: in the former case, you can tell Rust to use the system allocator to get some form of inter-op. But in your rust-to-rust case, I don't think there is a way to tell both libraries to use a common allocator.

So you need to drop the Rust structs in the same "module" where they were created: you cannot give ownership of a String to the cdylib, nor can you drop the structs obtained from it:

  • libdep.so must thus export all the usual suspects functions: xxx_new, xxx_insert, xxx_drop (e.g., for xxx in vec, btreemap, and hashmap), and these functions must not take an owned String, but a read-only view of it, such as a shared borrow over a String or over some string slice.

  • Also, ABI-wise, using #[repr(Rust)] types in extern "C" signatures is not well-defined, so you should try to always use the universal C ABI: (slim) pointers. This means that the obtained collections should be Boxed, and that you should avoid fat pointers such as &str (either use &String or &&str).

Applying both changes leads to the following compiling:

//! File `dep/src/lib.rs`

use ::std::collections::*;

#[no_mangle] pub extern "C"
fn btreemap_new ()
  -> Box<BTreeMap<String, String>>
{
   Box::new(BTreeMap::new())
}

#[no_mangle] pub extern "C"
fn btreemap_insert (
    bt: &mut BTreeMap<String, String>,
    k: &String,
    v: &String,
)
{
    bt.insert(k.clone(), v.clone());
}

#[no_mangle] pub extern "C"
fn btreemap_drop (_: Box<BTreeMap<String, String>>)
{}

#[no_mangle] pub extern "C"
fn vec_new()
  -> Box<Vec<String>>
{
   Box::new(Vec::new())
}

#[no_mangle] pub extern "C"
fn vec_push (v: &mut Vec<String>, s: &String)
{
    v.push(s.clone())
}

#[no_mangle] pub extern "C"
fn vec_drop (_: Box<Vec<String>>)
{}

and then use it from src/main.rs accordingly

Hacky / dirty way I have tested it with
//! File `src/main.rs`

use ::libloading::{Library, Symbol};
use ::std::collections::*;

macro_rules! ignore_fst {(
    $fst:tt $($rest:tt)*
) => (
    $($rest)*
)}
macro_rules! p { ($expr:expr) => (
    &$expr as *const _ as *mut ()
)}

fn main ()
{
    // dropping a shared library is a very bad idea, leaking it is much better
    let lib = ::core::mem::ManuallyDrop::new(Library::new("dep/target/debug/libdep.so").unwrap());
    let lib = &*lib;
    macro_rules! call {(
        $name:ident ( $($arg:expr),* $(,)? )
    ) => ({
        let func: Symbol<unsafe extern "C" fn( $(ignore_fst!($arg *mut ()),)* ) -> *mut ()>
            = lib.get(concat!(stringify!($name), "\0").as_bytes()).unwrap()
        ;
        func($($arg ,)*)
    })};

    unsafe {
        let v = call!(vec_new());
        call!(vec_push(v, p!("Hello, World!".to_string())));
        call!(vec_drop(v));

        let bt = call!(btreemap_new());
        call!(btreemap_insert(bt, p!("Hello".to_string()), p!("World".to_string())));
        call!(btreemap_drop(bt));
    }
}
  • here for laziness / convenience I am auto-generating the unsafe extern "C" fn (*mut (), *mut (), ..., *mut ()) -> ... signatures out of the number of args that appear in the macro call!. This should, of course, be done instead properly with actual types.

Conclusion: using extern "C" fn is quite cumbersome and hard to do correctly, so:

2 Likes

Ah, if this is the failure, then it makes a lot of sense that only BTreeMap fails!

As I understand it, BTreeMap is the only one of the three data structures to allocate in new. Neither Vec::new nor HashMap::new allocates anything until the first push. But BTreeMap always needs at least one root node allocated to make algorithms work efficiently, so BTreeMap::new does allocate. So returning empty Vecs and HashMaps wouldn't cause allocator conflicts, but by chance BTreeMap would.

2 Likes

Indeed, I have just tested with dep doing Vec::with_capacity(42) and then looping over the vec part and I do get a segfault (the looping seems necessary since a use-"after"-free may not always segfault, although it will corrupt some heap memory with high probability; and once it is corrupted enough, the segfault is doomed to happen :upside_down_face:)

2 Likes

Thanks for the detailed and well written responses! So the problem is more an allocator issue rather than an ABI mismatch?

I had (possibly incorrectly) assumed that ABIs were stable across the same compiler version; it sounds like this is not guaranteed, ABIs could be different between compilations?

I was unable to reproduce that result, but I can see that it can happen. Thanks for the investigating.

I appreciate the feedback, I will rethink how I am using the plugin system.

As far as I know, the compiler doesn't guarantee anything about the Rust ABI at this time.

I cant compiling with orginal Cargo.lock
When delete it all works

This problem doesn't have anything to do with allocators or #[repr(...)], instead it happens because BTreeMap uses a static singleton value to represent an empty node. When a BTreeMap is destroyed it'll free each node except those pointing to this special "empty" node.

The problem comes when a BTreeMap from library A is destroyed by library B. The drop code for library B will ignore anything pointing to library B's empty node so it'll end up trying to "free" the library A empty node, which is actually pointing to static memory and not something managed by the allocator.

See this comment from alexcrichton:

Statics guarantee a unique address, yes, and passing something like BTreeMap across dynamic library boundaries is not supported. Almost no types in the standard library are supported to cross dynamic library boundaries.


The "solution" is for the library creating the BTreeMap to also functions for freeing and manipulating it, that way it's using the correct "empty" node.

2 Likes

This makes a lot of sense, if the BTreeMap is initialised with an entry in library A, and then dropped in library B, no memory corruption occurs.

Thanks for the additional links as well.

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.