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

Rust - Fix QFunction FFI Inputs/Outputs #1469

Merged
merged 1 commit into from
Feb 10, 2024
Merged

Rust - Fix QFunction FFI Inputs/Outputs #1469

merged 1 commit into from
Feb 10, 2024

Conversation

jeremylt
Copy link
Member

@jeremylt jeremylt commented Feb 10, 2024

Fix new error in Rust wrapper uncovered by changes in Rust nightly

@jeremylt
Copy link
Member Author

unsafe extern "C" fn trampoline(
    ctx: *mut ::std::os::raw::c_void,
    q: bind_ceed::CeedInt,
    inputs: *const *const bind_ceed::CeedScalar,
    outputs: *const *mut bind_ceed::CeedScalar,
) -> ::std::os::raw::c_int {
    let trampoline_data: Pin<&mut QFunctionTrampolineData> = std::mem::transmute(ctx);

    // Inputs
    let inputs_slice: &[*const bind_ceed::CeedScalar] =
        std::slice::from_raw_parts(inputs, MAX_QFUNCTION_FIELDS);
    let mut inputs_array: [&[crate::Scalar]; MAX_QFUNCTION_FIELDS] = [&[0.0]; MAX_QFUNCTION_FIELDS];
    // BUG STARTS HERE
    // unsafe precondition(s) violated: slice::from_raw_parts requires the pointer to be aligned and non-null, and the total size of the slice not to exceed `isize::MAX`
    inputs_slice
        .iter()
        .enumerate()
        .map(|(i, &x)| {
            std::slice::from_raw_parts(x, trampoline_data.input_sizes[i] * q as usize)
                as &[crate::Scalar]
        })
        .zip(inputs_array.iter_mut())
        .for_each(|(x, a)| *a = x);

    // Outputs
    let outputs_slice: &[*mut bind_ceed::CeedScalar] =
        std::slice::from_raw_parts(outputs, MAX_QFUNCTION_FIELDS);
    let mut outputs_array: [&mut [crate::Scalar]; MAX_QFUNCTION_FIELDS] =
        mut_max_fields!(&mut [0.0]);
    outputs_slice
        .iter()
        .enumerate()
        .map(|(i, &x)| {
            std::slice::from_raw_parts_mut(x, trampoline_data.output_sizes[i] * q as usize)
                as &mut [crate::Scalar]
        })
        .zip(outputs_array.iter_mut())
        .for_each(|(x, a)| *a = x);

    // User closure
    (trampoline_data.get_unchecked_mut().user_f)(inputs_array, outputs_array)
}

@jeremylt jeremylt changed the title WIP - fix Rust CI failure Rust - Fix QFunction FFI Inputs/Outputs Feb 10, 2024
@jeremylt
Copy link
Member Author

Found the error - fix was to limit the number of iterations for mapping the input/output C arrays to Rust

@jeremylt
Copy link
Member Author

@rezgarshakeri with this fix, your branch should pass fine, so we can merge your branch after this branch merges.

@rezgarshakeri
Copy link
Collaborator

Great, thanks.

Copy link
Collaborator

@rezgarshakeri rezgarshakeri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

@jeremylt jeremylt merged commit eab6b3a into main Feb 10, 2024
27 of 28 checks passed
@jeremylt jeremylt deleted the jeremy/rust-ci branch February 10, 2024 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants