Cloning Dereferenced symbol may cause undefined behaviour.

[tyleo]

Since adding the Clone functionality I have been experimenting with cloning Symbols. I noticed that it is possible to clone a dereferenced Symbol. This may cause undefined behavior. In particular this code is broken:

let func = {
    let library = Library::new("path_to_lib").unwrap();
    let func = library.get::<fn()>(b"symbol").unwrap();
    let func_ref = &func;
    func_ref.clone()
};
func();

This derefs func as fn() and calls the Clone implementation on fn() rather than on the Symbol object. Since the Symbol's Library has already gone out of scope, the call can fail. I am currently investigating ways to get around this.

To be clear, this problem is not caused by the new Clone functionality and it exists on earlier versions of the code. The problem seems to be with the Deref implementation:

impl<T> ::std::ops::Deref for Symbol<T> {
    type Target = T;
    fn deref(&self) -> &T {
        unsafe {
            // Additional reference level for a dereference on `deref` return value.
            mem::transmute(&self.pointer)
        }
    }
}

In particular, the transmute drops lifetime information about the Symbol but allows it to be copied from the raw function pointer.

[tyleo]

My current thinking is that Symbol should not implement Deref at all unless it can Deref into an Fn rather than an fn.It should implement std::ops::Fn and also provide a stable call mechanism if possible (std::ops::Fn is currently unstable).

[nagisa]

My current thinking is that Symbol should not implement Deref at all unless it can Deref into an Fn rather than an fn.It should implement std::ops::Fn and also provide a stable call mechanism if possible (std::ops::Fn is currently unstable).

Keep in mind that you can also dlsym statics and TLS variables. These must be as usable as the functions.

This derefs func as fn() and calls the Clone implementation on fn() rather than on the Symbol object. Since the Symbol's Library has already gone out of scope, the call can fail. I am currently investigating ways to get around this.

Ew, that’s nasty.

[tyleo]

I've come to the conclusion that Symbols should not implement Deref for two reasons:

1. It becomes too easy for a Deref coercion to cause confusing behavior. I already described a basic case where a copied fn() causes undefined behavior. The general case of this happens for any object with type T: Copy or T: Clone which contains a pointer into the loaded library. A user can easily obtain a copy of the symbol which will satisfy incorrect lifetime requirements and outlive the library it is loaded from.
2. The Unsafe guidelines for rust state that, "all functions called from FFI must be marked as unsafe." (https://doc.rust-lang.org/book/unsafe.html)

If Symbol must return a reference to its contents, the function call which returns them must be unsafe. This rules out AsRef, Deref, and Borrow.

At this point, I think that the best implementation would require different structs for function symbols and data symbols. The function symbols could implement an unsafe method which allows them to be called. They cannot implement Fn, even in unstable rust, because it breaks requirement 2 above. The data symbols could implement an unsafe method similar to Deref which allows their data to be borrowed. When negative trait bounds are available, then they would be able to implement Deref for T: !Copy + !Clone.

[nagisa]

This is blocked on https://github.com/rust-lang/rust/issues/29625.