Uninitialized data read in safe code

[markcsaving]

Problem

The following code is safe but results in reading uninitialized data twice (which is undefined behavior):

use libffi::high::{Callback0, Cif0, Closure0, CType};
use libffi::low::ffi_cif;

extern "C" fn callback(_cif: &ffi_cif, result: &mut i32, _args: &(), _userdata: &()) {
    println!("Reading uninitialized result: {}", *result);
}

fn main() {
    println!("Hello, world!");
    let c: Callback0<(), i32> = callback;
    let closure = Closure0::from_parts(Cif0::new(i32::reify()), c, &());
    println!("Result = {}", closure.code_ptr().call());
}

First, we read uninitialized data inside the callback, since there is no guarantee result actually refers to a valid i32. Second, we read uninitialized data inside main, since the result of closure.code_ptr.call() is uninitialized.

There are two issues. The first is that Callbackn is defined to take result: &mut R instead of result: &mut MaybeUninit<R> or result: *mut R. The second is that there is no static way of enforcing the requirement that callbacks actually write to result.

Note that all issues raised here also apply to the Mut and Once versions, and to closures/callbacks of all arities (not just 0-ary). The same is true for the solutions.

Potential solutions

A mostly non-breaking solution

~All the types which implement CType share a convenient property; they can be safely zero-initialized. Instead of using MaybeUninit::new_uninit() to "initialize" the result and then passing a pointer to the callback, we could instead call MaybeUninit::zeroed().assume_init() to initialize the result before invoking the callback. Since CType is an unsafe trait and there is no documented way to safely implement it, this technically wouldn't be a breaking change. This would probably warrant updating the documentation for CType, but I think it's fairly intuitive that all C types should be zero-initializable.~

~Costs: the runtime cost of zero-initialization.~

Edit: it appears to be impossible to fix this in a non-breaking way from the Rust side, since we don't actually control how the code pointer is used except when we do dynamic calling. Unfortunately, any change would have to be breaking.

A breaking solution

Change the definition of Callbackn so that it takes result: &mut MaybeUninit<R>. Mark from_parts as unsafe.

Costs: breaks all code which relies on the definition of Callbackn or from_parts. Requires use of unsafe to call from_parts (but not to actually write the callbacks - the MaybeUninit API has enough safe functions that this is unnecessary).