Add transmute_slice_to_larger_element_type lint

[KisaragiEffective]

• Followed lint naming conventions
• Added passing UI tests (including committed .stderr file)
• cargo test passes locally
  • except fmt check
• Executed cargo dev update_lints
• Added lint documentation
• Run cargo dev fmt
  • This was not possible, because running cargo dev fmt somehow implied cargo dev fmt --check

close #10285

---

changelog: New lint: [transmute_slice_to_larger_element_type]
#10312

[rustbot]

r? @Jarcho

(rustbot has picked a reviewer for you, use r? to override)

[KisaragiEffective]

(force pushed to revert local change that was try about clippy dev fmt)

[samueltardieu]

Have you tried this suggestion? I don't think this will do what you want: this attempts to transmute &i8 to i32.

Also, isn't the vector you create dropped at the end of the statement? (and don't you mean .as_slice()?)

[asquared31415]

Yes, this code does not compile because of the temporary Vec being dropped, and the transmute on each element is wrong as well. A "proper" suggestion might be to go through core::slice::from_raw_parts, but also because of alignment requirements, I don't think it can be an automated suggestion. Perhaps "consider core::slice::from_raw_parts(_mut) instead" would suffice?

[samueltardieu]

Don't you mean unsafe { i8_slice.align_to::<i32>().1 }?

[samueltardieu]

You can use if_chain! to avoid this cascade fo else { false } (or if let … && let … && … since feature let_chains is enabled).

[Jarcho]

This would be fine without a suggestion. It's more likely that the size needs to change than anything.

This would be better as a lint to check if the size changes, not just becomes larger. Both almost certainly do not have the intended result.

[KisaragiEffective]

This would be better as a lint to check if the size changes, not just becomes larger.

Yes, that will not be what is wanted. However, if slice gets truncated by transmute, it does not cause UB immediately (still may cause: &[T] to &[bool] would cause UB for most T). So it would be fall in suspicious category and better to create separate lint IMO.

[Jarcho]

It's not instant UB to grow the size either, it depends on the underlying allocation. e.g.

let x: [u32; 2] = [0, 1];
let y: &[u64] = transmute(&x[0..1]); // This is 'fine'

[asquared31415]

Note that

let x: [u32; 2] = [0, 1];
let y: &[u64] = transmute(&x[0..1]); // This is 'fine'

is not fine, as per Miri. The input slice is only allowed to access the 4 bytes that make up the element from [0..1], and Miri errors before the transmute returns, because the transmute is trying to access 8 bytes. The only way to do this properly is to go through raw pointers, core::slice::from_raw_parts(x.as_ptr().cast::<u64>(), 1);

Transmuting a slice to any larger element type is always an error because of this, since the length in elements stays the same, but it tries to access more bytes. This is basically the same as cast_slice_different_sizes, but is immediate UB because it's references.

(additionally there's alignment issues and such, but those can be checked/fixed more easily)

re: shrinking instead of growing, it's probably useful as a different lint, since it could be intended, but still definitely suspicious, i would be in favor of either warn or deny on such a lint. But this lint is always correct to be deny because the code is always UB to access more bytes.

[Jarcho]

is not fine, as per Miri. The input slice is only allowed to access the 4 bytes that make up the element from [0..1], and Miri errors before the transmute returns, because the transmute is trying to access 8 bytes. The only way to do this properly is to go through raw pointers, core::slice::from_raw_parts(x.as_ptr().cast::(), 1);

Looks like this is much more defined than when I last checked (years ago). Both tree and stack borrows definitely set this as ub with references. As a tangential note, it is valid for raw pointers:

fn main(){
    let x = [0u64; 4];
    let x = &x as *const [u64; 4] as *const u32;
    let x = std::ptr::slice_from_raw_parts(x, 4) as *const [u64];
    let x = unsafe { &*x };
    println!("{:?}", x);
}

---

I'm still not convinced this is worth a lint separate from just any size changes. Converting to a smaller type without changing the length is incredibly close to certainly wrong. I've never seen a valid use case for it. I can't even imagine what that use case would be.

[bors]

☔ The latest upstream changes (presumably #11750) made this pull request unmergeable. Please resolve the merge conflicts.

[xFrednet]

Hey @KisaragiEffective this is a ping from triage. Are you planning to continue working on this PR?

[KisaragiEffective]

Hey @KisaragiEffective this is a ping from triage. Are you planning to continue working on this PR?

I will, i am not able to decide whether the lint will fire not only larger, but smaller one. The current impl has a bug that suggests take ref on temp, but other parts still can be used.

[xFrednet]

I will, i am not able to decide whether the lint will fire not only larger, but smaller one.

Interesting, I would lint that as well. Having &[u32] -> &[u8] will not result in out of bounds reading, but it's still very likely wrong. Then the lint could just be called transmute_slice or something similar.

If there is a valid use case to transmute &[u32] -> &[u8] we can just add a configuration value to allow this. But I think just linting it is fine and will catch more cases that should be linted :D

[xFrednet]

Hey this is triage, I'm closing this due to inactivity. If you want to continue this implementation, you're welcome to create a new PR. Thank you for the time, you already put into this!

Interested parties are welcome to pick this implementation up as well :)

@rustbot label +S-inactive-closed -S-waiting-on-author -S-waiting-on-review