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

BikeshedIntrinsicFrom permits lifetime extension #129097

Closed
jswrenn opened this issue Aug 14, 2024 · 2 comments · Fixed by #129217
Closed

BikeshedIntrinsicFrom permits lifetime extension #129097

jswrenn opened this issue Aug 14, 2024 · 2 comments · Fixed by #129217
Assignees
Labels
C-bug Category: This is a bug.

Comments

@jswrenn
Copy link
Member

jswrenn commented Aug 14, 2024

I tried this code:

#![feature(transmutability, core_intrinsics)]

use std::mem::{Assume, BikeshedIntrinsicFrom};

fn extend<'a>(src: &'a u8) -> &'static u8 {
    transmute::<&'a u8, &'static u8>(src)
}

fn transmute<Src, Dst>(src: Src) -> Dst
where
    Dst: BikeshedIntrinsicFrom<Src>,
{
    unsafe { core::intrinsics::transmute_unchecked(src) }
}


fn main() {}

I expected to see this to fail with a compilation error. Instead it succeeded.

Meta

rustc --version --verbose:

rustc 1.82.0-nightly (3e9bd8b56 2024-08-08)
binary: rustc
commit-hash: 3e9bd8b566a47c5d1c1dbc7e043b4b7fa5330eca
commit-date: 2024-08-08
host: x86_64-unknown-linux-gnu
release: 1.82.0-nightly
LLVM version: 19.1.0
@jswrenn jswrenn added the C-bug Category: This is a bug. label Aug 14, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 14, 2024
@jswrenn
Copy link
Member Author

jswrenn commented Aug 14, 2024

@rustbot claim

@jswrenn
Copy link
Member Author

jswrenn commented Aug 14, 2024

It looks like this was just never implemented when support for references was added, rather than this being a case of a subtle bug. I've begun to address this on a branch by modifying confirm_transmutability_candidate to emit outlives obligations: https://github.com/jswrenn/rust/blob/6176aafb7a9568ac1a78f16b4f9fba703df4cd55/compiler/rustc_trait_selection/src/traits/select/confirmation.rs#L290-L455

I'm emitting outlives obligations in two places. First, where we handle IfTransmutable answers: https://github.com/jswrenn/rust/blob/6176aafb7a9568ac1a78f16b4f9fba703df4cd55/compiler/rustc_trait_selection/src/traits/select/confirmation.rs#L397-L403

This should be sufficient. The other kinds of obligations emitted by the reference_obligations helper are exercised, and errors arise from violating them.

For some reason, it's not. I've also added these obligations at a top level in confirm_transmutability_candidate, peaking into the src and dst types and eagerly emitting reference obligations if needed: https://github.com/jswrenn/rust/blob/6176aafb7a9568ac1a78f16b4f9fba703df4cd55/compiler/rustc_trait_selection/src/traits/select/confirmation.rs#L431-L443

This is sufficient for catching 'top level' lifetime extensions; e.g.:

fn extend<'a>(src: &'a u8) -> &'static u8 {
    transmute(src) // error, yay!
}

fn transmute<Src, Dst>(src: Src) -> Dst
where
    Dst: BikeshedIntrinsicFrom<Src>,
{
    unsafe { core::intrinsics::transmute_unchecked(src) }
}

...but not nested lifetime extensions, like:

fn extend_nested<'a>(src: (&'a u8, u8)) -> (&'static u8, u8) {
    transmute(src) // no error :(
}

jswrenn added a commit to jswrenn/rust that referenced this issue Aug 17, 2024
Modifies `BikeshedIntrinsicFrom` to forbid lifetime extensions on
references. This static check can be opted out of with the
`Assume::lifetimes` flag.

Fixes rust-lang#129097
jswrenn added a commit to jswrenn/rust that referenced this issue Aug 17, 2024
Modifies `BikeshedIntrinsicFrom` to forbid lifetime extensions on
references. This static check can be opted out of with the
`Assume::lifetimes` flag.

Fixes rust-lang#129097
jswrenn added a commit to jswrenn/rust that referenced this issue Aug 17, 2024
Modifies `BikeshedIntrinsicFrom` to forbid lifetime extensions on
references. This static check can be opted out of with the
`Assume::lifetimes` flag.

Fixes rust-lang#129097
jswrenn added a commit to jswrenn/rust that referenced this issue Aug 18, 2024
Modifies `BikeshedIntrinsicFrom` to forbid lifetime extensions on
references. This static check can be opted out of with the
`Assume::lifetimes` flag.

Fixes rust-lang#129097
jswrenn added a commit to jswrenn/rust that referenced this issue Aug 18, 2024
Modifies `BikeshedIntrinsicFrom` to forbid lifetime extensions on
references. This static check can be opted out of with the
`Assume::lifetimes` flag.

Fixes rust-lang#129097
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 19, 2024
…piler-errors

safe transmute: check lifetimes

Modifies `BikeshedIntrinsicFrom` to forbid lifetime extensions on references. This static check can be opted out of with the `Assume::lifetimes` flag.

Fixes rust-lang#129097

Tracking Issue: rust-lang#99571

 r​? `@compiler-errors`
@bors bors closed this as completed in 17995d5 Aug 19, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Aug 19, 2024
Rollup merge of rust-lang#129217 - jswrenn:transmute-lifetimes, r=compiler-errors

safe transmute: check lifetimes

Modifies `BikeshedIntrinsicFrom` to forbid lifetime extensions on references. This static check can be opted out of with the `Assume::lifetimes` flag.

Fixes rust-lang#129097

Tracking Issue: rust-lang#99571

 r​? `@compiler-errors`
@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants