Single mutable reference rule and FFI

[kobrir]

In Rust there is a rule that there newer should be two mutable references to the same object at the same time, but I wonder how this plays together with FFI?
For example in Rust I can write this code:

fn f2(r2: &mut i32)
{
  //do something with r2
}

fn f1()
{
  let r1: &mut i32=...
  //do something with r1
  f2(r1);
  //do something with r1
}

r1 and r2 here considered as being the same reference, so everything is good, but what if between f1 and f2 I have some external C function?
In that case in f1 I need to convert r1 to a raw pointer, pass that pointer to C function, C function then forwards that pointer to f2, and in f2 I will convert it back to the mutable reference.
Will in this case these two references also be treated as being the same? Or do I now have the undefined behavior? All functions here are being called in the same thread recursively.

[Riateche]

You can always safely convert a reference to a raw pointer. The opposite operation is unsafe: you are only allowed to create a valid reference from a pointer, i.e. it must be well-aligned, point to a valid object of the target type, and the aliasing rules must be upheld. You need to verify that manually, as the compiler cannot help you with cross-FFI invariants. For example, if you keep the original mutable reference and then create another mutable reference to the same object in f2, it will probably be UB because you're not allowed to have two unrelated mutable references to the same thing.

@kobrir

r1 and r2 here considered as being the same reference, so everything is good

I believe it is more subtle than that. When you call f2, it reborrows the reference, blocking the use of the original one until the borrow ends.

Generally, I would advise to stick with raw pointers (and safe wrappers around them) for data passing through FFI. Lifetimes and aliasing rules are usually hard to apply when FFI is involved.

[steffahn]

@Riateche

stick with raw pointers

I’m not sure what this entails. Most ways to access a raw pointer include dereferencing it, in which case a temporary (mutable or immutable depending on the use case) reference is often created.

I’ll also ping @kobrir.

The point of re-borrowing is important, in pure rust r1 and r2 are allowed to alias because r2 re-borrows r1. I guess creating a pointer from a reference does not count as re-borrowing. When first reading OPs question a few hours ago, I thought that getting rid of r1 is not possible either since it’s supposed to be used later. However reborrowing r1 and getting rid of the reborrowed reference should be safe (I think.. maybe .. I’m really not an expert, perhaps we should ask e.g. @RalfJung — in particular I’m also curious if there’s a (legal) simpler way).

fn f2(r2: &mut i32) {
  // do something with r2
}

fn f1() {
  let r1: &mut i32=...
  // do something with r1
  fn ensure_long_enough_reborrow(x: &mut i32) {
      let ptr = x as *mut i32;
      drop(x);
      // now we can create a mutable reference from x
      // and safely use it as long as we’re still in this
      // function
      call_into_c_which_will_call_back_into_f2(ptr);
  }
  ensure_long_enough_reborrow(r1);
  // do something with r1
}

Edit: TIL, this kind of explicit drop(x) is a bad idea and doesn’t work in the stacked borrows model.

[2e71828]

@steffahn

I guess creating a pointer from a reference does not count as re-borrowing.

My rough understanding of stacked borrows is that producing a raw pointer acts like an unchecked reborrow: It’s safe to use the pointer, any copies of it, and any references produced from them until the original reference is used again.

As long as the foreign code doesn’t keep the pointer and use it again after it returns control, re-entrant code is probably safe.

See Also:

Is it fine to have multiple mutable pointers to something if I don't dereference more than one?

Neither NonNull<T> nor *mut T is exclusive, so they can safely alias each other, or even a live &T or &mut T. The problem is how you get that pointer and whether there is or is not an interpretation of events where only one &mut T is live at a time. I encourage you to read about the Stacked Borrows aliasing model if you want to have a deeper understanding of how aliasing relates to UB in Rust. This model is the closest thing we currently have to a definition of aliasing in Rust. It's a little …

[RalfJung]

@2e71828

My rough understanding of stacked borrows is that producing a raw pointer acts like an unchecked reborrow: It’s safe to use the pointer, any copies of it, and any references produced from them until the original reference is used again.

Yes, that is a very good way to put it.

@kobrir

what if between f1 and f2 I have some external C function?

Could you spell out a concrete example? I am not entirely sure what you mean by "between".

[Michael-F-Bryan]

Others have answered your question from the perspective of Rust's memory model, so I'll answer it from a more practical, day-to-day standpoint.

@kobrir

I wonder how this plays together with FFI?

Normally quite poorly.

It's super common to see non-const pointers being passed around in C code, so trying to reuse Rust's & and &mut references when the C code asks for *const T and *mut T can be quite difficult, possibly making the library unusable due to borrow checker errors.

Instead, when wrapping objects that contain business logic my usual approach is to create a Rust type which uses &self methods for all operations. You'll side-step the &mut T UB problem by just using raw pointers everywhere internally, and make sure the wrapper type is !Send + !Sync to avoid data races.

@kobrir

In that case in f1 I need to convert r1 to a raw pointer, pass that pointer to C function, C function then forwards that pointer to f2, and in f2 I will convert it back to the mutable reference.

This is the logical equivalent of transmuting from & to &mut, to which The Nomicon has this to say:

• Transmuting an & to &mut is UB.
  • Transmuting an & to &mut is always UB.
  • No you can't do it.
  • No you're not special.

C is the one doing the transmuting so you don't really have much control over that, but if the C function returns a *mut T I would leave it as a raw pointer and only read from it.

The UB comes when you turn a *mut T into a &mut T or try mutating the data being pointed to.

@kobrir

All functions here are being called in the same thread recursively.

This isn't necessarily a problem your Rust code has to worry about, but if there's recursion involved you should check that the C code is re-entrant safe.

C libraries like to use static variables a lot, and when they get called recursively you can accidentally corrupt the variable's state (see Reentrancy).

[Yandros]

@Michael-F-Bryan

@kobrir

In that case in f1 I need to convert r1 to a raw pointer, pass that pointer to C function, C function then forwards that pointer to f2, and in f2 I will convert it back to the mutable reference.

This is the logical equivalent of transmuting from & to &mut

Not necessarily, it depends on what is being done.

That being said, I agree that is is quite dangerous to use Rust's &mut with long-lived pointers in C, since their being long-lived will most probably lead to aliasing, and when that happens we are reaching the & state, which can only be upgraded back to allowing mutation if the mutated fields were UnsafeCell-wrapped, and if the Rust side had never re-created an &mut reference.

That is, we have the following scenarios:

• //! Scenario "Rust creates a `&mut` in between"
  let r1 = &mut thing;
  give_to_ffi(r1 as *mut _);
  let r2 = &mut thing;
  ffi_mutate();
  

  which is always UB;

• //! Scenario "Rust creates a `&` in between"
  
  let r1 = &mut thing;
  give_to_ffi(r1 as *mut _);
  let r2 = &thing;
  ffi_mutate(); // Through direct C mutation or by calling back to Rust
  

  which is UB if the mutated fields are not wrapped within an UnsafeCell or if the called-back-Rust-code is fed an &mut thing;

• //! Scenario 'Rust does "nothing" in between'
  let r1 = &mut thing;
  give_to_ffi(r1 as *mut _);
  stuff(); /* but nothing _w.r.t._ `r1` or anything transitively pointed to by it */
  ffi_mutate(); // FFI
  

  which is fine, even if the called-back-Rust code is fed an &mut thing.

This can be showcased if replacing the *mut Things with &UnsafeCell<Thing> tranformation I showcased in:

Is it fine to have multiple mutable pointers to something if I don't dereference more than one?

FYI, a good way to know if something is sound is to try and write the pointer code only using Rust references; and seeing *mut T as &UnsafeCell<T> (aliased upgradable-to-unique-on-demand pointer): impl<T : ?Sized> /* SomeExtensionTrait<T> for */ UnsafeCell<T> { /// Downgrade the uniqueness guarantee to only be applicable during /// the `'uniq` spans, instead of continually until the point of last usage. fn from_mut (it: &'_ mut T) -> &'_ UnsafeCell<T> { unsafe { ::core…

• (basically, it allows to be a bit more explicit about where the "unchecked operations" mentioned by @2e71828 are happening).

[kobrir]

Thank you for all the answers! What brought this question is actually a little different problem in my code, I am currently trying to write a Rust wrapper for C API that is split in two part:

First, there is a list of the Rust functions that are being called from the C code, (f1, f2...)
And second, there is a list of C API functions that are being called from Rust, (api1, api2...) and they in turn can recursively call f1, f2...
The problem appears because I also need to keep some global state across all these function calls. Right now I am doing it like this:

struct Global
{
}

thread_local!
(
  static GLOBAL: RefCell<Global> = RefCell::new(Global
  {
  });
);

fn f1()
{
  GLOBAL.with(|global|
  {
    let mut global=global.borrow_mut();
    ...
  });

  api1();

  GLOBAL.with(|global|
  {
    let mut global=global.borrow_mut();
    ...
  });

  api2();

  GLOBAL.with(|global|
  {
    let mut global=global.borrow_mut();
    ...
  });

  api3();

  GLOBAL.with(|global|
  {
    let mut global=global.borrow_mut();
    ...
  });
  ...
}

In f1 I am forced to borrow GLOBAL multiple times, because calls to the api functions can, in turn, lead to the recursive call of the f1.
This is cumbersome and I think is unnecessary, because all these calls are being done in the same thread. So I thought that maybe I can get away with a function wrappers done this way:

use std::ptr;
use std::sync::atomic::{AtomicPtr, Ordering};

struct Global
{
  data: i32,
}

static mut GLOBAL:Global=Global{data:0};
static GLOBAL_PTR:AtomicPtr<Global>=AtomicPtr::new(ptr::null_mut());

fn api1() // this is actually a C function
{
  f1();
}

fn api1_wrapper(global: &mut Global) // wrapper around call to C function
{
  let global=GLOBAL_PTR.swap(global as *mut _, Ordering::Relaxed);
  assert!(global.is_null());

  api1();
  
  let global=GLOBAL_PTR.swap(ptr::null_mut(), Ordering::Relaxed);
  assert!(!global.is_null());
}

fn f1_wrapped(global: &mut Global) // actual logic is here
{
  global.data+=1;
  if global.data < 2
  {
    api1_wrapper(global);
    global.data+=1;
    api1_wrapper(global);
  }
  global.data+=1;
}

fn f1() // wrapper, called from C code
{
  let global=GLOBAL_PTR.swap(ptr::null_mut(), std::sync::atomic::Ordering::Relaxed);
  let global=unsafe{ global.as_mut().unwrap() };

  f1_wrapped(global);

  let g=GLOBAL_PTR.swap(global as *mut _, Ordering::Relaxed);
  assert!(g.is_null());
}

fn init() // called only once, during initialization
{
  unsafe
  {
    let ptr=&mut GLOBAL as *mut _;
    GLOBAL_PTR.store(ptr, std::sync::atomic::Ordering::Relaxed);
  }
}

fn main()
{
  init();

  f1();

  
  let global=GLOBAL_PTR.swap(ptr::null_mut(), std::sync::atomic::Ordering::Relaxed);
  let global=unsafe{ global.as_mut().unwrap() };
  
  println!("{}", global.data);
}

So, will this work? Or should I stick with my current solution? Or maybe there is some other way to make working with the global state in recursive functions more convenient?

[Riateche]

The ideal solution would be to avoid globals and pass some kind of context variable through the FFI. That requires modifying the C implementation, though.

Unless you see a noticeable performance impact, I don't think this problem warrants any use of unsafe code, potentially introducing UB. thread_local + RefCell is a good, safe solution.

[kobrir]

@Riateche

Unless you see a noticeable performance impact, I don't think this problem warrants any use of unsafe code, potentially introducing UB. thread_local + RefCell is a good, safe solution.

On the another hand, being a wrapper around the C API - there is already plenty of unsafe code, and adding this will not make a big difference.

And writting actual logic in f1_wrapped will be less error prone, because you don't need to worry about making API calls in the wrong place.
With my current approach, code like this will cause panic at runtime:

  GLOBAL.with(|global|
  {
    let mut global=global.borrow_mut();
    ...
    api1();
  });

Getting burned by this is actual what made me think about making this wrapper.

[Riateche]

@kobrir

On the another hand, being a wrapper around the C API - there is already plenty of unsafe code, and adding this will not make a big difference.

Having unsafe code is not an argument for adding more unsafe code, especially if you're not sure it's correct.

If you're getting a panic, that means that you're trying to access the global twice at the same time. In the alternative unsafe variant, that would just be instant UB. At least RefCell gives you runtime errors instead of silent UB. To avoid the panic, make sure to drop the RefCell's lock before you try to lock it for the second time. You can do it with an explicit drop:

let mut global=global.borrow_mut();
...
drop(global);
api1();

Or with an extra block:

{
    let mut global=global.borrow_mut();
    ...
} // global is dropped here
api1();

[kobrir]

@Riateche

If you're getting a panic, that means that you're trying to access the global twice at the same time. In the alternative unsafe variant, that would just be instant UB.

But will this really cause UB? As pointed out by @2e71828 and @RalfJung, my first example should be safe, so, by extension, I think my second example should be safe also, the only thing changed there is how the pointer being passed around.

@Riateche

Having unsafe code is not an argument for adding more unsafe code, especially if you're not sure it's correct.

But removing all of the unsafe code is also not the goal in itself, the goal is to make it harder to make mistake. And, in this case, adding a few lines of unsafe code in the wrapper make it harder to make mistake in the main program logic.

As for not being sure about correctness - that is a good argument for avoiding this approach, but I hope someone can confirm that there in no UB in this case.

[Yandros]

@kobrir

But will this really cause UB? As pointed out by @2e71828 and @RalfJung, my first example should be safe, so, by extension, I think my second example should be safe also

They kind of pointed out that it could be safe, depending on a quite important and subtle thing:

@kobrir

how the pointer being passed around.

Basically, in Stacked Borrows parlance, you need to be having around Shared Read-Write pointers (that can thus alias), and only upgrade them to Exclusive Read-Write on use, by reborrowing one (and only one) of those Shared read-Writes.

Since RefCell is the runtime checked version of it,

• we can express that requirement in RefCell parlance: you are allowed to have &RefCell<T>s that alias each other around, but you can only upgrade at most one, at usage, to a RefMut<'_, T> that can deref to &'_ mut T;

• if you violate that requirement, RefCells is able to catch it an panic!, but should you be using an unchecked equivalent of it UnsafeCells or static muts, it's most likely UB.

That being said, the way you juggle with the AtomicPtr that you never really copy, but take, does make me think what you did is sound.

The best way to "prove" it is then to try and write the same semantics but without using unsafe:

#![forbid(unsafe_code)]

use ::crossbeam::atomic::AtomicCell;

struct Global
{
    data: i32,
}

static GLOBAL_PTR: AtomicCell<Option<&'static mut Global>> =
    AtomicCell::new(None)
;

fn api1 () // this is actually a C function
{
    f1();
}

fn api1_wrapper (global: &'static mut Global) // wrapper around call to C function
{
  let global = GLOBAL_PTR.swap(Some(global));
  assert!(global.is_none());

  api1();
}

fn f1_wrapped (mut global: &'static mut Global) // actual logic is here
  -> &'static mut Global
{
    global.data += 1;
    if global.data < 2 {
        api1_wrapper(global);
        global = GLOBAL_PTR.swap(None).unwrap();
        global.data += 1;
        api1_wrapper(global);
        global = GLOBAL_PTR.swap(None).unwrap();
    }
    global.data += 1;
    return global;
}

fn f1() // wrapper, called from C code
{
  let mut global = GLOBAL_PTR.swap(None).unwrap();
  global = f1_wrapped(global);
  let g = GLOBAL_PTR.swap(Some(global));
  assert!(g.is_none());
}

fn init() // called only once, during initialization
{
   GLOBAL_PTR.store(Some(Box::leak(Box::new(Global { data: 0 }))));
}

fn main()
{
    init();

    f1();

    let global=GLOBAL_PTR.swap(None).unwrap();
  
    println!("{}", global.data);
}

• Playground

@kobrir

Right now I am doing it like this:

struct Global
{
}

thread_local!
(
  static GLOBAL: RefCell<Global> = RefCell::new(Global
  {
  });
);

fn f1()
{
  GLOBAL.with(|global|
  {
    let mut global=global.borrow_mut();
    ...
  });

  api1();

  GLOBAL.with(|global|
  {
    let mut global=global.borrow_mut();
    ...
  });

  api2();

  GLOBAL.with(|global|
  {
    let mut global=global.borrow_mut();
    ...
  });

  api3();

  GLOBAL.with(|global|
  {
    let mut global=global.borrow_mut();
    ...
  });
  ...
}

What you can do for that case is to GLOBAL.with(...) once per function, but do perform short-lived .borrow_mut()s:

fn f1()
{GLOBAL.with(|global| {
    let mut global = global.borrow_mut();
    ...
    drop(global);

    api1();

    let mut global = global.borrow_mut();
    ...
    drop(global);

    api2();

    // etc.
})}

[RalfJung]

@kobrir

In f1 I am forced to borrow GLOBAL multiple times, because calls to the api functions can, in turn, lead to the recursive call of the f1.
This is cumbersome and I think is unnecessary, because all these calls are being done in the same thread. So I thought that maybe I can get away with a function wrappers done this way:

You do have to ensure that there is never more than one "active" mutable reference, even within the same thread. So I do not see how "all these calls are being done in the same thread" makes reasoning any simpler here; aliasing rules also apply within a single thread. Also, what happens when your AtomicPtr-based version is called from multiple threads at the same time?

Certainly, init should be unsafe fn as calling it multiple times will make things go horribly wrong.

[kobrir]

Thank you for the analysis, to describe my project in more detail - I am writing an extension for the program written in C, the API this program provides is inherently single threaded, main program is calling my module using one of the several entry points (f1 in my example). That is the only way my module is getting control, and these entry points also never called from rust directly, only from the the main program.

Once my module got the control, it can call back the main program using one of the API functions (api1 in the example), and that API function can call my module through the same set of entry points again.

I am planning to wrap all of these entry points and API functions in the similar way, and move this wrapper in a separate module, so that the main logic in f1_wrapped will not see f1 and api1 directly. In this model the only way to get two reffereces to the GLOBAL is in the case of recursion, such as this call chain:
main program -> f1 (first refferece is here) -> f1_wrapped -> api1_wrapper -> api1 -> f1 (second refferece)