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

Lint suggestion: slice_element_as_ptr #10560

Open
gendx opened this issue Mar 28, 2023 · 3 comments
Open

Lint suggestion: slice_element_as_ptr #10560

gendx opened this issue Mar 28, 2023 · 3 comments
Labels
A-lint Area: New lints

Comments

@gendx
Copy link
Contributor

gendx commented Mar 28, 2023

What it does

Warn against the use of casting a pointer from a slice element, and suggest the slice::as_ptr() function instead, i.e. &slice[index] as *const T can be re-written as slice.as_ptr().offset(index).


As I've recently learned in BurntSushi/memchr#58 (comment), &slice[0] as *const T is worse than slice.as_ptr(), because under the model of stacked borrows (and as evidenced by Miri), the former can only be used to dereference this element specifically, while the latter can be dereferenced anywhere in the slice.

For example, the following fails under Miri:

let ptr = &slice[0] as *const T;
let x = unsafe { ptr.offset(1); }

while this works:

let ptr = slice.as_ptr();
let x = unsafe { ptr.offset(1); }

Lint Name

slice_element_as_ptr

Category

No response

Advantage

  • The obtained pointer can be further offset and dereferenced throughout the slice without triggering UB (under the stacked borrows model).
  • Even though Miri could in principle catch this issue, Clippy may be more practical as Miri only performs runtime checks and can be quite slow when the problematic pattern occurs in a complex program.
  • More idiomatic. Removes a manual as cast.

Drawbacks

None that I could think of :)

Example

let x = vec![123u32; 100];
let y = &x[42] as *const u32;
unsafe { println!("*y = {}", *y); }

Could be written as:

let x = vec![123u32; 100];
let y = x.as_ptr().offset(42);
unsafe { println!("*y = {}", *y); }
@gendx gendx added the A-lint Area: New lints label Mar 28, 2023
@Alexendoo
Copy link
Member

The indexing version is bounds checked, whereas with .offset it would be UB if it lands outside of the slice

@asquared31415
Copy link
Contributor

I agree that it's more dangerous to use offset here even though as_ptr + offset is technically more powerful. I don't think that this sort of subtlety is something that clippy can help with in general, since the "real issue" is in later code that tries to access the pointer out of bounds. A pattern that might be common enough for clippy to warn about might be specifically index 0: &arr[0] as *const T -> arr.as_ptr().

But again in any of these cases, it might not be a good thing to have a more powerful, but less checked pointer. This code has different semantics, and even though that's more powerful, different semantics in unsafe code is very concerning.

@KisaragiEffective
Copy link
Contributor

I thought it is more suitable for before-rewrite example (get_unchecked causes UB if the indexer is out of bound):

let y = unsafe { x.get_unchecked(42) } as *const u32;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

No branches or pull requests

4 participants