Missing guidance on converting FFI ptr/length to slice?

help
1

I know from the docs of std::slice::from_raw_parts() that ...

data must be non-null and aligned even for zero-length slices. One reason for this is that enum layout optimizations may rely on references (including slices of any length) being aligned and non-null to distinguish them from other data. You can obtain a pointer that is usable as data for zero-length slices using NonNull::dangling().

Based on that advice, I have done something like this in a function that's part of the C API of some of my Rust code (where data is a pointer coming from a call to the C++ function std::vector::data() in my case):

let data = len != 0 {
    data
} else {
    std::ptr::NonNull::dangling().as_ptr()
};
let my_slice = std::slice::from_raw_parts(data, len);

This seems cumbersome, but much worse, I have just realized that I'm using std::slice::from_raw_parts() in other parts of my FFI code and there I did not do the dangling dance, which makes those already unsafe functions even more unsafe.

A few days ago, I stumbled upon a different piece of advice (Reddit - Dive into anything):

The better way to turn C++ spans into Rust slices is ptr::slice_from_raw_parts(ptr, len).as_ref(), which produces Option<&[T]>

This seems very reasonable to me, and I guess it would change my example above into something like this:

let my_slice = std::ptr::slice_from_raw_parts(data, len).as_ref().unwrap_or(&[]);

Now (finally!) my questions:

Is this indeed the recommendation in this general situation?
Or is there an even better way to do this?

Shouldn't the documentation of std::slice::from_raw_parts() point to std::ptr::slice_from_raw_parts() (which it currently doesn't), mentioning this important use case?

And latter should probably mention this use case as well (if it is indeed recommended)?

And maybe there could even be a Clippy lint that warns about using std::slice::from_raw_parts() with a potential NULL pointer?
But I guess this might lead to many false positives ...

2

I think it's a bit safer to do it this way:

if !data.is_null() {
    std::slice::from_raw_parts(data, len)
} else {
    // optionally report error if len > 0 here
    &[] 
}

because the problem is not the empty length, but the null pointer. And yes, you will need that check whenever you get pointers from FFI. You can make your own function for it, and grep your code and replace from_raw_parts with some crate::util::ffi_slice().

2 Likes
3

Thanks @kornel!

Now that you mention it, checking for NULL instead of len indeed makes a lot of sense.
I wouldn't report an error when len != 0, because std::ptr::slice_from_raw_parts() doesn't do that either (if I'm not mistaken).

I think I will write a helper function in my code, as you suggested.

However, this answers only part of my questions.

Should this advice be added to the docs of std::slice::from_raw_parts()?

Is there another way to avoid this footgun for people who are not aware of it?

I really like that Rust often creates a "pit of success", but this is not one of those cases.

I guess the behavior of std::slice::from_raw_parts() cannot be changed to include a NULL check, but is there a way to add another function to the standard library that's less dangerous?
Or can it actually be changed?

Is there something that can be done with tooling, like e.g. with Clippy?

4

The non-null requirement is problematic indeed, as it makes Rust have slices subtly incompatible with C and C++.

.as_ref().unwrap_or(&[]) is an interesting way of implementing it. It's nice it's a one-liner expression. However, std::slice and std::ptr versions have the same name, and a different as_ref is on both, so that can be confusing.

Yes. Clippy has access to the AST with types, so it could require functions to have a null check and suggest using NonNull<T> or NonNull<[T]> where null checks have already been done earlier. That would probably be an off-by-default lint though, because without a serious data flow analysis it might complain too often.

5

If Toggle assert_unsafe_precondition in codegen instead of expansion by saethlin · Pull Request #120594 · rust-lang/rust · GitHub is merged, builds with debug assertions enabled (default cargo build/run/test) will have such a check.

6

Thanks @saethlin, that's great!

This is already in beta, I've tried it and it indeed raises an error when passing a NULL pointer:

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`

I have seen that there is also already a Clippy lint invalid_null_ptr_usage: Clippy Lints.
This seems to cover only the very simplest wrong usage, but it is better than nothing!

7

Thanks for linking to this blog post. This was actually the reason why I started looking into this!

Thanks for this PR, it is already available in beta: slice_from_raw_parts in std::ptr - Rust

I have created a PR for extending the documentation of slice::from_raw_parts():

8

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.