Skip to content

Static checks for &mut in final value of constant with #![feature(const_mut_refs)] #71212

Closed

Description

@ecstatic-morse

On stable, we prevent users from creating an &mut that points to memory inside a const by forbidding the creation of mutable references during const-eval. This limitation is only temporary, see #57349. We have a feature flag, const_mut_refs, that allows users to create mutable references, but no attempt is made to prevent &mut from escaping into the final value of a const like so:

#![feature(const_mut_refs)]
const FOO: &mut i32 = &mut 4;

fn main() {
    *FOO = 2;
}

This errors on the current nightly, and if there were a feature gate that allowed it, we would get an ICE:

error: internal compiler error: src/librustc_mir/interpret/intern.rs:238: const qualif failed to prevent mutable references

I think we've not yet settled on the semantics we want. We're allowing them in const fn and relying on the borrow checker to prevent references from escaping.

Activity

  1. ecstatic-morse commented on Apr 16, 2020

    @ecstatic-morse
    ContributorAuthor

    static mut is also an interesting case. Should the following compile? Perhaps, although with my proposal the user would be forced to wrap the initializer of FOO_MUT in a const fn. This suggests to me that checking for &mut in the type of a const is not the correct solution.

    static mut FOO: i32 = 4;
    const FOO_MUT: &mut i32 = unsafe { &mut FOO };
    
    fn main() {
        *FOO_MUT = 2;
    }
  2. added
    A-const-evalArea: Constant evaluation, covers all const contexts (static, const fn, ...)
    C-bugCategory: This is a bug.
    I-ICEIssue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️
    requires-nightlyThis issue requires a nightly compiler in some way. When possible, use a F-* label instead.
    T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.
    on Apr 16, 2020
  3. added
    B-unstableBlocker: Implemented in the nightly compiler and unstable.
    and removed
    C-bugCategory: This is a bug.
    I-ICEIssue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️
    T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.
    requires-nightlyThis issue requires a nightly compiler in some way. When possible, use a F-* label instead.
    on Apr 16, 2020
  4. 7 remaining items

  5. RalfJung commented on Apr 17, 2020

    @RalfJung
    Member
    static mut FOO: i32 = 4;
    const FOO_MUT: &mut i32 = unsafe { &mut FOO };
    
    fn main() {
        *FOO_MUT = 2;
    }

    Another interesting question here could be aliasing. Usually mutable references are unique. Does that mean that the following is UB because uniqueness got violated (FOO was accessed through a different place between creation and use of FOO_MUT)?

    static mut FOO: i32 = 4;
    const FOO_MUT: &mut i32 = unsafe { &mut FOO };
    
    fn main() { unsafe {
        FOO += 5;
        *FOO_MUT = 2;
    } }
  6. RalfJung commented on Apr 17, 2020

    @RalfJung
    Member

    (Also strangely I did not get emailed about the ping above.)

  7. ecstatic-morse commented on Apr 17, 2020

    @ecstatic-morse
    ContributorAuthor

    Ah, that's a good point. In that case, I think we can ignore that example as an argument against the type visitor approach.

    Perhaps this is a better example. I believe that things at the top level of a const initializer have a static storage duration. I'm not actually sure whether this is user-observable and guaranteed since rvalue static promotion is pretty aggressive inside constants. NULL and A currently compile on stable and, assuming that I'm right that neither A nor B dangle, reading from either one is not UB. B, however, must fail to compile, but a type-based check on the final value of a constant would reject NULL as well.

    // Compile on stable and work as expected.
    const NULL: *mut i32 = ptr::null_mut(); 
    const A: *const i32 = &4;
    
    // Must not compile.
    const B: *mut i32 = &mut 4;
    
    fn main() {
        println!("{}", unsafe { *A });
        // unsafe { *B = 4 } // Bad news
    }
  8. JohnTitor commented on May 17, 2020

    @JohnTitor
    Member

    Triage: the ICE no longer occurs with the latest nightly by #71665.

  9. RalfJung commented on May 17, 2020

    @RalfJung
    Member

    The bug is not primarily about the ICE though, it is about what static checks we need in place to avoid mis-behaved &mut constants.

  10. removed
    I-ICEIssue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️
    on May 17, 2020
  11. RalfJung commented on May 17, 2020

    @RalfJung
    Member

    What actually happens currently is that interning detects the mutable reference in a const and complains about it -- good.

    I wonder if validation should do that check, instead... but currently validation is a separate query so there is always the risk that it does not run. :/ Things would be so much easier if the raw const query would always do validation and thus ensure that only validated values ever enter the rest of the compiler...

  12. oli-obk commented on May 19, 2020

    @oli-obk
    Contributor

    Things would be so much easier if the raw const query would always do validation and thus ensure that only validated values ever enter the rest of the compiler...

    If we make validation and interning stop when encountering GlobalAlloc::Static, we can do this.

  13. RalfJung commented on May 20, 2020

    @RalfJung
    Member

    @oli-obk That would be awesome! I laid down my thoughts in #72396. If that works, I'd be really happy. :D

  14. added a commit that references this issue on Jun 20, 2020
  15. oli-obk commented on Dec 4, 2020

    @oli-obk
    Contributor

    Another fun bug with static mut and const_mut_refs:

    https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=baf2a1bbbf7ba1b5a1c9d7015c2d5b2c

    #![feature(const_mut_refs)]
    pub mod a {
        #[no_mangle]
        pub static mut FOO: &mut [i32] = &mut [42];
    }
    
    pub mod b {
        #[no_mangle]
        pub static mut BAR: &mut [i32] = unsafe { &mut *crate::a::FOO };
    }
    
    fn main() {
        unsafe {
            assert_eq!(a::FOO.as_ptr(), b::BAR.as_ptr());
        }
    }

    The assertion fails, even though I belive that it should most definitely not. Only base allocations of statics have a guaranteed address and won't get deduplicated.

  16. RalfJung commented on Dec 5, 2020

    @RalfJung
    Member

    @oli-obk Shouldn't that be a separate bug? That has nothing to do with static checks or so, this seems like a codegen issue. &mut *crate::a::FOO should return the original pointer.

  17. oli-obk commented on Dec 7, 2020

    @oli-obk
    Contributor

    It's indeed a separate bug, but also a blocker for this feature gate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

A-const-evalArea: Constant evaluation, covers all const contexts (static, const fn, ...)C-bugCategory: This is a bug.F-const_mut_refs`#![feature(const_mut_refs)]`T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.glacierICE tracked in rust-lang/glacier.requires-nightlyThis issue requires a nightly compiler in some way. When possible, use a F-* label instead.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions