Skip to content

slice::from_raw_parts returns a different address in const context for u8 #105536

Closed

Description

@mxk

Possibly as a result of some u8-specific optimizations, a const &[u8] returned by std::slice::from_raw_parts has a different address than all other uses of the same expression. Forum discussion.

I tried this code:

Playground

use std::ptr::NonNull;
use std::slice::from_raw_parts;

const PTR_U8: *const u8 = NonNull::dangling().as_ptr();
const CONST_U8_REF: &[u8] = unsafe { from_raw_parts(PTR_U8, 0) };
const CONST_U8_PTR: *const u8 = unsafe { from_raw_parts(PTR_U8, 0).as_ptr() };
static STATIC_U8_REF: &[u8] = unsafe { from_raw_parts(PTR_U8, 0) };

const PTR_U16: *const u16 = NonNull::dangling().as_ptr();
const CONST_U16_REF: &[u16] = unsafe { from_raw_parts(PTR_U16, 0) };

const fn const_u8_fn() -> &'static [u8] {
    unsafe { from_raw_parts(PTR_U8, 0) }
}

fn main() {
    let ptr_u8 = unsafe { from_raw_parts(PTR_U8, 0) }.as_ptr();
    let ptr_u16 = unsafe { from_raw_parts(PTR_U16, 0) }.as_ptr();

    assert_eq!(ptr_u8, PTR_U8); // OK
    assert_eq!(ptr_u8, CONST_U8_PTR); // OK
    assert_eq!(ptr_u8, const_u8_fn().as_ptr()); // OK
    assert_eq!(ptr_u8, STATIC_U8_REF.as_ptr()); // OK
    assert_eq!(ptr_u16, CONST_U16_REF.as_ptr()); // OK
    assert_eq!(ptr_u8, CONST_U8_REF.as_ptr()); // ERROR
}

I expected to see this happen: All assertions should pass.

Instead, this happened:

thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `0x1`,
 right: `0x55f434eaa038`', src/main.rs:25:5

Meta

rustc --version --verbose:

rustc 1.65.0 (897e37553 2022-11-02)
binary: rustc
commit-hash: 897e37553bba8b42751c67658967889d11ecd120
commit-date: 2022-11-02
host: x86_64-pc-windows-msvc
release: 1.65.0
LLVM version: 15.0.0
Backtrace

thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `0x1`,
 right: `0x55c4dc25b038`', src/main.rs:25:5
stack backtrace:
   0: rust_begin_unwind
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/std/src/panicking.rs:584:5
   1: core::panicking::panic_fmt
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/core/src/panicking.rs:142:14
   2: core::panicking::assert_failed_inner
   3: core::panicking::assert_failed
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/core/src/panicking.rs:181:5
   4: playground::main
             at ./[src/main.rs:25](https://play.rust-lang.org/#):5
   5: core::ops::function::FnOnce::call_once
             at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/core/src/ops/function.rs:248:5

Activity

  1. steffahn commented on Dec 10, 2022

    @steffahn
    Member

    @rustbot label A-const-eval, T-compiler, A-slice

    In the linked URLO discussion, @LegionMammal978 mentioned some part of the compiler that seems relevant for why this behavior only affects u8 slices in particular, the code history is a bit hard to navigate in the git blame, but AFAICT, @oli-obk and @RalfJung are involved.

    Maybe they also have more insight into whether this kind of behavior classifies as intended or bug, since there definitely are cases where reference addresses of const-eval results being soft-of “unstable” is intended behavior, albeit surprising behavior to some, but I feel like those cases are usually different in that a reference to some actual static(-promoted) data and memory is created.

  2. added
    A-const-evalArea: Constant evaluation, covers all const contexts (static, const fn, ...)
    A-slice-vecArea: `[T]` and `Vec<T>`
    T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.
    on Dec 10, 2022
  3. oli-obk commented on Dec 10, 2022

    @oli-obk
    Contributor

    It's not promotion related: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=b89aaee74c5da29370ba7107576adc7a

    So I'm guessing some sort of codegen issue, because otherwise turning the reference back to a pointer in const land would cause the same issue, but it doesn't: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=6ac87b271a9b495b6497015d95b57815

  4. scottmcm commented on Dec 10, 2022

    @scottmcm
    Member

    Minimized, and emphasizing the same thing oli just showed about this being related to the const item somehow, not a problem in MIR interpreting itself:

    assert_eq!(
        // as_ptr INside the `const{}`
        (const { (unsafe { std::slice::from_raw_parts(3 as *const u8, 0) }).as_ptr() }),
        std::ptr::invalid(3),
    ); // PASSES
    assert_eq!(
        // as_ptr OUTside the `const{}`
        (const { (unsafe { std::slice::from_raw_parts(7 as *const u8, 0) }) }).as_ptr(),
        std::ptr::invalid(7),
    ); // FAILS, 0x56229d3aa00b != 0x7

    https://play.rust-lang.org/?version=nightly&mode=release&edition=2021&gist=5544bc2edee6a4276a57cbeedbb12212

  5. LegionMammal978 commented on Dec 11, 2022

    @LegionMammal978
    Contributor

    It seems like the whole point of op_to_const() is to make this transformation, which occurs a bit further down in the function:

                    debug!("ScalarPair(a: {:?}, b: {:?})", a, b);
                    // We know `offset` is relative to the allocation, so we can use `into_parts`.
                    let (data, start) = match a.to_pointer(ecx).unwrap().into_parts() {
                        (Some(alloc_id), offset) => {
                            (ecx.tcx.global_alloc(alloc_id).unwrap_memory(), offset.bytes())
                        }
                        (None, _offset) => (
                            ecx.tcx.intern_const_alloc(Allocation::from_bytes_byte_aligned_immutable(
                                b"" as &[u8],
                            )),
                            0,
                        ),
                    };
                    let len = b.to_machine_usize(ecx).unwrap();
                    let start = start.try_into().unwrap();
                    let len: usize = len.try_into().unwrap();
                    ConstValue::Slice { data, start, end: start + len }

    That is, a &[u8] or &str without an AllocId (e.g., one from an invalid pointer) is converted to an arbitrary allocation storing b"" as &[u8]. Looking up the call graph, this operation appears to be invoked by eval_to_const_value_raw() and valtree_to_const_value().

    So the reference isn't transformed every time it's used, only in certain contexts, where the slice has to exit const-eval-land and become a real instantiated value. For instance, using a slice as a const-generic parameter triggers the same behavior (Playground):

    #![allow(incomplete_features)]
    #![feature(adt_const_params)]
    use std::slice::from_raw_parts;
    
    struct AsPtr<const SLICE: &'static [u8]>;
    impl<const SLICE: &'static [u8]> AsPtr<SLICE> {
        const PTR: *const u8 = SLICE.as_ptr();
    }
    
    fn main() {
        let ptr = AsPtr::<{ unsafe { from_raw_parts(1 as *const u8, 0) } }>::PTR;
        println!("{ptr:p}");
    }

    Notably, this only occurs when the slice is the entire constant value. The address isn't modified if the slice is a field of a struct, enum, or array.

  6. oli-obk commented on Dec 11, 2022

    @oli-obk
    Contributor

    Considering that we got valtrees now, this will become more common. All addresses in type level constants are now de- or duplicated.

    For these ones that are not used in type level consts, we should just remove the ConstValue::Slice variant, as it is not needed anymore except potentially for perf, but we may be able to preserve that

  7. RalfJung commented on Dec 11, 2022

    @RalfJung
    Member

    op_to_const is a big hack and the sooner we can get rid of it the better...

    For the specific case of u8 slices, IIRC that was needed because one can pattern-match on them. How close is the pattern matching logic to being ported away from ConstValue, to valtrees? Once that is the case, some cleanup should be possible here.

  8. oli-obk commented on Sep 14, 2023

    @oli-obk
    Contributor

    this issue is mitigated by #115764, and we can probably fix it entirely based on that change

  9. RalfJung commented on Sep 14, 2023

    @RalfJung
    Member

    I think that PR should already fix this issue completely?

    ConstValue::Slice is then only created for string literals and when turning valtrees into constants, none of which should happen in the above code.

  10. oli-obk commented on Sep 14, 2023

    @oli-obk
    Contributor

    sweet. wanna add a regression test to the PR, too?

  11. RalfJung commented on Sep 14, 2023

    @RalfJung
    Member

    Sure, will do.

    @mxk note that we do not guarantee that these pointers will always be equal. We always reserve the right to duplicate the values of const.

    But in this case the inequality was a symptom of the compiler doing some unnecessary work, and that's why it got fixed.

  12. added a commit that references this issue on Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-const-evalArea: Constant evaluation, covers all const contexts (static, const fn, ...)A-slice-vecArea: `[T]` and `Vec<T>`C-bugCategory: This is a bug.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions