request: lint when raw pointer slicing implicitly creates a temporary reference

So let's say you know about aliasing models and have learned that to avoid all aliasing rules of Rust you need to use raw pointers throughout, no references. So for example you write:

use std::ptr::addr_of_mut;

pub fn test(ptr: *mut [u8]) -> *mut [u8] {
    let layout_size = 24;
    unsafe { addr_of_mut!((*(ptr))[..layout_size]) }
}

Looks good, doesn't it?
Well sadly the MIR for this code looks as follows:

fn test(_1: *mut [u8]) -> *mut [u8] {
    debug ptr => _1;                     // in scope 0 at [src/lib.rs:3:13: 3:16](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    let mut _0: *mut [u8];               // return place in scope 0 at [src/lib.rs:3:32: 3:41](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    let _2: usize;                       // in scope 0 at [src/lib.rs:4:9: 4:20](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    let mut _3: &mut [u8];               // in scope 0 at [src/lib.rs:5:27: 5:50](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    let mut _4: &mut [u8];               // in scope 0 at [src/lib.rs:5:27: 5:35](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    let mut _5: std::ops::RangeTo<usize>; // in scope 0 at [src/lib.rs:5:36: 5:49](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    scope 1 {
        debug layout_size => _2;         // in scope 1 at [src/lib.rs:4:9: 4:20](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
        scope 2 {
        }
    }

    bb0: {
        _2 = const 24_usize;             // scope 0 at [src/lib.rs:4:23: 4:25](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
        _4 = &mut (*_1);                 // scope 2 at [src/lib.rs:5:27: 5:35](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
        Deinit(_5);                      // scope 2 at [src/lib.rs:5:36: 5:49](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
        (_5.0: usize) = const 24_usize;  // scope 2 at [src/lib.rs:5:36: 5:49](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
        _3 = <[u8] as IndexMut<RangeTo<usize>>>::index_mut(move _4, move _5) -> bb1; // scope 2 at [src/lib.rs:5:27: 5:50](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
                                         // mir::Constant
                                         // + span: [src/lib.rs:5:27: 5:50](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
                                         // + literal: Const { ty: for<'r> fn(&'r mut [u8], RangeTo<usize>) -> &'r mut <[u8] as Index<RangeTo<usize>>>::Output {<[u8] as IndexMut<RangeTo<usize>>>::index_mut}, val: Value(<ZST>) }
    }

    bb1: {
        _0 = &raw mut (*_3);             // scope 2 at /rustc/263edd43c5255084292329423c61a9d69715ebfa/library/core/src/ptr/mod.rs:2005:5: 2005:20
        return;                          // scope 0 at [src/lib.rs:6:2: 6:2](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021#)
    }
}

Note the _4 = &mut (*_1): Rust "helpfully" created a reference for us to subslice into!
This is terrible, because it introduces UB.

This code should either be rejected, or use raw pointers throughout.
Related to #73987.

This ties into the concept that has come up a few times of distinguishing an "unsafe place" (one created from a raw pointer deref) from a "safe place" (on created from a reference deref).

I don't think we want to distinguish them in the AM semantics (just like unsafe code has no effect on the operational semantics), but I do think that this distinction could be quite meaningful for diagnostics and possibly for elaboration (auto-ref, auto-deref, all that stuff).

To repeat my other comment, the most unexpected part IMO is that this is valid with a single index. All official sources say that index notation goes through the Index[Mut] traits in general. Since these traits take and produce ordinary references, I'd assume by default that any indexing is unsound here. Only the Reference hints that arrays and slices might not use Index[Mut], and it says nothing about single indexing not producing transient references.

I agree that this particular case can be footgunny, though (you'd almost never want to directly IndexMut a raw *mut [T]), so I'd be in favor of making it a warn-by-default lint. The question is, what is the correct alternative? If the subslice is a constant-sized prefix, as it is here, existing stable methods can handle it, as long as you can stomach the lack of bounds checks:

use std::ptr::{self, addr_of_mut};

pub fn test(ptr: *mut [u8]) -> *mut [u8] {
    let layout_size = 24;
    unsafe { ptr::slice_from_raw_parts_mut(ptr.cast::<u8>(), layout_size) }
}

But if the subslice is length-dependent (e.g., for a suffix), it's a lot more complicated, since <*mut [T]>::len() is still unstable. We'd have to do a more roundabout metadata decomposition, which doesn't even work for null slice pointers:

use std::ptr::{self, addr_of_mut};

pub fn test(ptr: *mut [u8]) -> *mut [u8] {
    assert!(
        !ptr.is_null(),
        "no stable way to get the len of a null slice ptr"
    );
    let layout_size = 24;
    let len = unsafe { (*(ptr as *mut [()])).len() };
    assert!(len >= layout_size, "slice too short");
    let start = unsafe { ptr.cast::<u8>().add(len - layout_size) };
    unsafe { ptr::slice_from_raw_parts_mut(start, layout_size) }
}

(Weirdness like this is why I generally avoid slice pointers altogether.)

I think the correct alternative is to use the methods tracked in #74265, though those don't do bounds checks.

fix(sallyport): fix UB by avoiding implicit reference from indexing w…

fix(sallyport): avoid UB by removing implicit reference from indexing…

Rejecting code like that is probably going to be hard, but both this and #73987 can be mitigated by a lint I think. Concretely, what happens here is that the compiler desugars (*ptr)[..layout_size] to IndexMut::index_mut(&mut *ptr, ..layout_size). Whenever such an &mut is inserted implicitly, I think we want to check the place it is applied to, and if that place was constructed by deref'ing a raw pointer, we should have a lint. The lint should suggest to change the code to (&mut *ptr)[..layout_size], to make it explicit that a reference is being created here. The lint should also explain that creating a reference (explicitly or implicitly) comes with a bunch of requirements regarding the data the reference points to, and aliasing requirements on how the reference is being used.

@rustbot

@rustbot claim

@Urgau

@Urgau is implementing this in #123239

Navigation Menu

Repository navigation

request: lint when raw pointer slicing implicitly creates a temporary reference #99437

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

request: lint when raw pointer slicing implicitly creates a temporary reference #99437

Description

Activity

CAD97 commented on Jul 18, 2022

RalfJung commented on Jul 18, 2022

LegionMammal978 commented on Jul 19, 2022

RalfJung commented on Jul 19, 2022

RalfJung commented on Oct 29, 2022

WaffleLapkin commented on Oct 29, 2022

WaffleLapkin commented on Apr 13, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions