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

Consider having transmute also complain if the pointed-to objects are of different sizes #21590

Closed
larsbergstrom opened this issue Jan 24, 2015 · 8 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-type-system Area: Type system C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@larsbergstrom
Copy link
Contributor

Today, cast::transmute will complain if the subjects of the transmute are different sizes. However, if those objects are pointers and the pointed-to objects are different sizes, it will not complain.

This was an issue in Servo because we had two structs that were identical before a Rust upgrade, but after a Rust upgrade some smart pointer-related optimizations changed the layout of one of the structs, resulting in a silent memory corruption. The solution was to add a NonZero wrapper to one of the raw pointers within the structs to disable the compiler optimization (servo/servo@f400d79), but this feels fragile.

@jdm
Copy link
Contributor

jdm commented Jan 24, 2015

It wasn't actually memory corruption; it was the presence of NonNull (via the Box) within one of the types contained in the Option that was the target of the transmute. This caused the enum layout optimization code to deduce that it could use the NonNull value as the discriminant for the transmuted Option, which is what caused us to see Some values where the pre-transmute value was None.

@jdm
Copy link
Contributor

jdm commented Jan 24, 2015

For clarity, this is essentially what we had:

struct PreTransmute {
    ptr: *const (),
}

struct PostTransmute {
    ptr: Box<Something>,
}

let some_value: RefCell<Option<PreTransmute>> = RefCell::new(None);
let tmp = some_value.borrow();
let some_other_value: Ref<Option<PostTransmute>> = unsafe { mem::transmute(tmp) };

and some_other_value would now be classified as Some.

@kmcallister kmcallister added A-type-system Area: Type system A-diagnostics Area: Messages for errors, warnings, and lints labels Jan 24, 2015
@kmcallister
Copy link
Contributor

This seems like a generalization of -W fat-pointer-transmutes, which is currently broken (#19676).

@Thiez
Copy link
Contributor

Thiez commented Jan 25, 2015

Since struct layout is undefined, transmuting one type to another is probably a bad idea in the general case, unless they're both #[repr(C)], or when you're transmuting to and from an array of bytes or something. Which does seems like a good usecase that might be negatively affected by the change suggested in this issue.

@steveklabnik
Copy link
Member

Triage: no change that I'm aware of.

@steveklabnik steveklabnik added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 9, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-feature-request Category: A feature request, i.e: not implemented / a PR. label Jul 22, 2017
@steveklabnik
Copy link
Member

Triage: no changes I'm aware of

@asquared31415
Copy link
Contributor

asquared31415 commented Jul 1, 2022

This cannot be made an error specifically with raw pointers because it may be okay. The following code is valid (though odd):

let arr: [u8; 4] = [1, 2, 3, 4];
let arr_ptr: *const u8 = arr.as_ptr();
unsafe {
    let u32_ptr: *const u32 = core::mem::transmute::<_, *const u32>(arr_ptr);
    let u32_val = u32_ptr.read_unaligned();
    assert_eq!(u32_val.to_ne_bytes(), arr);
}

Of course, transmuting between pointers should not be done, in favor of .cast or as, and Clippy will tell you this, but it's perfectly valid because the pointer is a pointer to the whole array and can be used to access more than just one u8.

@workingjubilee
Copy link
Member

As @asquared31415 noted this cannot be made an error (breaks backcompat, churns too much code) and is a duplicate otherwise with issues like #34249 which recommend linting against transmutes of pointers in a more general fashion.

@workingjubilee workingjubilee closed this as not planned Won't fix, can't repro, duplicate, stale May 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-type-system Area: Type system C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

8 participants